Fix #7991: don't set JavaDefined for Dotty Enum module class #8008
Conversation
Top-level Dotty Enum classes have the flag JAVA_ACC_ENUM. We cannot tell from the flag whether a class is JavaDefined or not. The `moduleRoot` already has the flag `JavaDefined` set, it suffices to test the flag.
|
#7501 lists other NoSuchFieldError issues which should be fixed by this |
compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala
Outdated
Show resolved
Hide resolved
|
Ah, I see you already added tests for them :). |
|
for #7410 can we also test a statement in the REPL accessing an enum constant from the class path |
|
We should also have a variant of |
Moving definitions will make Scala code crash at runtime. See `tests/run/i6664b` for an example.
Currently the only way to put initializer inside the static constructor is to use @static. However, CheckStatic restricts that @static can only used in companion object, which is a user-facing check, as MoveStatics can handle @static member in the class. We need a refactoring to make @static a general mechanism that includes a phase to generate the static constructor. This needs to be addressed in another PR.
| new CompleteJavaEnums) :: // Fill in constructors for Java enums | ||
| List(new CheckStatic, // Check restrictions that apply to @static members | ||
| new CheckStatic) :: // Check restrictions that apply to @static members | ||
| List(new CompleteJavaEnums, // Fill in constructors for Java enums |
There was a problem hiding this comment.
Why is this swap needed? Since we are adding the @static annotation in CompleteJavaEnums, should this phase not go before CheckStatic so that the latter can check the newly added static members?
There was a problem hiding this comment.
CheckStatic will reject our code, because it requires that a @static member is located in a companion object, not the class.
There was a problem hiding this comment.
Would it be a better idea to modify CheckStatic to accept the code? E.g. (from CheckStatic doc), "1. Only objects and those classes which extend java.lang.Enum can have members annotated with @static"?
The meaning of CheckStatic is to provide guarantees on @static. If there exists code that doesn't follow the existing guarantees, this complicates the codebase maintenance and usage. We have the exact same scenario with setting flags on completion: there are rules about which flags are immutable wrt completion but they are not enforced. And hence there are lots of places in the codebase that don't follow these rules.
There was a problem hiding this comment.
I have a commit message in f6f3b28: the @static infrastructure needs fundamental refactoring, but it's out of the scope of this PR.
Fix #7991: don't set JavaDefined for Dotty Enum module class
Top-level Dotty Enum classes have the flag JAVA_ACC_ENUM.
We cannot tell from the flag whether a class is JavaDefined or not.
The
moduleRootalready has the flagJavaDefinedset, it sufficesto test the flag.