Skip to content

Avoid typing annotations twice #4662

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

Merged
merged 1 commit into from
Jun 26, 2018

Conversation

smarter
Copy link
Member

@smarter smarter commented Jun 14, 2018

typedAnnotation was used both in Namer#addAnnotations and
Typer#completeAnnotations, resulting in some annotations being typed
twice. Fixed by introducing a typedAheadAnnotation (and renaming an
existing method with the same name that doesn't actually type the full
annotation tree).

`typedAnnotation` was used both in `Namer#addAnnotations` and
`Typer#completeAnnotations`, resulting in some annotations being typed
twice. Fixed by introducing a `typedAheadAnnotation` (and renaming an
existing method with the same name that doesn't actually type the full
annotation tree).
@smarter
Copy link
Member Author

smarter commented Jun 14, 2018

@odersky Both Namer#addAnnotations and Typer#completeAnnotations define a lazy val annotCtx, but the definitions are different and don't always produce the same result, compare:
https://github.com/lampepfl/dotty/blob/78a5e092a834b76c54b18d5e58cb0a593f4bb8a5/compiler/src/dotty/tools/dotc/typer/Namer.scala#L741-L747
https://github.com/lampepfl/dotty/blob/78a5e092a834b76c54b18d5e58cb0a593f4bb8a5/compiler/src/dotty/tools/dotc/typer/Typer.scala#L1345-L1355

Is one "more correct" than the other? Can we come up with a single definition that we can use in both?

@smarter
Copy link
Member Author

smarter commented Jun 14, 2018

There's another issue which is that we're not actually using the annotCtx with Annotation.deferred:
https://github.com/lampepfl/dotty/blob/78a5e092a834b76c54b18d5e58cb0a593f4bb8a5/compiler/src/dotty/tools/dotc/typer/Namer.scala#L751
The ctx that will be passed here will just be the enclosing context.

@smarter smarter requested a review from odersky June 14, 2018 21:02
@odersky odersky merged commit e88a6a5 into scala:master Jun 26, 2018
@allanrenucci allanrenucci deleted the fix/typedAheadAnnotation branch June 26, 2018 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants