From 77b8b6a11fbcb067160052a54b9d777593787fb5 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Wed, 15 Jul 2015 19:59:57 +0200 Subject: SI-9393 fix modifiers of ClassBTypes for Java annotations The Scala classfile and java source parsers make Java annotation classes (which are actually interfaces at the classfile level) look like Scala annotation classes: - the INTERFACE / ABSTRACT flags are not added - scala.annotation.Annotation is added as superclass - scala.annotation.ClassfileAnnotation is added as interface This makes type-checking @Annot uniform, whether it is defined in Java or Scala. This is a hack that leads to various bugs (SI-9393, SI-9400). Instead the type-checking should be special-cased for Java annotations. This commit fixes SI-9393 and a part of SI-9400, but it's still easy to generate invalid classfiles. Restores the assertions that were disabled in #4621. I'd like to leave these assertions in: they are valuable and helped uncovering the issue being fixed here. A new flag JAVA_ANNOTATION is introduced for Java annotation ClassSymbols, similar to the existing ENUM flag. When building ClassBTypes for Java annotations, the flags, superclass and interfaces are recovered to represent the situation in the classfile. Cleans up and documents the flags space in the area of "late" and "anti" flags. The test for SI-9393 is extended to test both the classfile and the java source parser. --- test/files/jvm/innerClassAttribute/Test.scala | 4 +- test/files/pos/t9393/Named.java | 3 - test/files/pos/t9393/NamedImpl.java | 15 ---- test/files/pos/t9393/NamedImpl_1.java | 15 ++++ test/files/pos/t9393/NamedImpl_2.java | 15 ++++ test/files/pos/t9393/Named_1.java | 3 + test/files/pos/t9393/Named_2.java | 3 + test/files/pos/t9393/test.scala | 3 - test/files/pos/t9393/test_2.scala | 4 + test/junit/scala/tools/nsc/symtab/FlagsTest.scala | 89 +++++++++++++++++++++++ 10 files changed, 130 insertions(+), 24 deletions(-) delete mode 100644 test/files/pos/t9393/Named.java delete mode 100644 test/files/pos/t9393/NamedImpl.java create mode 100644 test/files/pos/t9393/NamedImpl_1.java create mode 100644 test/files/pos/t9393/NamedImpl_2.java create mode 100644 test/files/pos/t9393/Named_1.java create mode 100644 test/files/pos/t9393/Named_2.java delete mode 100644 test/files/pos/t9393/test.scala create mode 100644 test/files/pos/t9393/test_2.scala create mode 100644 test/junit/scala/tools/nsc/symtab/FlagsTest.scala (limited to 'test') diff --git a/test/files/jvm/innerClassAttribute/Test.scala b/test/files/jvm/innerClassAttribute/Test.scala index ca50beae7f..702e5e279a 100644 --- a/test/files/jvm/innerClassAttribute/Test.scala +++ b/test/files/jvm/innerClassAttribute/Test.scala @@ -145,9 +145,7 @@ object Test extends BytecodeTest { def testA11() = { val List(ann) = innerClassNodes("A11") - // in the java class file, the INNERCLASS attribute has more flags (public | static | abstract | interface | annotation) - // the scala compiler has its own interpretation of java annotations ant their flags.. it only emits publicStatic. - assertMember(ann, "JavaAnnot_1", "Ann", flags = publicStatic) + assertMember(ann, "JavaAnnot_1", "Ann", flags = publicAbstractInterface | Flags.ACC_STATIC | Flags.ACC_ANNOTATION) } def testA13() = { diff --git a/test/files/pos/t9393/Named.java b/test/files/pos/t9393/Named.java deleted file mode 100644 index 144ddbf26e..0000000000 --- a/test/files/pos/t9393/Named.java +++ /dev/null @@ -1,3 +0,0 @@ -package bug; - -public @interface Named {} diff --git a/test/files/pos/t9393/NamedImpl.java b/test/files/pos/t9393/NamedImpl.java deleted file mode 100644 index 7918739c2b..0000000000 --- a/test/files/pos/t9393/NamedImpl.java +++ /dev/null @@ -1,15 +0,0 @@ -/* - * Copyright (C) 2009-2015 Typesafe Inc. - */ -package bug; - -import bug.Named; -import java.io.Serializable; -import java.lang.annotation.Annotation; - -public class NamedImpl implements Named { - - public Class annotationType() { - return null; - } -} diff --git a/test/files/pos/t9393/NamedImpl_1.java b/test/files/pos/t9393/NamedImpl_1.java new file mode 100644 index 0000000000..02ec9b4671 --- /dev/null +++ b/test/files/pos/t9393/NamedImpl_1.java @@ -0,0 +1,15 @@ +/* + * Copyright (C) 2009-2015 Typesafe Inc. + */ +package bug; + +import bug.Named_1; +import java.io.Serializable; +import java.lang.annotation.Annotation; + +public class NamedImpl_1 implements Named_1 { + + public Class annotationType() { + return null; + } +} diff --git a/test/files/pos/t9393/NamedImpl_2.java b/test/files/pos/t9393/NamedImpl_2.java new file mode 100644 index 0000000000..c87e94016d --- /dev/null +++ b/test/files/pos/t9393/NamedImpl_2.java @@ -0,0 +1,15 @@ +/* + * Copyright (C) 2009-2015 Typesafe Inc. + */ +package bug; + +import bug.Named_2; +import java.io.Serializable; +import java.lang.annotation.Annotation; + +public class NamedImpl_2 implements Named_2 { + + public Class annotationType() { + return null; + } +} diff --git a/test/files/pos/t9393/Named_1.java b/test/files/pos/t9393/Named_1.java new file mode 100644 index 0000000000..30a6c9839a --- /dev/null +++ b/test/files/pos/t9393/Named_1.java @@ -0,0 +1,3 @@ +package bug; + +public @interface Named_1 {} diff --git a/test/files/pos/t9393/Named_2.java b/test/files/pos/t9393/Named_2.java new file mode 100644 index 0000000000..3210fb636a --- /dev/null +++ b/test/files/pos/t9393/Named_2.java @@ -0,0 +1,3 @@ +package bug; + +public @interface Named_2 {} diff --git a/test/files/pos/t9393/test.scala b/test/files/pos/t9393/test.scala deleted file mode 100644 index 4df0476c98..0000000000 --- a/test/files/pos/t9393/test.scala +++ /dev/null @@ -1,3 +0,0 @@ -class C { - new bug.NamedImpl -} diff --git a/test/files/pos/t9393/test_2.scala b/test/files/pos/t9393/test_2.scala new file mode 100644 index 0000000000..8ea346129d --- /dev/null +++ b/test/files/pos/t9393/test_2.scala @@ -0,0 +1,4 @@ +class C { + new bug.NamedImpl_1 // separate compilation, testing the classfile parser + new bug.NamedImpl_2 // mixed compilation, testing the java source parser +} diff --git a/test/junit/scala/tools/nsc/symtab/FlagsTest.scala b/test/junit/scala/tools/nsc/symtab/FlagsTest.scala new file mode 100644 index 0000000000..fc0e8b0f6b --- /dev/null +++ b/test/junit/scala/tools/nsc/symtab/FlagsTest.scala @@ -0,0 +1,89 @@ +package scala.tools.nsc +package symtab + +import org.junit.Assert._ +import scala.tools.testing.AssertUtil._ +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +@RunWith(classOf[JUnit4]) +class FlagsTest { + object symbolTable extends SymbolTableForUnitTesting + import symbolTable._ + import Flags._ + + def sym = NoSymbol.newTermSymbol(nme.EMPTY) + + def withFlagMask[A](mask: Long)(body: => A): A = enteringPhase(new Phase(NoPhase) { + override def flagMask = mask + def name = "" + def run() = () + })(body) + + def testTimedFlag(flag: Long, test: Symbol => Boolean, enabling: Boolean) = { + assertEquals(withFlagMask(InitialFlags)(test(sym.setFlag(flag))), !enabling) + assertEquals(withFlagMask(InitialFlags | flag)(test(sym.setFlag(flag))), enabling) + } + + def testLate(flag: Long, test: Symbol => Boolean) = testTimedFlag(flag, test, enabling = true) + def testNot(flag: Long, test: Symbol => Boolean) = testTimedFlag(flag, test, enabling = false) + + @Test + def testTimedFlags(): Unit = { + testLate(lateDEFERRED, _.isDeferred) + testLate(lateFINAL, _.isFinal) + testLate(lateINTERFACE, _.isInterface) + testLate(lateMETHOD, _.isMethod) + testLate(lateMODULE, _.isModule) + testNot(PROTECTED | notPROTECTED, _.isProtected) + testNot(OVERRIDE | notOVERRIDE, _.isOverride) + testNot(PRIVATE | notPRIVATE, _.isPrivate) + + assertFalse(withFlagMask(AllFlags)(sym.setFlag(PRIVATE | notPRIVATE).isPrivate)) + + assertEquals(withFlagMask(InitialFlags)(sym.setFlag(PRIVATE | notPRIVATE).flags & PRIVATE), PRIVATE) + assertEquals(withFlagMask(AllFlags)(sym.setFlag(PRIVATE | notPRIVATE).flags & PRIVATE), 0) + } + + @Test + def normalLateOverlap(): Unit = { + // late flags are shifted by LateShift == 47. + // however, the first late flag is lateDEFERRED, which is DEFERRED << 47 == (1 << 4) << 47 == 1 << 51 + // the flags from 1 << 47 to 1 << 50 are not late flags. this is ensured by the LateFlags mask. + + for (i <- 0 to 3) { + val f = 1L << i + assertEquals(withFlagMask(AllFlags)(sym.setFlag(f << LateShift).flags & f), 0) // not treated as late flag + } + for (i <- 4 to 8) { + val f = 1L << i + assertEquals(withFlagMask(AllFlags)(sym.setFlag(f << LateShift).flags & f), f) // treated as late flag + } + } + + @Test + def normalAnti(): Unit = { + for (i <- 0 to 2) { + val f = 1L << i + assertEquals(withFlagMask(AllFlags)(sym.setFlag(f | (f << AntiShift)).flags & f), 0) // negated flags + } + for (i <- 3 to 7) { + val f = 1L << i + assertEquals(withFlagMask(AllFlags)(sym.setFlag(f | (f << AntiShift)).flags & f), f) // not negated + } + } + + @Test + def lateAntiCrossCheck(): Unit = { + val allButNegatable = AllFlags & ~(PROTECTED | OVERRIDE | PRIVATE) + val lateable = 0L | DEFERRED | FINAL | INTERFACE | METHOD | MODULE + val lateFlags = lateable << LateShift + val allButLateable = AllFlags & ~lateable + + assertEquals(withFlagMask(AllFlags)(sym.setFlag(AllFlags).flags), allButNegatable) + assertEquals(withFlagMask(AllFlags)(sym.setFlag(allButLateable).flags), allButNegatable) + + assertEquals(withFlagMask(AllFlags)(sym.setFlag(lateFlags).flags), lateFlags | lateable) + } +} -- cgit v1.2.3