Skip to content

Evaluate annotations in Namer #641

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 3 commits into from
Jun 21, 2015

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jun 15, 2015

Annotations were evaluated in Typer, which meant that they could be used only after type checking was
complete (or compilation order dependencies would be introduced). This commit makes annotations be
installed as part of completion in Namer. However, annotations are now lazy so as to avoid cyclic
references. This is completely analogous to the scheme in unpickler and ClassfileParser.

@odersky
Copy link
Contributor Author

odersky commented Jun 15, 2015

Review by @xeno-by

@odersky
Copy link
Contributor Author

odersky commented Jun 15, 2015

@xeno-by I would like to do it even earlier, during enter, but that currently does not pass the test suite.

@xeno-by
Copy link

xeno-by commented Jun 15, 2015

I will try take a look tomorrow. Would that work for you?

@odersky
Copy link
Contributor Author

odersky commented Jun 15, 2015

@xeno-by Sure.

@xeno-by
Copy link

xeno-by commented Jun 16, 2015

So, if I'm getting this right, this pull requests makes typechecking annotation types eager upon completion of the underlying member, but annotation arguments still remain lazy?

Annotations were evaluated in Typer, which meant that they could be used only after type checking was
complete (or compilation order dependencies would be introduced). This commit makes annotations be
installed as part of completion in Namer. However, annotations are now lazy so as to avoid cyclic
references. This is completely analogous to the scheme in unpickler and ClassfileParser.
@odersky odersky force-pushed the change/early-annotations branch from 7196517 to d59ab5d Compare June 19, 2015 10:08
@odersky
Copy link
Contributor Author

odersky commented Jun 19, 2015

I tried to follow @olhotak's suggestion but that caused errors, unfortunately. A more conservative commit will follow.

@odersky odersky force-pushed the change/early-annotations branch from d59ab5d to abbc707 Compare June 19, 2015 10:53
@odersky
Copy link
Contributor Author

odersky commented Jun 19, 2015

@olhotak Anything further, or can we merge this?

@olhotak
Copy link
Contributor

olhotak commented Jun 19, 2015

LGTM

odersky added a commit that referenced this pull request Jun 21, 2015
@odersky odersky merged commit 478be16 into scala:master Jun 21, 2015
@allanrenucci allanrenucci deleted the change/early-annotations branch December 14, 2017 16:58
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.

4 participants