From 63812c18b23de60039fd3267e4806449ea679972 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Fri, 19 Jun 2015 20:28:40 +0200 Subject: 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). --- .../tools/nsc/backend/jvm/BCodeAsmCommon.scala | 52 ++++++++++++++++++++++ .../scala/tools/nsc/backend/jvm/BTypes.scala | 13 +----- .../tools/nsc/backend/jvm/BTypesFromSymbols.scala | 29 ------------ .../scala/tools/nsc/backend/jvm/GenASM.scala | 8 +--- .../scala/tools/nsc/javac/JavaParsers.scala | 24 ++++++++-- .../nsc/symtab/classfile/ClassfileParser.scala | 2 + .../reflect/internal/ClassfileConstants.scala | 13 +++--- 7 files changed, 85 insertions(+), 56 deletions(-) (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala index eadc404bee..dec5adc9aa 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala @@ -9,6 +9,7 @@ package backend.jvm import scala.tools.nsc.Global import scala.tools.nsc.backend.jvm.BTypes.{InternalName, MethodInlineInfo, InlineInfo} import BackendReporting.ClassSymbolInfoFailureSI9111 +import scala.tools.asm /** * 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) { sym.isPackageClass || sym.isModuleClass && isOriginallyStaticOwner(sym.originalOwner) } + /** + * Reconstruct the classfile flags from a Java defined class symbol. + * + * The implementation of this method is slightly different that `javaFlags` in BTypesFromSymbols. + * The javaFlags method is primarily used to map Scala symbol flags to sensible classfile flags + * that are used in the generated classfiles. For example, all classes emitted by the Scala + * compiler have ACC_PUBLIC. + * + * When building a [[ClassBType]] from a Java class symbol, the flags in the type's `info` have + * to correspond exactly to the flags in the classfile. For example, if the class is package + * protected (i.e., it doesn't have the ACC_PUBLIC flag), this needs to be reflected in the + * ClassBType. For example, the inliner needs the correct flags for access checks. + * + * Class flags are listed here: + * https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.1-200-E.1 + */ + def javaClassfileFlags(classSym: Symbol): Int = { + assert(classSym.isJava, s"Expected Java class symbol, got ${classSym.fullName}") + import asm.Opcodes._ + def enumFlags = ACC_ENUM | { + // Java enums have the `ACC_ABSTRACT` flag if they have a deferred method. + // We cannot trust `hasAbstractFlag`: the ClassfileParser adds `ABSTRACT` and `SEALED` to all + // Java enums for exhaustiveness checking. + val hasAbstractMethod = classSym.info.decls.exists(s => s.isMethod && s.isDeferred) + if (hasAbstractMethod) ACC_ABSTRACT else 0 + } + GenBCode.mkFlags( + if (classSym.isPublic) ACC_PUBLIC else 0, + if (classSym.isFinal) ACC_FINAL else 0, + // see the link above. javac does the same: ACC_SUPER for all classes, but not interfaces. + if (classSym.isInterface) ACC_INTERFACE else ACC_SUPER, + // for Java enums, we cannot trust `hasAbstractFlag` (see comment in enumFlags) + if (!classSym.hasEnumFlag && classSym.hasAbstractFlag) ACC_ABSTRACT else 0, + if (classSym.isArtifact) ACC_SYNTHETIC else 0, + if (classSym.hasEnumFlag) enumFlags else 0 + ) + } + /** * The member classes of a class symbol. Note that the result of this method depends on the * 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) { InlineInfo(traitSelfType, isEffectivelyFinal, methodInlineInfos, warning) } } + +object BCodeAsmCommon { + /** + * Valid flags for InnerClass attribute entry. + * See http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.6 + */ + val INNER_CLASSES_FLAGS = { + asm.Opcodes.ACC_PUBLIC | asm.Opcodes.ACC_PRIVATE | asm.Opcodes.ACC_PROTECTED | + asm.Opcodes.ACC_STATIC | asm.Opcodes.ACC_FINAL | asm.Opcodes.ACC_INTERFACE | + asm.Opcodes.ACC_ABSTRACT | asm.Opcodes.ACC_SYNTHETIC | asm.Opcodes.ACC_ANNOTATION | + asm.Opcodes.ACC_ENUM + } +} diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala index e61190bf3a..176292669c 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala @@ -898,7 +898,7 @@ abstract class BTypes { // the static flag in the InnerClass table has a special meaning, see InnerClass comment i.flags & ~Opcodes.ACC_STATIC, if (isStaticNestedClass) Opcodes.ACC_STATIC else 0 - ) & ClassBType.INNER_CLASSES_FLAGS + ) & BCodeAsmCommon.INNER_CLASSES_FLAGS ) }) @@ -987,17 +987,6 @@ abstract class BTypes { } object ClassBType { - /** - * Valid flags for InnerClass attribute entry. - * See http://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.6 - */ - private val INNER_CLASSES_FLAGS = { - asm.Opcodes.ACC_PUBLIC | asm.Opcodes.ACC_PRIVATE | asm.Opcodes.ACC_PROTECTED | - asm.Opcodes.ACC_STATIC | asm.Opcodes.ACC_FINAL | asm.Opcodes.ACC_INTERFACE | - asm.Opcodes.ACC_ABSTRACT | asm.Opcodes.ACC_SYNTHETIC | asm.Opcodes.ACC_ANNOTATION | - asm.Opcodes.ACC_ENUM - } - // Primitive classes have no super class. A ClassBType for those is only created when // they are actually being compiled (e.g., when compiling scala/Boolean.scala). private val hasNoSuper = Set( diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala index fffb9286b8..93734d3935 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala @@ -213,35 +213,6 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes { assert(!primitiveTypeMap.contains(sym) || isCompilingPrimitive, sym) } - /** - * Reconstruct the classfile flags from a Java defined class symbol. - * - * The implementation of this method is slightly different that [[javaFlags]]. The javaFlags - * method is primarily used to map Scala symbol flags to sensible classfile flags that are used - * in the generated classfiles. For example, all classes emitted by the Scala compiler have - * ACC_PUBLIC. - * - * When building a [[ClassBType]] from a Java class symbol, the flags in the type's `info` have - * to correspond exactly to the flags in the classfile. For example, if the class is package - * protected (i.e., it doesn't have the ACC_PUBLIC flag), this needs to be reflected in the - * ClassBType. For example, the inliner needs the correct flags for access checks. - * - * Class flags are listed here: - * https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.1-200-E.1 - */ - private def javaClassfileFlags(classSym: Symbol): Int = { - assert(classSym.isJava, s"Expected Java class symbol, got ${classSym.fullName}") - import asm.Opcodes._ - GenBCode.mkFlags( - if (classSym.isPublic) ACC_PUBLIC else 0, - if (classSym.isFinal) ACC_FINAL else 0, - if (classSym.isInterface) ACC_INTERFACE else ACC_SUPER, // see the link above. javac does the same: ACC_SUPER for all classes, but not interfaces. - if (classSym.hasAbstractFlag) ACC_ABSTRACT else 0, - if (classSym.isArtifact) ACC_SYNTHETIC else 0, - if (classSym.hasEnumFlag) ACC_ENUM else 0 - ) - } - private def setClassInfo(classSym: Symbol, classBType: ClassBType): ClassBType = { val superClassSym = if (classSym.isImplClass) ObjectClass else classSym.superClass assert( diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala index 76af40b330..71686fd9d7 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala @@ -479,10 +479,6 @@ abstract class GenASM extends SubComponent with BytecodeWriters { self => val CLASS_CONSTRUCTOR_NAME = "" val INSTANCE_CONSTRUCTOR_NAME = "" - val INNER_CLASSES_FLAGS = - (asm.Opcodes.ACC_PUBLIC | asm.Opcodes.ACC_PRIVATE | asm.Opcodes.ACC_PROTECTED | - asm.Opcodes.ACC_STATIC | asm.Opcodes.ACC_INTERFACE | asm.Opcodes.ACC_ABSTRACT | asm.Opcodes.ACC_FINAL) - // ----------------------------------------------------------------------------------------- // factory methods // ----------------------------------------------------------------------------------------- @@ -756,9 +752,9 @@ abstract class GenASM extends SubComponent with BytecodeWriters { self => val flagsWithFinal: Int = mkFlags( // See comment in BTypes, when is a class marked static in the InnerClass table. if (isOriginallyStaticOwner(innerSym.originalOwner)) asm.Opcodes.ACC_STATIC else 0, - javaFlags(innerSym), + (if (innerSym.isJava) javaClassfileFlags(innerSym) else javaFlags(innerSym)) & ~asm.Opcodes.ACC_STATIC, if(isDeprecated(innerSym)) asm.Opcodes.ACC_DEPRECATED else 0 // ASM pseudo-access flag - ) & (INNER_CLASSES_FLAGS | asm.Opcodes.ACC_DEPRECATED) + ) & (BCodeAsmCommon.INNER_CLASSES_FLAGS | asm.Opcodes.ACC_DEPRECATED) val flags = if (innerSym.isModuleClass) flagsWithFinal & ~asm.Opcodes.ACC_FINAL else flagsWithFinal // For SI-5676, object overriding. val jname = javaName(innerSym) // never null val oname = outerName(innerSym) // null when method-enclosed diff --git a/src/compiler/scala/tools/nsc/javac/JavaParsers.scala b/src/compiler/scala/tools/nsc/javac/JavaParsers.scala index d34c14be0f..9708cba281 100644 --- a/src/compiler/scala/tools/nsc/javac/JavaParsers.scala +++ b/src/compiler/scala/tools/nsc/javac/JavaParsers.scala @@ -761,9 +761,13 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners { val interfaces = interfacesOpt() accept(LBRACE) val buf = new ListBuffer[Tree] + var enumIsFinal = true def parseEnumConsts() { if (in.token != RBRACE && in.token != SEMI && in.token != EOF) { - buf += enumConst(enumType) + val (const, hasClassBody) = enumConst(enumType) + buf += const + // if any of the enum constants has a class body, the enum class is not final (JLS 8.9.) + enumIsFinal &&= !hasClassBody if (in.token == COMMA) { in.nextToken() parseEnumConsts() @@ -793,15 +797,25 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners { accept(RBRACE) val superclazz = AppliedTypeTree(javaLangDot(tpnme.Enum), List(enumType)) + val finalFlag = if (enumIsFinal) Flags.FINAL else 0l + val abstractFlag = { + // javac adds `ACC_ABSTRACT` to enum classes with deferred members + val hasAbstractMember = body exists { + case d: DefDef => d.mods.isDeferred + case _ => false + } + if (hasAbstractMember) Flags.ABSTRACT else 0l + } addCompanionObject(consts ::: statics ::: predefs, atPos(pos) { - ClassDef(mods | Flags.ENUM, name, List(), + ClassDef(mods | Flags.ENUM | finalFlag | abstractFlag, name, List(), makeTemplate(superclazz :: interfaces, body)) }) } - def enumConst(enumType: Tree) = { + def enumConst(enumType: Tree): (ValDef, Boolean) = { annotations() - atPos(in.currentPos) { + var hasClassBody = false + val res = atPos(in.currentPos) { val name = ident() if (in.token == LPAREN) { // skip arguments @@ -809,12 +823,14 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners { accept(RPAREN) } if (in.token == LBRACE) { + hasClassBody = true // skip classbody skipAhead() accept(RBRACE) } ValDef(Modifiers(Flags.ENUM | Flags.STABLE | Flags.JAVA | Flags.STATIC), name.toTermName, enumType, blankExpr) } + (res, hasClassBody) } def typeDecl(mods: Modifiers): List[Tree] = in.token match { diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala index 518a402230..660028eab8 100644 --- a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala +++ b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala @@ -539,6 +539,8 @@ abstract class ClassfileParser { devWarning(s"no linked class for java enum $sym in ${sym.owner}. A referencing class file might be missing an InnerClasses entry.") case linked => if (!linked.isSealed) + // Marking the enum class SEALED | ABSTRACT enables exhaustiveness checking. + // This is a bit of a hack and requires excluding the ABSTRACT flag in the backend, see method javaClassfileFlags. linked setFlag (SEALED | ABSTRACT) linked addChild sym } diff --git a/src/reflect/scala/reflect/internal/ClassfileConstants.scala b/src/reflect/scala/reflect/internal/ClassfileConstants.scala index e0a6757d34..53241fb15b 100644 --- a/src/reflect/scala/reflect/internal/ClassfileConstants.scala +++ b/src/reflect/scala/reflect/internal/ClassfileConstants.scala @@ -344,10 +344,12 @@ object ClassfileConstants { case JAVA_ACC_STATIC => STATIC case JAVA_ACC_ABSTRACT => if (isAnnotation) 0L else if (isClass) ABSTRACT else DEFERRED case JAVA_ACC_INTERFACE => if (isAnnotation) 0L else TRAIT | INTERFACE | ABSTRACT + case JAVA_ACC_ENUM => ENUM case _ => 0L } - private def translateFlags(jflags: Int, baseFlags: Long, isAnnotation: Boolean, isClass: Boolean): Long = { - def translateFlag0(jflags: Int): Long = translateFlag(jflags, isAnnotation, isClass) + private def translateFlags(jflags: Int, baseFlags: Long, isClass: Boolean): Long = { + val isAnnot = isAnnotation(jflags) + def translateFlag0(jflags: Int): Long = translateFlag(jflags, isAnnot, isClass) var res: Long = JAVA | baseFlags /* fast, elegant, maintainable, pick any two... */ res |= translateFlag0(jflags & JAVA_ACC_PRIVATE) @@ -357,17 +359,18 @@ object ClassfileConstants { res |= translateFlag0(jflags & JAVA_ACC_STATIC) res |= translateFlag0(jflags & JAVA_ACC_ABSTRACT) res |= translateFlag0(jflags & JAVA_ACC_INTERFACE) + res |= translateFlag0(jflags & JAVA_ACC_ENUM) res } def classFlags(jflags: Int): Long = { - translateFlags(jflags, 0, isAnnotation(jflags), isClass = true) + translateFlags(jflags, 0, isClass = true) } def fieldFlags(jflags: Int): Long = { - translateFlags(jflags, if ((jflags & JAVA_ACC_FINAL) == 0) MUTABLE else 0 , isAnnotation(jflags), isClass = false) + translateFlags(jflags, if ((jflags & JAVA_ACC_FINAL) == 0) MUTABLE else 0 , isClass = false) } def methodFlags(jflags: Int): Long = { - translateFlags(jflags, if ((jflags & JAVA_ACC_BRIDGE) != 0) BRIDGE | ARTIFACT else 0, isAnnotation(jflags), isClass = false) + translateFlags(jflags, if ((jflags & JAVA_ACC_BRIDGE) != 0) BRIDGE | ARTIFACT else 0, isClass = false) } } object FlagTranslation extends FlagTranslation { } -- cgit v1.2.3