Skip to content

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

Merged
SethTisue merged 1 commit intoscala:2.13.xfrom
lrytz:t12336
Feb 18, 2021
Merged

Omit @nowarn annotations from generated code, for forwards compatibility at compile-time#9491
SethTisue merged 1 commit intoscala:2.13.xfrom
lrytz:t12336

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

Development

Successfully merging this pull request may close these issues.

@nowarn breaks forwards compatibility at compile-time

4 participants