-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Preserve singleton types for final val aliases to Java enum constants #24980
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
base: main
Are you sure you want to change the base?
Conversation
|
I mentioned on the ticket that they can't be used in annotations. I tried one approach that was too clever, rewriting them as Edit: I guess the singleton type will be visible in the back end, at least, which is currently widened! |
@som-snytt now they can! Look at 7fee34d !:> Used exactly your insight - the singleton type is visible in backend. Should I mark this as fixing #21881? |
|
@zielinsky that's amazing, thanks. I took a look yesterday and didn't formulate it correctly. Yes, I think this is sufficient for the use case in #21881. Edit: I just deleted my local branch |
| else | ||
| val isJavaEnumValue = tp match | ||
| case tp: TermRef => tp.termSymbol.isAllOf(JavaDefined | Enum) | ||
| case _ => false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically, in this form the val is too eager, albeit not expensive. Currently, I like inline def for local helpers. (It would be nice if it were inlined without asking.)
|
FSR I get the complement in the warn test. I was about to inspect the back end fix. Clean build (?) has similar warning in Edit: the difference is bootstrapped. |
| av.visitEnum(name, edesc, evalue) | ||
| // Handle final val aliases to Java enum values. | ||
| // Check if the symbol's pre-erasure type was a singleton of a Java enum value. | ||
| case t: tpd.RefTree if atPhase(erasurePhase) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if there is a reason to prefer atPhaseBeforeTransforms or conversely to avoid it.
I see to my chagrin that braceless colon syntax does not work, atPhaseBeforeTransforms:.
som-snytt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Better you than me!"
Fixes #24750
Fixes #21881