diff options
| -rw-r--r-- | bincompat-backward.whitelist.conf | 9 | ||||
| -rw-r--r-- | bincompat-forward.whitelist.conf | 97 | ||||
| -rw-r--r-- | src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala | 15 | ||||
| -rw-r--r-- | src/compiler/scala/tools/nsc/transform/Flatten.scala | 7 | ||||
| -rw-r--r-- | src/library/scala/collection/TraversableLike.scala | 2 | ||||
| -rw-r--r-- | src/library/scala/collection/immutable/Stream.scala | 24 | ||||
| -rw-r--r-- | src/reflect/scala/reflect/internal/Symbols.scala | 44 | ||||
| -rw-r--r-- | test/files/run/t4332.scala | 2 | ||||
| -rw-r--r-- | test/files/run/t8907.scala | 39 | ||||
| -rw-r--r-- | test/junit/scala/collection/immutable/StreamTest.scala | 18 | ||||
| -rw-r--r-- | test/junit/scala/issues/BytecodeTests.scala | 41 |
11 files changed, 131 insertions, 167 deletions
diff --git a/bincompat-backward.whitelist.conf b/bincompat-backward.whitelist.conf index 6c98dc62a1..076b9bb9aa 100644 --- a/bincompat-backward.whitelist.conf +++ b/bincompat-backward.whitelist.conf @@ -186,15 +186,6 @@ filter { matchName="scala.reflect.runtime.SynchronizedOps.newNestedScope" problemName=MissingMethodProblem }, - // see github.com/scala/scala/pull/3925, SI-8627, SI-6440 - { - matchName="scala.collection.TraversableLike.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.immutable.Stream.filteredTail" - problemName=MissingMethodProblem - }, // https://github.com/scala/scala/pull/3848 -- SI-8680 { matchName="scala.collection.immutable.Stream.scala$collection$immutable$Stream$$loop$6" diff --git a/bincompat-forward.whitelist.conf b/bincompat-forward.whitelist.conf index 87a59f2d53..53401eefad 100644 --- a/bincompat-forward.whitelist.conf +++ b/bincompat-forward.whitelist.conf @@ -272,103 +272,6 @@ filter { matchName="scala.reflect.api.PredefTypeCreator" problemName=MissingClassProblem }, - // see github.com/scala/scala/pull/3925, SI-8627, SI-6440 - { - matchName="scala.collection.IterableViewLike#AbstractTransformed.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.AbstractTraversable.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.TraversableViewLike#AbstractTransformed.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.TraversableLike.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.SeqViewLike#AbstractTransformed.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.immutable.TreeSet.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.immutable.Stream.filteredTail" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.immutable.Stream.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.immutable.Stream.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.immutable.StringOps.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.immutable.TreeMap.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.concurrent.TrieMap.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.mutable.ArrayOps#ofByte.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.mutable.ArrayOps#ofLong.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.mutable.ArrayOps#ofUnit.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.mutable.ArrayOps#ofInt.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.mutable.ArrayOps#ofChar.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.mutable.ArrayOps#ofRef.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.mutable.ArrayOps#ofDouble.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.mutable.ArrayOps#ofFloat.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.mutable.ArrayOps#ofBoolean.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.mutable.ArrayOps#ofShort.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.collection.mutable.TreeSet.filterImpl" - problemName=MissingMethodProblem - }, - { - matchName="scala.reflect.io.AbstractFile.filterImpl" - problemName=MissingMethodProblem - }, // https://github.com/scala/scala/pull/3848 -- SI-8680 { matchName="scala.collection.immutable.Stream.scala$collection$immutable$Stream$$loop$6" diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala index 14b61777b0..54d4a30553 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeAsmCommon.scala @@ -19,7 +19,7 @@ final class BCodeAsmCommon[G <: Global](val global: G) { val ExcludedForwarderFlags = { import scala.tools.nsc.symtab.Flags._ // Should include DEFERRED but this breaks findMember. - ( SPECIALIZED | LIFTED | PROTECTED | STATIC | EXPANDEDNAME | BridgeAndPrivateFlags | MACRO ) + SPECIALIZED | LIFTED | PROTECTED | STATIC | EXPANDEDNAME | BridgeAndPrivateFlags | MACRO } /** @@ -147,9 +147,16 @@ final class BCodeAsmCommon[G <: Global](val global: G) { annot.args.isEmpty } - def isRuntimeVisible(annot: AnnotationInfo): Boolean = - annot.atp.typeSymbol.getAnnotation(AnnotationRetentionAttr) - .exists(_.assocs.contains((nme.value -> LiteralAnnotArg(Constant(AnnotationRetentionPolicyRuntimeValue))))) + def isRuntimeVisible(annot: AnnotationInfo): Boolean = { + annot.atp.typeSymbol.getAnnotation(AnnotationRetentionAttr) match { + case Some(retentionAnnot) => + retentionAnnot.assocs.contains(nme.value -> LiteralAnnotArg(Constant(AnnotationRetentionPolicyRuntimeValue))) + case _ => + // SI-8926: if the annotation class symbol doesn't have a @RetentionPolicy annotation, the + // annotation is emitted with visibility `RUNTIME` + true + } + } private def retentionPolicyOf(annot: AnnotationInfo): Symbol = annot.atp.typeSymbol.getAnnotation(AnnotationRetentionAttr).map(_.assocs).map(assoc => diff --git a/src/compiler/scala/tools/nsc/transform/Flatten.scala b/src/compiler/scala/tools/nsc/transform/Flatten.scala index fa53ef48b5..4662ef6224 100644 --- a/src/compiler/scala/tools/nsc/transform/Flatten.scala +++ b/src/compiler/scala/tools/nsc/transform/Flatten.scala @@ -77,8 +77,11 @@ abstract class Flatten extends InfoTransform { if (sym.isTerm && !sym.isStaticModule) { decls1 enter sym if (sym.isModule) { - // Nested, non-static moduls are transformed to methods. - assert(sym.isMethod, s"Non-static $sym should have the lateMETHOD flag from RefChecks") + // In theory, we could assert(sym.isMethod), because nested, non-static moduls are + // transformed to methods (lateMETHOD flag added in RefChecks). But this requires + // forcing sym.info (see comment on isModuleNotMethod), which forces stub symbols + // too eagerly (SI-8907). + // Note that module classes are not entered into the 'decls' of the ClassInfoType // of the outer class, only the module symbols are. So the current loop does // not visit module classes. Therefore we set the LIFTED flag here for module diff --git a/src/library/scala/collection/TraversableLike.scala b/src/library/scala/collection/TraversableLike.scala index a8731a51b1..d3a7db6968 100644 --- a/src/library/scala/collection/TraversableLike.scala +++ b/src/library/scala/collection/TraversableLike.scala @@ -253,7 +253,7 @@ trait TraversableLike[+A, +Repr] extends Any b.result } - private[scala] def filterImpl(p: A => Boolean, isFlipped: Boolean): Repr = { + private def filterImpl(p: A => Boolean, isFlipped: Boolean): Repr = { val b = newBuilder for (x <- this) if (p(x) != isFlipped) b += x diff --git a/src/library/scala/collection/immutable/Stream.scala b/src/library/scala/collection/immutable/Stream.scala index 91a4e1c43d..714d5117d3 100644 --- a/src/library/scala/collection/immutable/Stream.scala +++ b/src/library/scala/collection/immutable/Stream.scala @@ -499,16 +499,6 @@ self => ) else super.flatMap(f)(bf) - override private[scala] def filterImpl(p: A => Boolean, isFlipped: Boolean): Stream[A] = { - // optimization: drop leading prefix of elems for which f returns false - // var rest = this dropWhile (!p(_)) - forget DRY principle - GC can't collect otherwise - var rest = this - while (!rest.isEmpty && p(rest.head) == isFlipped) rest = rest.tail - // private utility func to avoid `this` on stack (would be needed for the lazy arg) - if (rest.nonEmpty) Stream.filteredTail(rest, p, isFlipped) - else Stream.Empty - } - /** Returns all the elements of this `Stream` that satisfy the predicate `p` * in a new `Stream` - i.e., it is still a lazy data structure. The order of * the elements is preserved @@ -522,7 +512,15 @@ self => * // produces * }}} */ - override def filter(p: A => Boolean): Stream[A] = filterImpl(p, isFlipped = false) // This override is only left in 2.11 because of binary compatibility, see PR #3925 + override def filter(p: A => Boolean): Stream[A] = { + // optimization: drop leading prefix of elems for which f returns false + // var rest = this dropWhile (!p(_)) - forget DRY principle - GC can't collect otherwise + var rest = this + while (!rest.isEmpty && !p(rest.head)) rest = rest.tail + // private utility func to avoid `this` on stack (would be needed for the lazy arg) + if (rest.nonEmpty) Stream.filteredTail(rest, p) + else Stream.Empty + } override final def withFilter(p: A => Boolean): StreamWithFilter = new StreamWithFilter(p) @@ -1286,8 +1284,8 @@ object Stream extends SeqFactory[Stream] { else cons(start, range(start + step, end, step)) } - private[immutable] def filteredTail[A](stream: Stream[A], p: A => Boolean, isFlipped: Boolean) = { - cons(stream.head, stream.tail.filterImpl(p, isFlipped)) + private[immutable] def filteredTail[A](stream: Stream[A], p: A => Boolean) = { + cons(stream.head, stream.tail filter p) } private[immutable] def collectedTail[A, B, That](head: B, stream: Stream[A], pf: PartialFunction[A, B], bf: CanBuildFrom[Stream[A], B, That]) = { diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index b0c23ef45d..2c039ab5a7 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -735,31 +735,31 @@ trait Symbols extends api.Symbols { self: SymbolTable => final def hasGetter = isTerm && nme.isLocalName(name) - /** A little explanation for this confusing situation. - * Nested modules which have no static owner when ModuleDefs - * are eliminated (refchecks) are given the lateMETHOD flag, - * which makes them appear as methods after refchecks. - * Here's an example where one can see all four of FF FT TF TT - * for (isStatic, isMethod) at various phases. + /** + * Nested modules which have no static owner when ModuleDefs are eliminated (refchecks) are + * given the lateMETHOD flag, which makes them appear as methods after refchecks. + * + * Note: the lateMETHOD flag is added lazily in the info transformer of the RefChecks phase. + * This means that forcing the `sym.info` may change the value of `sym.isMethod`. Forcing the + * info is in the responsability of the caller. Doing it eagerly here was tried (0ccdb151f) but + * has proven to lead to bugs (SI-8907). * - * trait A1 { case class Quux() } - * object A2 extends A1 { object Flax } - * // -- namer object Quux in trait A1 - * // -M flatten object Quux in trait A1 - * // S- flatten object Flax in object A2 - * // -M posterasure object Quux in trait A1 - * // -M jvm object Quux in trait A1 - * // SM jvm object Quux in object A2 + * Here's an example where one can see all four of FF FT TF TT for (isStatic, isMethod) at + * various phases. * - * So "isModuleNotMethod" exists not for its achievement in - * brevity, but to encapsulate the relevant condition. + * trait A1 { case class Quux() } + * object A2 extends A1 { object Flax } + * // -- namer object Quux in trait A1 + * // -M flatten object Quux in trait A1 + * // S- flatten object Flax in object A2 + * // -M posterasure object Quux in trait A1 + * // -M jvm object Quux in trait A1 + * // SM jvm object Quux in object A2 + * + * So "isModuleNotMethod" exists not for its achievement in brevity, but to encapsulate the + * relevant condition. */ - def isModuleNotMethod = { - if (isModule) { - if (phase.refChecked) this.info // force completion to make sure lateMETHOD is there. - !isMethod - } else false - } + def isModuleNotMethod = isModule && !isMethod // After RefChecks, the `isStatic` check is mostly redundant: all non-static modules should // be methods (and vice versa). There's a corner case on the vice-versa with mixed-in module diff --git a/test/files/run/t4332.scala b/test/files/run/t4332.scala index 1c7e7d73de..5a67922911 100644 --- a/test/files/run/t4332.scala +++ b/test/files/run/t4332.scala @@ -12,7 +12,7 @@ object Test extends DirectTest { } def isExempt(sym: Symbol) = { - val exempt = Set("view", "repr", "sliceWithKnownDelta", "sliceWithKnownBound", "transform", "filterImpl") + val exempt = Set("view", "repr", "sliceWithKnownDelta", "sliceWithKnownBound", "transform") (exempt contains sym.name.decoded) } diff --git a/test/files/run/t8907.scala b/test/files/run/t8907.scala new file mode 100644 index 0000000000..7952ac82d9 --- /dev/null +++ b/test/files/run/t8907.scala @@ -0,0 +1,39 @@ +import scala.tools.partest._ +import java.io.File + +object Test extends StoreReporterDirectTest { + def code = ??? + + def compileCode(code: String) = { + val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator") + compileString(newCompiler("-cp", classpath, "-d", testOutput.path))(code) + } + + def show(): Unit = { + compileCode(""" + class C { class Inner } + + class D { + object O { + def foo(c: C)(i: c.Inner): c.Inner = ??? + } + } + """) + assert(filteredInfos.isEmpty, filteredInfos) + deleteClass("C") + compileCode(""" + class E { + def foo = { + (null: D).toString + } + } + """) + assert(storeReporter.infos.isEmpty, storeReporter.infos.mkString("\n")) // Included a MissingRequirementError before. + } + + def deleteClass(name: String) { + val classFile = new File(testOutput.path, name + ".class") + assert(classFile.exists) + assert(classFile.delete()) + } +} diff --git a/test/junit/scala/collection/immutable/StreamTest.scala b/test/junit/scala/collection/immutable/StreamTest.scala deleted file mode 100644 index 6dc1c79a48..0000000000 --- a/test/junit/scala/collection/immutable/StreamTest.scala +++ /dev/null @@ -1,18 +0,0 @@ -package scala.collection.immutable - -import org.junit.runner.RunWith -import org.junit.runners.JUnit4 -import org.junit.Test -import org.junit.Assert._ - -@RunWith(classOf[JUnit4]) -class StreamTest { - - @Test - def t6727_and_t6440(): Unit = { - assertTrue(Stream.continually(()).filter(_ => true).take(2) == Seq((), ())) - assertTrue(Stream.continually(()).filterNot(_ => false).take(2) == Seq((), ())) - assertTrue(Stream(1,2,3,4,5).filter(_ < 4) == Seq(1,2,3)) - assertTrue(Stream(1,2,3,4,5).filterNot(_ > 4) == Seq(1,2,3,4)) - } -} diff --git a/test/junit/scala/issues/BytecodeTests.scala b/test/junit/scala/issues/BytecodeTests.scala index 7a05472277..d4ed063a03 100644 --- a/test/junit/scala/issues/BytecodeTests.scala +++ b/test/junit/scala/issues/BytecodeTests.scala @@ -4,6 +4,7 @@ import org.junit.runner.RunWith import org.junit.runners.JUnit4 import org.junit.Test import scala.tools.asm.Opcodes +import scala.tools.nsc.backend.jvm.AsmUtils import scala.tools.nsc.backend.jvm.CodeGenTools._ import org.junit.Assert._ import scala.collection.JavaConverters._ @@ -36,4 +37,44 @@ class BytecodeTests { assertTrue(getSingleMethod(c, "f").instructions.count(_.isInstanceOf[TableSwitch]) == 1) assertTrue(getSingleMethod(c, "g").instructions.count(_.isInstanceOf[LookupSwitch]) == 1) } + + @Test + def t8926(): Unit = { + import scala.reflect.internal.util.BatchSourceFile + + // this test cannot be implemented using partest because of its mixed-mode compilation strategy: + // partest first compiles all files with scalac, then the java files, and then again the scala + // using the output classpath. this shadows the bug SI-8926. + + val annotA = + """import java.lang.annotation.Retention; + |import java.lang.annotation.RetentionPolicy; + |@Retention(RetentionPolicy.RUNTIME) + |public @interface AnnotA { } + """.stripMargin + val annotB = "public @interface AnnotB { }" + + val scalaSrc = + """@AnnotA class A + |@AnnotB class B + """.stripMargin + + val compiler = newCompiler() + val run = new compiler.Run() + run.compileSources(List(new BatchSourceFile("AnnotA.java", annotA), new BatchSourceFile("AnnotB.java", annotB), new BatchSourceFile("Test.scala", scalaSrc))) + val outDir = compiler.settings.outputDirs.getSingleOutput.get + val outfiles = (for (f <- outDir.iterator if !f.isDirectory) yield (f.name, f.toByteArray)).toList + + def check(classfile: String, annotName: String) = { + val f = (outfiles collect { case (`classfile`, bytes) => AsmUtils.readClass(bytes) }).head + val descs = f.visibleAnnotations.asScala.map(_.desc).toList + assertTrue(descs.toString, descs exists (_ contains annotName)) + } + + check("A.class", "AnnotA") + + // known issue SI-8928: the visibility of AnnotB should be CLASS, but annotation classes without + // a @Retention annotation are currently emitted as RUNTIME. + check("B.class", "AnnotB") + } } |