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

Omit @nowarn annotations from generated code, for forwards compatibility at compile-time #9491

Merged
merged 1 commit into from Feb 18, 2021

Conversation

lrytz
Copy link
Member

@lrytz lrytz commented Feb 10, 2021

...by special-casing them.

In principle we have extends Annotation vs extends StaticAnnotation
for that, but sadly ConstantAnnotation extends StaticAnnotation, so
we don't get to choose for those :-/

Fixes scala/bug#12336

(Note that the bug only caused problems at compile time, not at runtime)

...by special-casing them.

In principle we have `extends Annotation` vs `extends StaticAnnotation`
for that, but sadly `ConstantAnnotation extends StaticAnnotation`, so
we don't get to choose for those :-/
@lrytz lrytz requested a review from retronym February 10, 2021 21:09
@scala-jenkins scala-jenkins added this to the 2.13.6 milestone Feb 10, 2021
@lrytz lrytz modified the milestones: 2.13.6, 2.13.5 Feb 10, 2021
@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Feb 10, 2021
@SethTisue SethTisue changed the title Don't pickle @nowarn annotations... Omit @nowarn annotations from generated code, for forwards compatibility Feb 10, 2021
@SethTisue SethTisue added the prio:blocker release blocker (used only by core team, only near release time) label Feb 10, 2021
@SethTisue SethTisue changed the title Omit @nowarn annotations from generated code, for forwards compatibility Omit @nowarn annotations from generated code, for forwards compatibility at compile-time Feb 10, 2021
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.

LGTM. I guess we don't have the testing infra to simulate compiling with a different/older stdlib.

@SethTisue
Copy link
Member

SethTisue commented Feb 16, 2021

@lrytz this should target 2.12.x, I think? won't 2.12.13 users be affected as well?

@SethTisue
Copy link
Member

Lukas in team meeting: "We should backport it" to 2.12

@SethTisue SethTisue merged commit eb8cb18 into scala:2.13.x Feb 18, 2021
@SethTisue
Copy link
Member

@lrytz I thought I'd take 1 minute to submit a backport but it isn't 1-minute job, the method you modified doesn't exist in 2.12. do you want to backport it, or shall I look into it next week?

@lrytz
Copy link
Member Author

lrytz commented Feb 18, 2021

def isStatic = symbol isNonBottomSubClass StaticAnnotationClass
would be the equivalent spot

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
4 participants