From 57a5da8140f71da11ee7d7d61f79565b4d13ba94 Mon Sep 17 00:00:00 2001 From: Tom Grigg Date: Thu, 12 May 2022 20:33:10 -0700 Subject: [PATCH] Require seeing `ScalaSig` before scanning for Scala pickling annotations An alternative fix for #15166, which aligns with the behavior of the Scala 2 classfile parser. Before this commit, all classfiles, including those produced by the Java compiler and compilers of other JVM languages, were scanned for Scala pickling annotations. In certain situations (as in tests/pos/i15166), this is problematic, as the denotation of an annotation symbol defined as a Java inner class may be forced before the inner class table is populated and setClassInfo is called on the class root. We avoid this situation by only performing the pickling annotation scan for those classfiles having the `ScalaSig` attribute, i.e. those produced by the Scala 2 compiler. We also drop support for pickling TASTy using the classfile annotations `scala.annotation.internal.TASTYSignature` and `scala.annotation.internal.TASTYLongSignature`. These were never used by the compiler, there are no plans for future use, and preserving support would complicate this fix. --- compiler/src/dotty/tools/dotc/core/Definitions.scala | 2 -- .../tools/dotc/core/classfile/ClassfileParser.scala | 12 ++++-------- .../annotation/internal/TASTYLongSignature.java | 1 + .../scala/annotation/internal/TASTYSignature.java | 1 + tests/pos/i15166/InterfaceAudience_JAVA_ONLY_1.java | 8 ++++++++ tests/pos/i15166/InterfaceStability_JAVA_ONLY_1.java | 8 ++++++++ tests/pos/i15166/Test_2.scala | 4 ++++ 7 files changed, 26 insertions(+), 10 deletions(-) create mode 100644 tests/pos/i15166/InterfaceAudience_JAVA_ONLY_1.java create mode 100644 tests/pos/i15166/InterfaceStability_JAVA_ONLY_1.java create mode 100644 tests/pos/i15166/Test_2.scala diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index dc672690702c..e709cb0ce914 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -920,8 +920,6 @@ class Definitions { @tu lazy val ScalaStrictFPAnnot: ClassSymbol = requiredClass("scala.annotation.strictfp") @tu lazy val ScalaStaticAnnot: ClassSymbol = requiredClass("scala.annotation.static") @tu lazy val SerialVersionUIDAnnot: ClassSymbol = requiredClass("scala.SerialVersionUID") - @tu lazy val TASTYSignatureAnnot: ClassSymbol = requiredClass("scala.annotation.internal.TASTYSignature") - @tu lazy val TASTYLongSignatureAnnot: ClassSymbol = requiredClass("scala.annotation.internal.TASTYLongSignature") @tu lazy val TailrecAnnot: ClassSymbol = requiredClass("scala.annotation.tailrec") @tu lazy val ThreadUnsafeAnnot: ClassSymbol = requiredClass("scala.annotation.threadUnsafe") @tu lazy val ConstructorOnlyAnnot: ClassSymbol = requiredClass("scala.annotation.constructorOnly") diff --git a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala index 8550ffb5fd8e..71d1b0270595 100644 --- a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala +++ b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala @@ -871,7 +871,7 @@ class ClassfileParser( /** Parse inner classes. Expects `in.bp` to point to the superclass entry. * Restores the old `bp`. - * @return true iff classfile is from Scala, so no Java info needs to be read. + * @return Some(unpickler) iff classfile is from Scala, so no Java info needs to be read. */ def unpickleOrParseInnerClasses()(using ctx: Context, in: DataReader): Option[Embedded] = { val oldbp = in.bp @@ -1028,7 +1028,7 @@ class ClassfileParser( // attribute isn't, this classfile is a compilation artifact. return Some(NoEmbedded) - if (scan(tpnme.RuntimeVisibleAnnotationATTR) || scan(tpnme.RuntimeInvisibleAnnotationATTR)) { + if (scan(tpnme.ScalaSignatureATTR) && scan(tpnme.RuntimeVisibleAnnotationATTR)) { val attrLen = in.nextInt val nAnnots = in.nextChar var i = 0 @@ -1039,14 +1039,10 @@ class ClassfileParser( while (j < nArgs) { val argName = pool.getName(in.nextChar) if (argName.name == nme.bytes) { - if (attrClass == defn.ScalaSignatureAnnot) + if attrClass == defn.ScalaSignatureAnnot then return unpickleScala(parseScalaSigBytes) - else if (attrClass == defn.ScalaLongSignatureAnnot) + else if attrClass == defn.ScalaLongSignatureAnnot then return unpickleScala(parseScalaLongSigBytes) - else if (attrClass == defn.TASTYSignatureAnnot) - return unpickleTASTY(parseScalaSigBytes) - else if (attrClass == defn.TASTYLongSignatureAnnot) - return unpickleTASTY(parseScalaLongSigBytes) } parseAnnotArg(skip = true) j += 1 diff --git a/library/src/scala/annotation/internal/TASTYLongSignature.java b/library/src/scala/annotation/internal/TASTYLongSignature.java index 2278da258a5d..e1acf8279637 100644 --- a/library/src/scala/annotation/internal/TASTYLongSignature.java +++ b/library/src/scala/annotation/internal/TASTYLongSignature.java @@ -7,6 +7,7 @@ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.TYPE) +@Deprecated public @interface TASTYLongSignature { public String[] bytes(); } diff --git a/library/src/scala/annotation/internal/TASTYSignature.java b/library/src/scala/annotation/internal/TASTYSignature.java index a6372f008397..ee042eff5f9e 100644 --- a/library/src/scala/annotation/internal/TASTYSignature.java +++ b/library/src/scala/annotation/internal/TASTYSignature.java @@ -7,6 +7,7 @@ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.TYPE) +@Deprecated public @interface TASTYSignature { public String bytes(); } diff --git a/tests/pos/i15166/InterfaceAudience_JAVA_ONLY_1.java b/tests/pos/i15166/InterfaceAudience_JAVA_ONLY_1.java new file mode 100644 index 000000000000..2b6256620594 --- /dev/null +++ b/tests/pos/i15166/InterfaceAudience_JAVA_ONLY_1.java @@ -0,0 +1,8 @@ +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +@InterfaceStability_JAVA_ONLY_1.Evolving(bytes="no") +public class InterfaceAudience_JAVA_ONLY_1 { + @Retention(RetentionPolicy.RUNTIME) + public @interface Public { String bytes(); } +} diff --git a/tests/pos/i15166/InterfaceStability_JAVA_ONLY_1.java b/tests/pos/i15166/InterfaceStability_JAVA_ONLY_1.java new file mode 100644 index 000000000000..5bdf9b90a0f3 --- /dev/null +++ b/tests/pos/i15166/InterfaceStability_JAVA_ONLY_1.java @@ -0,0 +1,8 @@ +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +@InterfaceAudience_JAVA_ONLY_1.Public(bytes="yes") +public class InterfaceStability_JAVA_ONLY_1 { + @Retention(RetentionPolicy.RUNTIME) + public @interface Evolving { String bytes(); } +} diff --git a/tests/pos/i15166/Test_2.scala b/tests/pos/i15166/Test_2.scala new file mode 100644 index 000000000000..53e6da69a42c --- /dev/null +++ b/tests/pos/i15166/Test_2.scala @@ -0,0 +1,4 @@ +// scalac: -Xfatal-warnings +object Test { + val x: InterfaceAudience_JAVA_ONLY_1.Public = ??? +}