-
Notifications
You must be signed in to change notification settings - Fork 3.1k
SI-3236 constant types for literal final static java fields #5487
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
Conversation
97c5cc0
to
6a90bd3
Compare
Since we don't parse Java expressions, fields of Java classes coming from source files never have constant types. This prevents using static java fields in annotation arguments in mixed compilation This PR assigns constant types to final static java fields if the initializer is a simple literal.
As mentioned in the original PR, this could be expanded to support (some) composite expressions, but it would be a bigger effort. For that, instead of trying to constant fold and assign types in the parser, I think we should just parse the expression and pass it on to the Scala type checker. But that's also not easy by any means :) |
I think the code here is an OK compromise, even with its limitations. For example, it treats |
@jrudolph please take a look. |
On first glance, I think it looks good 👍 Would it make sense to add tests for the additional feature of implicitly converting the constants? |
yes, also some neg tests to make sure the limits are checked. will do. |
One last test failure: test/files/run/t3236 doesn't compile. |
For example, public static final byte b = 127 is allowed, but 128 is not. Also factor out a method that parses a literal. It could be used to parse annotations (and their literal arguments) in Java sources.
Added a neg test to frame the limitations |
review by @adriaanm |
👍 new tests LGTM |
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.
LGTM
Thanks! |
No description provided.