Simplify super call handling: remove inConstrCall, remove InSuperCall #8591
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `inConstrCall` parameter passed to `tpd.Super` and `assignType`
should be true for super-calls to parent class constructors and false
otherwise, this is used to correctly type the Super node:
else if (inConstrCall || ctx.erasedTypes) cls.info.firstParent.typeConstructor
But calls to super-constructor are only generated by Mixin which runs
after Erasure, so we can don't `inConstrCall` to take the correct branch.
There were two places before erasure where we didn't just pass
`inConstrCall = false` and they were both wrong:
- In Typer, we had:
case pt: SelectionProto if pt.name == nme.CONSTRUCTOR => true
But Typer never generates super-calls to constructors (such calls are
just regular constructor calls in the extends clause until Mixin)
- In TreeUnpickler, we had:
ctx.mode.is(Mode.InSuperCall)
Which doesn't really make sense (it checks whether the super-call is
happening inside the extends clause or a secondary constructor)
smarter
force-pushed
the
try/super-constr-call
branch
from
b026c5d to
cc86be4
Compare
After the previous commit, there was one usage of InSuperCall left in typedIdent to provide a better error message, but that usage can be replaced by a check to make sure we're in a secondary constructor instead, which means we can get rid of this mode avoid having to set and unset it in a bunch of places.
smarter
force-pushed
the
try/super-constr-call
branch
from
cc86be4 to
e9b62b7
Compare
We could use a context function but for now let's avoid using Scala 3 features in the tasty code to make it easier to port to Scala 2.
smarter
force-pushed
the
try/super-constr-call
branch
from
e9b62b7 to
c94c58e
Compare
odersky
approved these changes
Mar 22, 2020
| @@ -670,7 +670,7 @@ class TreeUnpickler(reader: TastyReader, | |||
| readByte() | |||
| val end = readEnd() | |||
| val tp = readType() | |||
| val lazyAnnotTree = readLaterWithOwner(end, rdr => ctx => rdr.readTerm()(ctx)) | |||
| val lazyAnnotTree = readLaterWithOwner(end, rdr => implicit ctx => rdr.readTerm()) | |||
There was a problem hiding this comment.
I propose we go to the new syntax (using ctx) => rdr.readTerm().
There was a problem hiding this comment.
That doesn't work because the expected type is not a context function type but a regular function type, and I was hesitant to change this since it'll make it harder to port code between the Scala 2 and 3 readers.
| @@ -780,7 +780,7 @@ class TreeUnpickler(reader: TastyReader, | |||
| def complete(implicit ctx: Context) = typer.Inliner.bodyToInline(sym) | |||
| } | |||
| else | |||
| readLater(end, rdr => ctx => rdr.readTerm()(ctx)) | |||
| readLater(end, rdr => implicit ctx => rdr.readTerm()) | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.