Skip to content

Add the right constructor to Java annotations #184

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

Closed
wants to merge 1 commit into from

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Oct 10, 2014

Review by @olhotak please.

@DarkDimius DarkDimius force-pushed the fix/java-annotations branch from 8a6c719 to ec486ff Compare October 11, 2014 10:12
@DarkDimius
Copy link
Contributor

@odersky error that is seen for annotaions by @olhotak is not specific to annotations.
By enabling transformation of trees inside annotations I'm still able to observe problem with
elimRepeated.

I've added a testcase that shows it.

@DarkDimius DarkDimius force-pushed the fix/java-annotations branch from 886bc90 to 1b79def Compare October 11, 2014 10:26
@odersky
Copy link
Contributor Author

odersky commented Oct 11, 2014

My patch was not intended to fix elimRepeated. The error it addresses is much more basic than that. The patch allows annotations with arguments and allos default arguments to be elided.

@odersky
Copy link
Contributor Author

odersky commented Oct 26, 2014

What should we do with this? I think Ondrej will need the original commit. The question of transforming annotations is a separate one.

@DarkDimius
Copy link
Contributor

I agree that the question of transforming annotations is a separate one.
With annotations I believe we need a better definition what they are and what they mean. If annotations are simple constructor calls with constant arguments than the only face that seems to be concerned with transforming annotations is elimRepeated and 2 commits in this branch will allow it to work.

If trees can be more complicated than I seems reasonable to have the tree inside the annotation be transformed on the definition side only with a definition context and this should be propagated(eg by propagating not of the annotation itself but the indirection reference to original annotated symbol).

@olhotak olhotak mentioned this pull request Oct 27, 2014
@odersky odersky force-pushed the fix/java-annotations branch from 531bb40 to 8a6c719 Compare November 18, 2014 17:08
@DarkDimius DarkDimius closed this Dec 3, 2014
WojciechMazur pushed a commit to WojciechMazur/dotty that referenced this pull request Mar 19, 2025
Backport "Fix scala#21841: Check more that an `unapplySeq` on a `NonEmptyTuple` is valid." to 3.3 LTS
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.

3 participants