Merge pull request scala#4067 from lrytz/t8926

SI-8926 default visbility RUNTIME for java annotations

Author: gkossakowski

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 =>

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")
+  }
 }