Skip to content

Commit aae695e

Browse files
retronymretronym
committed
Merge pull request scala#4638 from lrytz/t9393
SI-9393 fix modifiers of ClassBTypes for Java annotations
2 parents 1a74e38 + 59f1ee5 commit aae695e

File tree

26 files changed

+333
-162
lines changed

26 files changed

+333
-162
lines changed

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

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -256,14 +256,17 @@ final class BCodeAsmCommon[G <: Global](val global: G) {
256256
if (hasAbstractMethod) ACC_ABSTRACT else 0
257257
}
258258
GenBCode.mkFlags(
259-
if (classSym.isPublic) ACC_PUBLIC else 0,
260-
if (classSym.isFinal) ACC_FINAL else 0,
259+
// SI-9393: the classfile / java source parser make java annotation symbols look like classes.
260+
// here we recover the actual classfile flags.
261+
if (classSym.hasJavaAnnotationFlag) ACC_ANNOTATION | ACC_INTERFACE | ACC_ABSTRACT else 0,
262+
if (classSym.isPublic) ACC_PUBLIC else 0,
263+
if (classSym.isFinal) ACC_FINAL else 0,
261264
// 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,
265+
if (classSym.isInterface) ACC_INTERFACE else ACC_SUPER,
263266
// 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+
if (!classSym.hasJavaEnumFlag && classSym.hasAbstractFlag) ACC_ABSTRACT else 0,
268+
if (classSym.isArtifact) ACC_SYNTHETIC else 0,
269+
if (classSym.hasJavaEnumFlag) enumFlags else 0
267270
)
268271
}
269272

@@ -310,10 +313,10 @@ final class BCodeAsmCommon[G <: Global](val global: G) {
310313
}
311314

312315
private def retentionPolicyOf(annot: AnnotationInfo): Symbol =
313-
annot.atp.typeSymbol.getAnnotation(AnnotationRetentionAttr).map(_.assocs).map(assoc =>
316+
annot.atp.typeSymbol.getAnnotation(AnnotationRetentionAttr).map(_.assocs).flatMap(assoc =>
314317
assoc.collectFirst {
315318
case (`nme`.value, LiteralAnnotArg(Constant(value: Symbol))) => value
316-
}).flatten.getOrElse(AnnotationRetentionPolicyClassValue)
319+
}).getOrElse(AnnotationRetentionPolicyClassValue)
317320

318321
def implementedInterfaces(classSym: Symbol): List[Symbol] = {
319322
// Additional interface parents based on annotations and other cues
@@ -322,9 +325,18 @@ final class BCodeAsmCommon[G <: Global](val global: G) {
322325
case _ => None
323326
}
324327

325-
def isInterfaceOrTrait(sym: Symbol) = sym.isInterface || sym.isTrait
328+
// SI-9393: java annotations are interfaces, but the classfile / java source parsers make them look like classes.
329+
def isInterfaceOrTrait(sym: Symbol) = sym.isInterface || sym.isTrait || sym.hasJavaAnnotationFlag
326330

327-
val allParents = classSym.info.parents ++ classSym.annotations.flatMap(newParentForAnnotation)
331+
val classParents = {
332+
val parents = classSym.info.parents
333+
// SI-9393: the classfile / java source parsers add Annotation and ClassfileAnnotation to the
334+
// parents of a java annotations. undo this for the backend (where we need classfile-level information).
335+
if (classSym.hasJavaAnnotationFlag) parents.filterNot(c => c.typeSymbol == ClassfileAnnotationClass || c.typeSymbol == AnnotationClass)
336+
else parents
337+
}
338+
339+
val allParents = classParents ++ classSym.annotations.flatMap(newParentForAnnotation)
328340

329341
// We keep the superClass when computing minimizeParents to eliminate more interfaces.
330342
// Example: T can be eliminated from D

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,9 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
153153
*/
154154
private def initJClass(jclass: asm.ClassVisitor) {
155155

156-
val ps = claszSymbol.info.parents
157-
val superClass: String = if (ps.isEmpty) ObjectReference.internalName else internalName(ps.head.typeSymbol)
158-
val interfaceNames = classBTypeFromSymbol(claszSymbol).info.get.interfaces map {
156+
val bType = classBTypeFromSymbol(claszSymbol)
157+
val superClass = bType.info.get.superClass.getOrElse(ObjectReference).internalName
158+
val interfaceNames = bType.info.get.interfaces map {
159159
case classBType =>
160160
if (classBType.isNestedClass.get) { innerClassBufferASM += classBType }
161161
classBType.internalName

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

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -841,17 +841,16 @@ abstract class BTypes {
841841

842842
assert(!ClassBType.isInternalPhantomType(internalName), s"Cannot create ClassBType for phantom type $this")
843843

844-
// TODO bring these back in a way that doesn't trip pos/t9393
845-
// assert(
846-
// if (info.get.superClass.isEmpty) { isJLO(this) || (isCompilingPrimitive && ClassBType.hasNoSuper(internalName)) }
847-
// else if (isInterface.get) isJLO(info.get.superClass.get)
848-
// else !isJLO(this) && ifInit(info.get.superClass.get)(!_.isInterface.get),
849-
// s"Invalid superClass in $this: ${info.get.superClass}"
850-
// )
851-
// assert(
852-
// info.get.interfaces.forall(c => ifInit(c)(_.isInterface.get)),
853-
// s"Invalid interfaces in $this: ${info.get.interfaces}"
854-
// )
844+
assert(
845+
if (info.get.superClass.isEmpty) { isJLO(this) || (isCompilingPrimitive && ClassBType.hasNoSuper(internalName)) }
846+
else if (isInterface.get) isJLO(info.get.superClass.get)
847+
else !isJLO(this) && ifInit(info.get.superClass.get)(!_.isInterface.get),
848+
s"Invalid superClass in $this: ${info.get.superClass}"
849+
)
850+
assert(
851+
info.get.interfaces.forall(c => ifInit(c)(_.isInterface.get)),
852+
s"Invalid interfaces in $this: ${info.get.interfaces}"
853+
)
855854

856855
assert(info.get.nestedClasses.forall(c => ifInit(c)(_.isNestedClass.get)), info.get.nestedClasses)
857856
}

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,18 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
216216
}
217217

218218
private def setClassInfo(classSym: Symbol, classBType: ClassBType): ClassBType = {
219-
val superClassSym = if (classSym.isImplClass) ObjectClass else classSym.superClass
219+
// Check for isImplClass: trait implementation classes have NoSymbol as superClass
220+
// Check for hasAnnotationFlag for SI-9393: the classfile / java source parsers add
221+
// scala.annotation.Annotation as superclass to java annotations. In reality, java
222+
// annotation classfiles have superclass Object (like any interface classfile).
223+
val superClassSym = if (classSym.isImplClass || classSym.hasJavaAnnotationFlag) ObjectClass else {
224+
val sc = classSym.superClass
225+
// SI-9393: Java annotation classes don't have the ABSTRACT/INTERFACE flag, so they appear
226+
// (wrongly) as superclasses. Fix this for BTypes: the java annotation will appear as interface
227+
// (handled by method implementedInterfaces), the superclass is set to Object.
228+
if (sc.hasJavaAnnotationFlag) ObjectClass
229+
else sc
230+
}
220231
assert(
221232
if (classSym == ObjectClass)
222233
superClassSym == NoSymbol
@@ -567,7 +578,7 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
567578
if (sym.isBridge) ACC_BRIDGE | ACC_SYNTHETIC else 0,
568579
if (sym.isArtifact) ACC_SYNTHETIC else 0,
569580
if (sym.isClass && !sym.isInterface) ACC_SUPER else 0,
570-
if (sym.hasEnumFlag) ACC_ENUM else 0,
581+
if (sym.hasJavaEnumFlag) ACC_ENUM else 0,
571582
if (sym.isVarargsMethod) ACC_VARARGS else 0,
572583
if (sym.hasFlag(symtab.Flags.SYNCHRONIZED)) ACC_SYNCHRONIZED else 0,
573584
if (sym.isDeprecated) asm.Opcodes.ACC_DEPRECATED else 0

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters { self =>
307307
if (sym.isBridge) ACC_BRIDGE | ACC_SYNTHETIC else 0,
308308
if (sym.isArtifact) ACC_SYNTHETIC else 0,
309309
if (sym.isClass && !sym.isInterface) ACC_SUPER else 0,
310-
if (sym.hasEnumFlag) ACC_ENUM else 0,
310+
if (sym.hasJavaEnumFlag) ACC_ENUM else 0,
311311
if (sym.isVarargsMethod) ACC_VARARGS else 0,
312312
if (sym.hasFlag(Flags.SYNCHRONIZED)) ACC_SYNCHRONIZED else 0
313313
)

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,7 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
370370
flags |= Flags.FINAL
371371
in.nextToken()
372372
case DEFAULT =>
373-
flags |= Flags.DEFAULTMETHOD
373+
flags |= Flags.JAVA_DEFAULTMETHOD
374374
in.nextToken()
375375
case NATIVE =>
376376
addAnnot(NativeAttr)
@@ -489,7 +489,7 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
489489
val vparams = formalParams()
490490
if (!isVoid) rtpt = optArrayBrackets(rtpt)
491491
optThrows()
492-
val isConcreteInterfaceMethod = !inInterface || (mods hasFlag Flags.DEFAULTMETHOD) || (mods hasFlag Flags.STATIC)
492+
val isConcreteInterfaceMethod = !inInterface || (mods hasFlag Flags.JAVA_DEFAULTMETHOD) || (mods hasFlag Flags.STATIC)
493493
val bodyOk = !(mods1 hasFlag Flags.DEFERRED) && isConcreteInterfaceMethod
494494
val body =
495495
if (bodyOk && in.token == LBRACE) {
@@ -751,7 +751,7 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
751751
val (statics, body) = typeBody(AT, name)
752752
val templ = makeTemplate(annotationParents, body)
753753
addCompanionObject(statics, atPos(pos) {
754-
ClassDef(mods, name, List(), templ)
754+
ClassDef(mods | Flags.JAVA_ANNOTATION, name, List(), templ)
755755
})
756756
}
757757

@@ -809,7 +809,7 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
809809
if (hasAbstractMember) Flags.ABSTRACT else 0l
810810
}
811811
addCompanionObject(consts ::: statics ::: predefs, atPos(pos) {
812-
ClassDef(mods | Flags.ENUM | finalFlag | abstractFlag, name, List(),
812+
ClassDef(mods | Flags.JAVA_ENUM | finalFlag | abstractFlag, name, List(),
813813
makeTemplate(superclazz :: interfaces, body))
814814
})
815815
}
@@ -830,7 +830,7 @@ trait JavaParsers extends ast.parser.ParsersCommon with JavaScanners {
830830
skipAhead()
831831
accept(RBRACE)
832832
}
833-
ValDef(Modifiers(Flags.ENUM | Flags.STABLE | Flags.JAVA | Flags.STATIC), name.toTermName, enumType, blankExpr)
833+
ValDef(Modifiers(Flags.JAVA_ENUM | Flags.STABLE | Flags.JAVA | Flags.STATIC), name.toTermName, enumType, blankExpr)
834834
}
835835
(res, hasClassBody)
836836
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -862,7 +862,7 @@ abstract class ClassfileParser {
862862
srcfile0 = settings.outputDirs.srcFilesFor(in.file, srcpath).find(_.exists)
863863
case tpnme.CodeATTR =>
864864
if (sym.owner.isInterface) {
865-
sym setFlag DEFAULTMETHOD
865+
sym setFlag JAVA_DEFAULTMETHOD
866866
log(s"$sym in ${sym.owner} is a java8+ default method.")
867867
}
868868
in.skip(attrLen)

‎src/compiler/scala/tools/nsc/typechecker/Namers.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,8 @@ trait Namers extends MethodSynthesis {
145145
// while Scala's enum constants live directly in the class.
146146
// We don't check for clazz.superClass == JavaEnumClass, because this causes a illegal
147147
// cyclic reference error. See the commit message for details.
148-
if (context.unit.isJava) owner.companionClass.hasEnumFlag else owner.hasEnumFlag
149-
vd.mods.hasAllFlags(ENUM | STABLE | STATIC) && ownerHasEnumFlag
148+
if (context.unit.isJava) owner.companionClass.hasJavaEnumFlag else owner.hasJavaEnumFlag
149+
vd.mods.hasAllFlags(JAVA_ENUM | STABLE | STATIC) && ownerHasEnumFlag
150150
}
151151

152152
def setPrivateWithin[T <: Symbol](tree: Tree, sym: T, mods: Modifiers): T =

‎src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
421421
overrideError("cannot be used here - classes can only override abstract types")
422422
} else if (other.isEffectivelyFinal) { // (1.2)
423423
overrideError("cannot override final member")
424-
} else if (!other.isDeferredOrDefault && !other.hasFlag(DEFAULTMETHOD) && !member.isAnyOverride && !member.isSynthetic) { // (*)
424+
} else if (!other.isDeferredOrJavaDefault && !other.hasFlag(JAVA_DEFAULTMETHOD) && !member.isAnyOverride && !member.isSynthetic) { // (*)
425425
// (*) Synthetic exclusion for (at least) default getters, fixes SI-5178. We cannot assign the OVERRIDE flag to
426426
// the default getter: one default getter might sometimes override, sometimes not. Example in comment on ticket.
427427
if (isNeitherInClass && !(other.owner isSubClass member.owner))
@@ -604,7 +604,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
604604
def checkNoAbstractMembers(): Unit = {
605605
// Avoid spurious duplicates: first gather any missing members.
606606
def memberList = clazz.info.nonPrivateMembersAdmitting(VBRIDGE)
607-
val (missing, rest) = memberList partition (m => m.isDeferredNotDefault && !ignoreDeferred(m))
607+
val (missing, rest) = memberList partition (m => m.isDeferredNotJavaDefault && !ignoreDeferred(m))
608608
// Group missing members by the name of the underlying symbol,
609609
// to consolidate getters and setters.
610610
val grouped = missing groupBy (sym => analyzer.underlyingSymbol(sym).name)

‎src/reflect/scala/reflect/api/FlagSets.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ trait FlagSets { self: Universe =>
173173
* - the enum's class
174174
* - enum constants
175175
**/
176+
@deprecated("Use `isJavaEnum` on the corresponding symbol instead.", since = "2.11.8")
176177
val ENUM: FlagSet
177178

178179
/** Flag indicating that tree represents a parameter of the primary constructor of some class

‎src/reflect/scala/reflect/api/Symbols.scala

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,18 @@ trait Symbols { self: Universe =>
504504
*/
505505
def isImplicit: Boolean
506506

507+
/** Does this symbol represent a java enum class or a java enum value?
508+
*
509+
* @group Tests
510+
*/
511+
def isJavaEnum: Boolean
512+
513+
/** Does this symbol represent a java annotation interface?
514+
*
515+
* @group Tests
516+
*/
517+
def isJavaAnnotation: Boolean
518+
507519
/******************* helpers *******************/
508520

509521
/** Provides an alternate if symbol is a NoSymbol.

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,8 @@ 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
347+
case JAVA_ACC_ENUM => JAVA_ENUM
348+
case JAVA_ACC_ANNOTATION => JAVA_ANNOTATION
348349
case _ => 0L
349350
}
350351
private def translateFlags(jflags: Int, baseFlags: Long, isClass: Boolean): Long = {
@@ -360,6 +361,7 @@ object ClassfileConstants {
360361
res |= translateFlag0(jflags & JAVA_ACC_ABSTRACT)
361362
res |= translateFlag0(jflags & JAVA_ACC_INTERFACE)
362363
res |= translateFlag0(jflags & JAVA_ACC_ENUM)
364+
res |= translateFlag0(jflags & JAVA_ACC_ANNOTATION)
363365
res
364366
}
365367

‎src/reflect/scala/reflect/internal/Definitions.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -815,7 +815,7 @@ trait Definitions extends api.StandardDefinitions {
815815
// must filter out "universal" members (getClass is deferred for some reason)
816816
val deferredMembers = (
817817
tp membersBasedOnFlags (excludedFlags = BridgeAndPrivateFlags, requiredFlags = METHOD)
818-
filter (mem => mem.isDeferredNotDefault && !isUniversalMember(mem)) // TODO: test
818+
filter (mem => mem.isDeferredNotJavaDefault && !isUniversalMember(mem)) // TODO: test
819819
)
820820

821821
// if there is only one, it's monomorphic and has a single argument list

‎src/reflect/scala/reflect/internal/FlagSets.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ trait FlagSets extends api.FlagSets { self: SymbolTable =>
4242
val DEFAULTPARAM : FlagSet = Flags.DEFAULTPARAM
4343
val PRESUPER : FlagSet = Flags.PRESUPER
4444
val DEFAULTINIT : FlagSet = Flags.DEFAULTINIT
45-
val ENUM : FlagSet = Flags.ENUM
45+
val ENUM : FlagSet = Flags.JAVA_ENUM
4646
val PARAMACCESSOR : FlagSet = Flags.PARAMACCESSOR
4747
val CASEACCESSOR : FlagSet = Flags.CASEACCESSOR
4848
val SYNTHETIC : FlagSet = Flags.SYNTHETIC

0 commit comments

Comments
 (0)