-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #2760: Support defaults in Java annotations parsed from sources #3865
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
56ec914
to
d00f754
Compare
@@ -0,0 +1 @@ | |||
@Fork(value = 16) class Test |
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.
How do we know we get the right default in the backend? Can we make this a run test?
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.
Yes, done :).
d00f754
to
fb316ee
Compare
Hmm, the run test revealed another issue: Exception in thread "main" java.lang.NoSuchFieldError: value
at Test$.main(Test.scala:6)
at Test.main(Test.scala) Calling abstract class Fork(val value: Int = _root_.scala.Predef.???,
val warmups: Int = _root_.scala.Predef.???
) extends _root_.scala.annotation.Annotation with _root_.java.lang.annotation.
Annotation
with _root_.scala.annotation.ClassfileAnnotation {} and the |
It's defined in scala.annotation.internal, not scala.runtime
This commit is enough to get the added testcase to compile but not to run (it crashes with a NoSuchFieldError), this is fixed in the next commit.
Previously, for tests/run/i2760/Fork.java, the parser output was: abstract class Fork(val value: Int = ???, val warmups: Int = ???) extends ... Since Fork is JavaDefined, calls to `value` were translated into field calls, but `value` is a method, not a field, so i2760 crashed at runtime with NoSuchFieldError. We now generate instead: abstract class Fork private[this](x$1: _root_.scala.Unit) extends ... { def <init>(value: Int = ???, warmups: Int = ???) def value(): Int = 1 def warmups(): Int = 1 } This way we get both named parameters for the constructor and real method calls.
fb316ee
to
a1a1a4b
Compare
Fixed by tweaking the parser output, see last commit. |
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
No description provided.