-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Path dependency on trait/class-parameters in class parent uses constructor argument instead of member #5636
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
Comments
I played a bit with this time ago. The problem is that there are two distinct as, the constructor parameter and the field, and a priori they are not given the same singleton type (tho of course they could). And when the super call is typechecked, a refers to the constructor argument, not the field, which matches the runtime semantics. When I tried changing that, I ended up producing a super call that accessed the field instead of the constructor parameter, which failed the bytecode verifier. I have more extensive notes somewhere on what I tried and what else I thought one might try, I hope I can get to them. |
Hey, are there any new insights on this problem? |
Just to make that clear, the problem not only occurs with singleton types, but of course also extends to type members as in: class A { type M }
trait Bar[X] {
// same for `val foo: X = ???`
def foo: X = ???
}
class Foo(val a: A) extends Bar[a.M] {
val same: a.M = foo
} which in
|
@Blaisorblade I don't understand why you would change the runtime semantics (the bytecode modifications you mentioned above).
Is there a reason why (immutable) field and constructor argument are not given the same singleton type? |
Yeah, that's indeed the natural consequence.
Let's ignore that — I tried a certain fix, and it had the wrong effects. IIRC, I changed some references to
Because you'd need to write special code for that. Constructor arguments and fields are different variables. After I now wonder if/how this relates to "double vision" or is distinct — a tricky problem that showed up when implementing opaque types, and is known in the literature (and solved in Dreyer MixML papers, and others). Fixing this would probably require a dedicated type rule to use in constructors. And I don't know how easy it'd be to make that algorithmic. Now, maybe this still takes Martin an afternoon, but dunno. |
Actually, why not? Would this cause unsoundness or problems with type inference?
Of course the above does not type check for very similar reasons. It again comes down to:
Expressing a similar scenario with functions
obviously shows the same behavior, since no singleton type is inferred as result type of the function. From an engineering point of view we don't want that since this leaks implementation details, right? So maybe the same argument also holds for the constructor case? However, the function can easily be changed to return a singleton type, while for the constructor case we have to go down a much more involved path:
|
What I mean is: If we replaced I've looked a bit at your example, but it doesn't share the problematic circularity. Maybe it should be a separate issue? Re double vision, see page 7 of https://people.mpi-sws.org/~rossberg/mixml/mixml-icfp08.pdf, for why I think the problem is at least analogous. |
I am way too far away from the compiler internals to make precise statements here, but I would have actually proposed it the other way around. Something like Oh, actually. For the original example, this is somewhat a workaround: trait Foo(val _a: A)(val a: _a.type = _a) extends Bar[a.type] {
val same: a.type = foo
} I guess I am using the constructor argument in |
Oh yeah, I also thought of that direction, but indeed that doesn't work as-is... but maybe somebody more expert could add code to simulate that, but only inside the constructor. IIRC I've even seen code doing similar hacks, but it's subtle.
Uh, can you use trait Foo[AT <: Singleton & A](val a: AT = _a) extends Bar[AT] {
val same: AT = foo // easy
// val same: a.type = foo //maybe needs a fix for #4583
} Now, #4583 has also been open for a while, but it's maybe a bit easier to imagine a fix. |
Yes, I sometimes used |
I thought so too but that's not actually the case, the extends clause is typechecked in this context: https://github.com/lampepfl/dotty/blob/10526a7d0aa8910729b6036ee51942e05b71abf6/compiler/src/dotty/tools/dotc/core/Contexts.scala#L366 note in particular the comment: * - At the same time the context should see the parameter accessors of the current class,
* that's why they get added to the local scope. An alternative would have been to have the
* context see the constructor parameters instead, but then we'd need a final substitution step
* from constructor parameters to class parameter accessors. So the singleton type we see should really be |
Ah, I remember I saw that code and considered trying to patch that — but it seemed far too complex for my knowledge level! @smarter thanks!!! @b-studios can you please make sure to stress-test the PR, in particular with the original non-minimized code (if you have any)? |
Thanks @smarter for looking into it. @Blaisorblade I'm happy to stress test it and will report on the results here, later. |
I tested it on my codebase and it looks good to me 👍 I am now left with the (mostly) unrelated problem, that the following does not type check:
Since the field is not refined to the singleton type per-se, Anyways: Thanks for fixing the current issue :) |
Ooh, that's where we'd try using
|
Fix #5636: properly type param accessors in constructors
@b-studios see #3920 for some related discussions. |
Using path-dependent types as type parameters fails. The following example
gives a type error:
The text was updated successfully, but these errors were encountered: