Skip to content

Commit 63812c1

Browse files
lrytzlrytz
committed
SI-9359 Fix InnerClass entry flags for nested Java enums
The access flags in InnerClass entries for nested Java enums were basically completely off. A first step is to use the recently introduced backend method `javaClassfileFlags`, which is now moved to BCodeAsmCommon. See its doc for an explanation. Then the flags of the enum class symbol were off. An enum is - final if none of its values has a class body - abstract if it has an abstract method (https://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.9) When using the ClassfileParser: - ENUM was never added. I guess that's just an oversight. - ABSTRACT (together with SEALED) was always added. This is to enable exhaustiveness checking, see 3f7b8b5. This is a hack and we have to go through the class members in the backend to find out if the enum actually has the `ACC_ABSTRACT` flag or not. When using the JavaParser: - FINAL was never added. - ABSTRACT was never added. This commit fixes all of the above and tests cases (Java enum read from the classfile and from source).
1 parent 305dd96 commit 63812c1

File tree

11 files changed

+169
-56
lines changed

11 files changed

+169
-56
lines changed

‎src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ package backend.jvm
99
import scala.tools.nsc.Global
1010
import scala.tools.nsc.backend.jvm.BTypes.{InternalName, MethodInlineInfo, InlineInfo}
1111
import BackendReporting.ClassSymbolInfoFailureSI9111
12+
import scala.tools.asm
1213

1314
/**
1415
* This trait contains code shared between GenBCode and GenASM that depends on types defined in
@@ -228,6 +229,44 @@ final class BCodeAsmCommon[G <: Global](val global: G) {
228229
sym.isPackageClass || sym.isModuleClass && isOriginallyStaticOwner(sym.originalOwner)
229230
}
230231

232+
/**
233+
* Reconstruct the classfile flags from a Java defined class symbol.
234+
*
235+
* The implementation of this method is slightly different that `javaFlags` in BTypesFromSymbols.
236+
* The javaFlags method is primarily used to map Scala symbol flags to sensible classfile flags
237+
* that are used in the generated classfiles. For example, all classes emitted by the Scala
238+
* compiler have ACC_PUBLIC.
239+
*
240+
* When building a [[ClassBType]] from a Java class symbol, the flags in the type's `info` have
241+
* to correspond exactly to the flags in the classfile. For example, if the class is package
242+
* protected (i.e., it doesn't have the ACC_PUBLIC flag), this needs to be reflected in the
243+
* ClassBType. For example, the inliner needs the correct flags for access checks.
244+
*
245+
* Class flags are listed here:
246+
* https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.1-200-E.1
247+
*/
248+
def javaClassfileFlags(classSym: Symbol): Int = {
249+
assert(classSym.isJava, s"Expected Java class symbol, got ${classSym.fullName}")
250+
import asm.Opcodes._
251+
def enumFlags = ACC_ENUM | {
252+
// Java enums have the `ACC_ABSTRACT` flag if they have a deferred method.
253+
// We cannot trust `hasAbstractFlag`: the ClassfileParser adds `ABSTRACT` and `SEALED` to all
254+
// Java enums for exhaustiveness checking.
255+
val hasAbstractMethod = classSym.info.decls.exists(s => s.isMethod && s.isDeferred)
256+
if (hasAbstractMethod) ACC_ABSTRACT else 0
257+
}
258+
GenBCode.mkFlags(
259+
if (classSym.isPublic) ACC_PUBLIC else 0,
260+
if (classSym.isFinal) ACC_FINAL else 0,
261+
// see the link above. javac does the same: ACC_SUPER for all classes, but not interfaces.
262+
if (classSym.isInterface) ACC_INTERFACE else ACC_SUPER,
263+
// for Java enums, we cannot trust `hasAbstractFlag` (see comment in enumFlags)
264+
if (!classSym.hasEnumFlag && classSym.hasAbstractFlag) ACC_ABSTRACT else 0,
265+
if (classSym.isArtifact) ACC_SYNTHETIC else 0,
266+
if (classSym.hasEnumFlag) enumFlags else 0
267+
)
268+
}
269+
231270
/**
232271
* The member classes of a class symbol. Note that the result of this method depends on the
233272
* current phase, for example, after lambdalift, all local classes become member of the enclosing
@@ -399,3 +438,16 @@ final class BCodeAsmCommon[G <: Global](val global: G) {
399438
InlineInfo(traitSelfType, isEffectivelyFinal, methodInlineInfos, warning)
400439
}
401440
}
441+
442+
object BCodeAsmCommon {
443+
/**
444+
* Valid flags for InnerClass attribute entry.
445+
* See http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.6
446+
*/
447+
val INNER_CLASSES_FLAGS = {
448+
asm.Opcodes.ACC_PUBLIC | asm.Opcodes.ACC_PRIVATE | asm.Opcodes.ACC_PROTECTED |
449+
asm.Opcodes.ACC_STATIC | asm.Opcodes.ACC_FINAL | asm.Opcodes.ACC_INTERFACE |
450+
asm.Opcodes.ACC_ABSTRACT | asm.Opcodes.ACC_SYNTHETIC | asm.Opcodes.ACC_ANNOTATION |
451+
asm.Opcodes.ACC_ENUM
452+
}
453+
}

‎src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -898,7 +898,7 @@ abstract class BTypes {
898898
// the static flag in the InnerClass table has a special meaning, see InnerClass comment
899899
i.flags & ~Opcodes.ACC_STATIC,
900900
if (isStaticNestedClass) Opcodes.ACC_STATIC else 0
901-
) & ClassBType.INNER_CLASSES_FLAGS
901+
) & BCodeAsmCommon.INNER_CLASSES_FLAGS
902902
)
903903
})
904904

@@ -987,17 +987,6 @@ abstract class BTypes {
987987
}
988988

989989
object ClassBType {
990-
/**
991-
* Valid flags for InnerClass attribute entry.
992-
* See http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.6
993-
*/
994-
private val INNER_CLASSES_FLAGS = {
995-
asm.Opcodes.ACC_PUBLIC | asm.Opcodes.ACC_PRIVATE | asm.Opcodes.ACC_PROTECTED |
996-
asm.Opcodes.ACC_STATIC | asm.Opcodes.ACC_FINAL | asm.Opcodes.ACC_INTERFACE |
997-
asm.Opcodes.ACC_ABSTRACT | asm.Opcodes.ACC_SYNTHETIC | asm.Opcodes.ACC_ANNOTATION |
998-
asm.Opcodes.ACC_ENUM
999-
}
1000-
1001990
// Primitive classes have no super class. A ClassBType for those is only created when
1002991
// they are actually being compiled (e.g., when compiling scala/Boolean.scala).
1003992
private val hasNoSuper = Set(

‎src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -213,35 +213,6 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
213213
assert(!primitiveTypeMap.contains(sym) || isCompilingPrimitive, sym)
214214
}
215215

216-
/**
217-
* Reconstruct the classfile flags from a Java defined class symbol.
218-
*
219-
* The implementation of this method is slightly different that [[javaFlags]]. The javaFlags
220-
* method is primarily used to map Scala symbol flags to sensible classfile flags that are used
221-
* in the generated classfiles. For example, all classes emitted by the Scala compiler have
222-
* ACC_PUBLIC.
223-
*
224-
* When building a [[ClassBType]] from a Java class symbol, the flags in the type's `info` have
225-
* to correspond exactly to the flags in the classfile. For example, if the class is package
226-
* protected (i.e., it doesn't have the ACC_PUBLIC flag), this needs to be reflected in the
227-
* ClassBType. For example, the inliner needs the correct flags for access checks.
228-
*
229-
* Class flags are listed here:
230-
* https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.1-200-E.1
231-
*/
232-
private def javaClassfileFlags(classSym: Symbol): Int = {
233-
assert(classSym.isJava, s"Expected Java class symbol, got ${classSym.fullName}")
234-
import asm.Opcodes._
235-
GenBCode.mkFlags(
236-
if (classSym.isPublic) ACC_PUBLIC else 0,
237-
if (classSym.isFinal) ACC_FINAL else 0,
238-
if (classSym.isInterface) ACC_INTERFACE else ACC_SUPER, // see the link above. javac does the same: ACC_SUPER for all classes, but not interfaces.
239-
if (classSym.hasAbstractFlag) ACC_ABSTRACT else 0,
240-
if (classSym.isArtifact) ACC_SYNTHETIC else 0,
241-
if (classSym.hasEnumFlag) ACC_ENUM else 0
242-
)
243-
}
244-
245216
private def setClassInfo(classSym: Symbol, classBType: ClassBType): ClassBType = {
246217
val superClassSym = if (classSym.isImplClass) ObjectClass else classSym.superClass
247218
assert(

‎src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -479,10 +479,6 @@ abstract class GenASM extends SubComponent with BytecodeWriters { self =>
479479
val CLASS_CONSTRUCTOR_NAME = "<clinit>"
480480
val INSTANCE_CONSTRUCTOR_NAME = "<init>"
481481

482-
val INNER_CLASSES_FLAGS =
483-
(asm.Opcodes.ACC_PUBLIC | asm.Opcodes.ACC_PRIVATE | asm.Opcodes.ACC_PROTECTED |
484-
asm.Opcodes.ACC_STATIC | asm.Opcodes.ACC_INTERFACE | asm.Opcodes.ACC_ABSTRACT | asm.Opcodes.ACC_FINAL)
485-
486482
// -----------------------------------------------------------------------------------------
487483
// factory methods
488484
// -----------------------------------------------------------------------------------------
@@ -756,9 +752,9 @@ abstract class GenASM extends SubComponent with BytecodeWriters { self =>
756752
val flagsWithFinal: Int = mkFlags(
757753
// See comment in BTypes, when is a class marked static in the InnerClass table.
758754
if (isOriginallyStaticOwner(innerSym.originalOwner)) asm.Opcodes.ACC_STATIC else 0,
759-
javaFlags(innerSym),
755+
(if (innerSym.isJava) javaClassfileFlags(innerSym) else javaFlags(innerSym)) & ~asm.Opcodes.ACC_STATIC,
760756
if(isDeprecated(innerSym)) asm.Opcodes.ACC_DEPRECATED else 0 // ASM pseudo-access flag
761-
) & (INNER_CLASSES_FLAGS | asm.Opcodes.ACC_DEPRECATED)
757+
) & (BCodeAsmCommon.INNER_CLASSES_FLAGS | asm.Opcodes.ACC_DEPRECATED)
762758
val flags = if (innerSym.isModuleClass) flagsWithFinal & ~asm.Opcodes.ACC_FINAL else flagsWithFinal // For SI-5676, object overriding.
763759
val jname = javaName(innerSym) // never null
764760
val oname = outerName(innerSym) // null when method-enclosed

‎src/compiler/scala/tools/nsc/javac/JavaParsers.scala‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -761,9 +761,13 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
761761
val interfaces = interfacesOpt()
762762
accept(LBRACE)
763763
val buf = new ListBuffer[Tree]
764+
var enumIsFinal = true
764765
def parseEnumConsts() {
765766
if (in.token != RBRACE && in.token != SEMI && in.token != EOF) {
766-
buf += enumConst(enumType)
767+
val (const, hasClassBody) = enumConst(enumType)
768+
buf += const
769+
// if any of the enum constants has a class body, the enum class is not final (JLS 8.9.)
770+
enumIsFinal &&= !hasClassBody
767771
if (in.token == COMMA) {
768772
in.nextToken()
769773
parseEnumConsts()
@@ -793,28 +797,40 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
793797
accept(RBRACE)
794798
val superclazz =
795799
AppliedTypeTree(javaLangDot(tpnme.Enum), List(enumType))
800+
val finalFlag = if (enumIsFinal) Flags.FINAL else 0l
801+
val abstractFlag = {
802+
// javac adds `ACC_ABSTRACT` to enum classes with deferred members
803+
val hasAbstractMember = body exists {
804+
case d: DefDef => d.mods.isDeferred
805+
case _ => false
806+
}
807+
if (hasAbstractMember) Flags.ABSTRACT else 0l
808+
}
796809
addCompanionObject(consts ::: statics ::: predefs, atPos(pos) {
797-
ClassDef(mods | Flags.ENUM, name, List(),
810+
ClassDef(mods | Flags.ENUM | finalFlag | abstractFlag, name, List(),
798811
makeTemplate(superclazz :: interfaces, body))
799812
})
800813
}
801814

802-
def enumConst(enumType: Tree) = {
815+
def enumConst(enumType: Tree): (ValDef, Boolean) = {
803816
annotations()
804-
atPos(in.currentPos) {
817+
var hasClassBody = false
818+
val res = atPos(in.currentPos) {
805819
val name = ident()
806820
if (in.token == LPAREN) {
807821
// skip arguments
808822
skipAhead()
809823
accept(RPAREN)
810824
}
811825
if (in.token == LBRACE) {
826+
hasClassBody = true
812827
// skip classbody
813828
skipAhead()
814829
accept(RBRACE)
815830
}
816831
ValDef(Modifiers(Flags.ENUM | Flags.STABLE | Flags.JAVA | Flags.STATIC), name.toTermName, enumType, blankExpr)
817832
}
833+
(res, hasClassBody)
818834
}
819835

820836
def typeDecl(mods: Modifiers): List[Tree] = in.token match {

‎src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,8 @@ abstract class ClassfileParser {
539539
devWarning(s"no linked class for java enum $sym in ${sym.owner}. A referencing class file might be missing an InnerClasses entry.")
540540
case linked =>
541541
if (!linked.isSealed)
542+
// Marking the enum class SEALED | ABSTRACT enables exhaustiveness checking.
543+
// This is a bit of a hack and requires excluding the ABSTRACT flag in the backend, see method javaClassfileFlags.
542544
linked setFlag (SEALED | ABSTRACT)
543545
linked addChild sym
544546
}

‎src/reflect/scala/reflect/internal/ClassfileConstants.scala‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -344,10 +344,12 @@ object ClassfileConstants {
344344
case JAVA_ACC_STATIC => STATIC
345345
case JAVA_ACC_ABSTRACT => if (isAnnotation) 0L else if (isClass) ABSTRACT else DEFERRED
346346
case JAVA_ACC_INTERFACE => if (isAnnotation) 0L else TRAIT | INTERFACE | ABSTRACT
347+
case JAVA_ACC_ENUM => ENUM
347348
case _ => 0L
348349
}
349-
private def translateFlags(jflags: Int, baseFlags: Long, isAnnotation: Boolean, isClass: Boolean): Long = {
350-
def translateFlag0(jflags: Int): Long = translateFlag(jflags, isAnnotation, isClass)
350+
private def translateFlags(jflags: Int, baseFlags: Long, isClass: Boolean): Long = {
351+
val isAnnot = isAnnotation(jflags)
352+
def translateFlag0(jflags: Int): Long = translateFlag(jflags, isAnnot, isClass)
351353
var res: Long = JAVA | baseFlags
352354
/* fast, elegant, maintainable, pick any two... */
353355
res |= translateFlag0(jflags & JAVA_ACC_PRIVATE)
@@ -357,17 +359,18 @@ object ClassfileConstants {
357359
res |= translateFlag0(jflags & JAVA_ACC_STATIC)
358360
res |= translateFlag0(jflags & JAVA_ACC_ABSTRACT)
359361
res |= translateFlag0(jflags & JAVA_ACC_INTERFACE)
362+
res |= translateFlag0(jflags & JAVA_ACC_ENUM)
360363
res
361364
}
362365

363366
def classFlags(jflags: Int): Long = {
364-
translateFlags(jflags, 0, isAnnotation(jflags), isClass = true)
367+
translateFlags(jflags, 0, isClass = true)
365368
}
366369
def fieldFlags(jflags: Int): Long = {
367-
translateFlags(jflags, if ((jflags & JAVA_ACC_FINAL) == 0) MUTABLE else 0 , isAnnotation(jflags), isClass = false)
370+
translateFlags(jflags, if ((jflags & JAVA_ACC_FINAL) == 0) MUTABLE else 0 , isClass = false)
368371
}
369372
def methodFlags(jflags: Int): Long = {
370-
translateFlags(jflags, if ((jflags & JAVA_ACC_BRIDGE) != 0) BRIDGE | ARTIFACT else 0, isAnnotation(jflags), isClass = false)
373+
translateFlags(jflags, if ((jflags & JAVA_ACC_BRIDGE) != 0) BRIDGE | ARTIFACT else 0, isClass = false)
371374
}
372375
}
373376
object FlagTranslation extends FlagTranslation { }

‎test/files/run/t9359.check‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// access flags 0x4009
2+
public static enum INNERCLASS A_1$A1N A_1 A1N
3+
4+
// access flags 0x4409
5+
public static abstract enum INNERCLASS A_1$A1N_ABSTRACT A_1 A1N_ABSTRACT
6+
7+
// access flags 0x4019
8+
public final static enum INNERCLASS A_1$A1N_FINAL A_1 A1N_FINAL
9+
10+
// access flags 0x4009
11+
public static enum INNERCLASS B_2$A1N B_2 A1N
12+
13+
// access flags 0x4409
14+
public static abstract enum INNERCLASS B_2$A1N_ABSTRACT B_2 A1N_ABSTRACT
15+
16+
// access flags 0x4019
17+
public final static enum INNERCLASS B_2$A1N_FINAL B_2 A1N_FINAL
18+

‎test/files/run/t9359/A_1.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
public class A_1 {
2+
// nested final
3+
public static enum A1N_FINAL {
4+
A1N_FINAL_VAL
5+
}
6+
7+
// nested, non-final
8+
public enum A1N {
9+
A1N_VAL { } // value has a body, so a class extending A1N is generated
10+
}
11+
12+
// nested, non-final, abstract
13+
public enum A1N_ABSTRACT {
14+
A1N_ABSTRACT_VAL {
15+
void foo() { return; }
16+
};
17+
abstract void foo(); // abstract member makes the enum class abstract
18+
}
19+
}

‎test/files/run/t9359/B_2.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
public class B_2 {
2+
// nested final
3+
public enum A1N_FINAL {
4+
A1N_FINAL_VAL
5+
}
6+
7+
// nested, non-final
8+
public enum A1N {
9+
A1N_VAL { } // value has a body, so a class extending A1N is generated
10+
}
11+
12+
// nested, non-final, abstract
13+
public enum A1N_ABSTRACT {
14+
A1N_ABSTRACT_VAL {
15+
void foo() { return; }
16+
};
17+
abstract void foo(); // abstract member makes the enum class abstract
18+
}
19+
}

0 commit comments

Comments
 (0)