Skip to content

Add support for repeatable annotations #10751

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
Mar 18, 2021
Merged

Add support for repeatable annotations #10751

merged 3 commits into from
Mar 18, 2021

Conversation

Kordyjan
Copy link
Contributor

Fixes #5092

Before this PR, using multiple annotations of the same type on any symbol would generate incorrect bytecode; this can lead to runtime exceptions when reflection is used.
This PR makes the compiler check if they are multiple annotations of the same type and raise a compile error or aggregate them if their type supports it.

@Kordyjan Kordyjan marked this pull request as draft December 11, 2020 08:53
@He-Pin
Copy link
Contributor

He-Pin commented Dec 11, 2020

refs: scala/scala#6846

@Kordyjan Kordyjan marked this pull request as ready for review December 11, 2020 10:20
@Kordyjan Kordyjan marked this pull request as draft December 11, 2020 10:25
@Kordyjan Kordyjan marked this pull request as ready for review December 11, 2020 12:53
@Kordyjan Kordyjan requested a review from sjrd January 13, 2021 21:30
@smarter
Copy link
Member

smarter commented Mar 15, 2021

@sjrd, will you be able to review this?

@sjrd
Copy link
Member

sjrd commented Mar 15, 2021

Hum, probably not. I didn't notice I had been assigned, and I'm not sure why either: it's not at all my area of expertise.

@smarter
Copy link
Member

smarter commented Mar 15, 2021

@liufengyun would you be available to review this?

@liufengyun
Copy link
Contributor

@liufengyun would you be available to review this?

Yes, I can have a look.

@smarter smarter assigned liufengyun and unassigned sjrd Mar 15, 2021
@smarter smarter requested review from liufengyun and removed request for sjrd March 15, 2021 11:58
Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

Otherwise, LGTM 👍

private def transformDef(tree: DefTree)(using Context) =
val annotations = tree.symbol.annotations
if (!annotations.isEmpty) then
tree.symbol.annotations = aggregateAnnotations(tree.symbol.annotations)
Copy link
Contributor

@liufengyun liufengyun Mar 16, 2021

Choose a reason for hiding this comment

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

Usually, we use SymTransformer to change symbol denotations. I think here it's fine to do so as the change is not supposed to be visible in a separately compiled module.

@liufengyun liufengyun assigned Kordyjan and unassigned liufengyun Mar 16, 2021
@Kordyjan Kordyjan enabled auto-merge March 16, 2021 11:04
@Kordyjan Kordyjan merged commit e60ef35 into scala:master Mar 18, 2021
@Kordyjan Kordyjan added this to the 3.0.0 milestone Aug 2, 2023
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.

Support @Repeatable Java annotations
5 participants