Skip to content

Regression on path dependent types from 3.2 #21536

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
AnnelineD opened this issue Sep 4, 2024 · 6 comments
Closed

Regression on path dependent types from 3.2 #21536

AnnelineD opened this issue Sep 4, 2024 · 6 comments

Comments

@AnnelineD
Copy link

Compiler version

Works on 3.2.2 and fails on 3.3.2 and 3.5.0.

Minimized code

object PatchG:
   inline def apply[EL, MG <: DNIELMG[EL]](inline mg: MG)(inline ptps: List[(mg.Node | C.type, mg.Node | C.type)]): PatchG[EL, MG]

https://github.com/Adam-Vandervorst/LLGraph/blob/10f010e536beb1c8fb5f5a8bf80f7c308dc72768/src/main/scala/be.adamv.llgraph/graphs/PatchG.scala#L112

Output

[error] -- [E083] Type Error: /home/anneline/IdeaProjects/LLGraph/src/main/scala/be.adamv.llgraph/graphs/PatchG.scala:112:76 
[error] 112 |  inline def apply[EL, MG <: DNIELMG[EL]](inline mg: MG)(inline ptps: List[(mg.Node | C.type, mg.Node | C.type)]): PatchG[EL, MG] =
[error]     |                                                                            ^^
[error]     |(mg : MG) is not a valid type prefix, since it is not an immutable path
[error]     |---------------------------------------------------------------------------
[error]     | Explanation (enabled by `-explain`)
[error]     |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[error]     | An immutable path is
[error]     |  - a reference to an immutable value, or
[error]     |  - a reference to `this`, or
[error]     |  - a selection of an immutable path with an immutable value.
[error]      ---------------------------------------------------------------------------

Expectation

To compile, since the inlined version does type-check.
Example usage:

  val rhs = DNIELMG[Char]()
  val r = rhs.newNodes(2)
  rhs.connect(r(0), r(0), 'b')

  val rpg = PatchG[Char, rhs.type](rhs)(List(C -> r(0), r(1) -> C))

https://github.com/Adam-Vandervorst/LLGraph/blob/10f010e536beb1c8fb5f5a8bf80f7c308dc72768/src/test/scala/PatchGTest.scala#L24

@AnnelineD AnnelineD added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Sep 4, 2024
@AnnelineD
Copy link
Author

I guess this relates to #16804 but the answer there is not satisfactory, since dropping the path-dependent-types from this (and its down the line) project seriously hampers its type safety.

What is the workaround? Can this be expressed via implicits somehow since they are unique (the cause identified in @soronpo his issue)?

@Gedochao Gedochao added stat:needs minimization Needs a self contained minimization area:inline and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Sep 4, 2024
@Gedochao
Copy link
Contributor

Gedochao commented Sep 4, 2024

cc @jchyb

@jchyb
Copy link
Contributor

jchyb commented Sep 4, 2024

Hi @AnnelineD. Looking at the linked project, it looks to me like it should be enough to just drop the inline modifier from the mg parameter, since its "inlineness" does not seem to be required anywhere:

object PatchG:
   inline def apply[EL, MG <: DNIELMG[EL]](mg: MG)(inline ptps: List[(mg.Node | C.type, mg.Node | C.type)]): PatchG[EL, MG]

This will make the mg into an immutable path, and inline into roughly the same form as in the Expectation field, with the val mg = DNIELMG[Char]() being a separate val (with an immutable path), instead of being directly inlined.

@soronpo
Copy link
Contributor

soronpo commented Sep 4, 2024

@Gedochao I'm looking again on the original issue, and it seems that people run into it several more times and spent time to minimize it. I think we should change the problem to a reporting issue. The compiler needs to suggest to remove the inline to solve the problem.

@Gedochao
Copy link
Contributor

Gedochao commented Sep 4, 2024

@soronpo Agreed. Can you re-raise this as a reporting issue? linking back to #21536 and #16804.
The scope/requirements are different enough to warrant a separate ticket IMO, if we're to address it.
The behaviour is as intended, we just need to improve the error.
This issue is a won'tfix as it's currently defined.

@Gedochao
Copy link
Contributor

Gedochao commented Sep 4, 2024

Closing this, the reporting improvement should be tracked under #21538

@Gedochao Gedochao closed this as not planned Won't fix, can't repro, duplicate, stale Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants