Skip to content
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

Lift artificial restrictions on ConstantAnnotations #9379

Merged
merged 3 commits into from Dec 17, 2020
Merged

Conversation

lrytz
Copy link
Member

@lrytz lrytz commented Dec 9, 2020

This PR drops some artificial restrictions for annotations extending ConstantAnnotation

  • @constAnn(arg1, arg2) is now allowed without using named arguments
  • @constAnn(arg) is now allowed for a generic annotation class constAnn[T](x: T) extends ConstantAnnotation

This accomplished by typechecking ConstantAnnotations normally, not like Java annotations.

So far we handled ConstantAnnotations the same as Java annotations.
Instead of typechecking the full parse tree Apply(New(annot), args),
the New and the individual args were typed individually. This is
needed for Java annotations as there's no corresponding constructor.
But for ConstantAnnotations we can just type check them normally and
then extract the constants.

Fixes scala/bug#11724.

Context: #9336 (comment)

@scala-jenkins scala-jenkins added this to the 2.13.5 milestone Dec 9, 2020
@lrytz lrytz force-pushed the constAnn branch 2 times, most recently from 56d2c42 to 0110f9a Compare December 11, 2020 20:22
@lrytz lrytz changed the title typecheck ConstantAnnotation normally Typecheck ConstantAnnotations normally, not like Java annotations Dec 11, 2020
@lrytz lrytz marked this pull request as ready for review December 11, 2020 20:28
@lrytz lrytz requested a review from dwijnand December 11, 2020 20:29
Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shame this has to undo what typer did to named/default args, making it net more LOC in the compiler. But I guess the effect is progress.

So far we handled ConstantAnnotations the same as Java annotations.
Instead of typechecking the full parse tree `Apply(New(annot), args)`,
the `New` and the individual `args` were typed individually. This is
needed for Java annotations as there's no corresponding constructor.
But for ConstantAnnotations we can just type check them normally and
then extract the constants.
When extracting constants from an already typed ConstantAnnotation,
don't call the typer again.
Java supports nested annotations as constant annotation arguments, but
Scala's ConstantAnnotation doesn't. Better error message for this case.
@lrytz lrytz merged commit 0772f76 into scala:2.13.x Dec 17, 2020
@lrytz lrytz deleted the constAnn branch December 17, 2020 11:59
@dwijnand dwijnand added the release-notes worth highlighting in next release notes label Dec 17, 2020
@dwijnand
Copy link
Member

It's not big, but it allows for more things, so I feel we should bullet point this for users, in the release notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
3 participants