From ae17256f1dcde4dd82008c6e355604d68d5a07b3 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Fri, 28 Oct 2016 12:08:55 +0200 Subject: For scala classfiles, only parse the scala signature annotation Skipping other annotations not only saves some cycles / GC, but also prevents some spurious warnings / errors related to cyclic dependencies when parsing annotation arguments refering to members of the class. --- .../nsc/symtab/classfile/ClassfileParser.scala | 69 ++++++++++++++-------- test/files/neg/t7014.check | 5 ++ test/files/neg/t7014.flags | 1 + test/files/neg/t7014/ThreadSafety.java | 9 +++ test/files/neg/t7014/ThreadSafetyLevel.java | 8 +++ test/files/neg/t7014/t7014.scala | 3 + test/files/pos/t5165b.flags | 1 + test/files/pos/t7014/ThreadSafety.java | 9 --- test/files/pos/t7014/ThreadSafetyLevel.java | 8 --- test/files/pos/t7014/t7014.scala | 3 - test/files/pos/t7551.flags | 1 + test/files/pos/t7551/A.java | 9 +++ test/files/pos/t7551/T.scala | 9 +++ test/files/pos/t7551/Test.scala | 5 ++ 14 files changed, 96 insertions(+), 44 deletions(-) create mode 100644 test/files/neg/t7014.check create mode 100644 test/files/neg/t7014.flags create mode 100644 test/files/neg/t7014/ThreadSafety.java create mode 100644 test/files/neg/t7014/ThreadSafetyLevel.java create mode 100644 test/files/neg/t7014/t7014.scala create mode 100644 test/files/pos/t5165b.flags delete mode 100644 test/files/pos/t7014/ThreadSafety.java delete mode 100644 test/files/pos/t7014/ThreadSafetyLevel.java delete mode 100644 test/files/pos/t7014/t7014.scala create mode 100644 test/files/pos/t7551.flags create mode 100644 test/files/pos/t7551/A.java create mode 100644 test/files/pos/t7551/T.scala create mode 100644 test/files/pos/t7551/Test.scala diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala index f1ccc29afc..e136fdf6d1 100644 --- a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala +++ b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala @@ -820,16 +820,19 @@ abstract class ClassfileParser { // Java annotations on classes / methods / fields with RetentionPolicy.RUNTIME case tpnme.RuntimeAnnotationATTR => if (isScalaAnnot || !isScala) { - val scalaSigAnnot = parseAnnotations(attrLen) - if (isScalaAnnot) - scalaSigAnnot match { - case Some(san: AnnotationInfo) => - val bytes = - san.assocs.find({ _._1 == nme.bytes }).get._2.asInstanceOf[ScalaSigBytes].bytes - unpickler.unpickle(bytes, 0, clazz, staticModule, in.file.name) - case None => - throw new RuntimeException("Scala class file does not contain Scala annotation") - } + // For Scala classfiles we are only interested in the scala signature annotations. Other + // annotations should be skipped (the pickle contains the symbol's annotations). + // Skipping them also prevents some spurious warnings / errors related to SI-7014, + // SI-7551, pos/5165b + val scalaSigAnnot = parseAnnotations(onlyScalaSig = isScalaAnnot) + if (isScalaAnnot) scalaSigAnnot match { + case Some(san: AnnotationInfo) => + val bytes = + san.assocs.find({ _._1 == nme.bytes }).get._2.asInstanceOf[ScalaSigBytes].bytes + unpickler.unpickle(bytes, 0, clazz, staticModule, in.file.name) + case None => + throw new RuntimeException("Scala class file does not contain Scala annotation") + } debuglog("[class] << " + sym.fullName + sym.annotationsString) } else @@ -863,6 +866,24 @@ abstract class ClassfileParser { } } + def skipAnnotArg(): Unit = { + u1 match { + case STRING_TAG | BOOL_TAG | BYTE_TAG | CHAR_TAG | SHORT_TAG | + INT_TAG | LONG_TAG | FLOAT_TAG | DOUBLE_TAG | CLASS_TAG => + in.skip(2) + + case ENUM_TAG => + in.skip(4) + + case ARRAY_TAG => + val num = u2 + for (i <- 0 until num) skipAnnotArg() + + case ANNOTATION_TAG => + parseAnnotation(u2, onlyScalaSig = true) + } + } + def parseAnnotArg: Option[ClassfileAnnotArg] = { val tag = u1 val index = u2 @@ -896,7 +917,7 @@ abstract class ClassfileParser { if (hasError) None else Some(ArrayAnnotArg(arr.toArray)) case ANNOTATION_TAG => - parseAnnotation(index) map (NestedAnnotArg(_)) + parseAnnotation(index, onlyScalaSig = false) map (NestedAnnotArg(_)) } } @@ -923,7 +944,7 @@ abstract class ClassfileParser { /* Parse and return a single annotation. If it is malformed, * return None. */ - def parseAnnotation(attrNameIndex: Int): Option[AnnotationInfo] = try { + def parseAnnotation(attrNameIndex: Int, onlyScalaSig: Boolean): Option[AnnotationInfo] = try { val attrType = pool.getType(attrNameIndex) val nargs = u2 val nvpairs = new ListBuffer[(Name, ClassfileAnnotArg)] @@ -944,7 +965,8 @@ abstract class ClassfileParser { case None => hasError = true } else - parseAnnotArg match { + if (onlyScalaSig) skipAnnotArg() + else parseAnnotArg match { case Some(c) => nvpairs += ((name, c)) case None => hasError = true } @@ -986,19 +1008,18 @@ abstract class ClassfileParser { /* Parse a sequence of annotations and attaches them to the * current symbol sym, except for the ScalaSignature annotation that it returns, if it is available. */ - def parseAnnotations(len: Int): Option[AnnotationInfo] = { + def parseAnnotations(onlyScalaSig: Boolean): Option[AnnotationInfo] = { val nAttr = u2 var scalaSigAnnot: Option[AnnotationInfo] = None - for (n <- 0 until nAttr) - parseAnnotation(u2) match { - case Some(scalaSig) if (scalaSig.atp == ScalaSignatureAnnotation.tpe) => - scalaSigAnnot = Some(scalaSig) - case Some(scalaSig) if (scalaSig.atp == ScalaLongSignatureAnnotation.tpe) => - scalaSigAnnot = Some(scalaSig) - case Some(annot) => - sym.addAnnotation(annot) - case None => - } + for (n <- 0 until nAttr) parseAnnotation(u2, onlyScalaSig) match { + case Some(scalaSig) if scalaSig.atp == ScalaSignatureAnnotation.tpe => + scalaSigAnnot = Some(scalaSig) + case Some(scalaSig) if scalaSig.atp == ScalaLongSignatureAnnotation.tpe => + scalaSigAnnot = Some(scalaSig) + case Some(annot) => + sym.addAnnotation(annot) + case None => + } scalaSigAnnot } diff --git a/test/files/neg/t7014.check b/test/files/neg/t7014.check new file mode 100644 index 0000000000..07ad51e9d3 --- /dev/null +++ b/test/files/neg/t7014.check @@ -0,0 +1,5 @@ +warning: While parsing annotations in t7014-neg.obj/t7014/ThreadSafetyLevel.class, could not find COMPLETELY_THREADSAFE in enum object ThreadSafetyLevel. +This is likely due to an implementation restriction: an annotation argument cannot refer to a member of the annotated class (SI-7014). +error: No warnings can be incurred under -Xfatal-warnings. +one warning found +one error found diff --git a/test/files/neg/t7014.flags b/test/files/neg/t7014.flags new file mode 100644 index 0000000000..85d8eb2ba2 --- /dev/null +++ b/test/files/neg/t7014.flags @@ -0,0 +1 @@ +-Xfatal-warnings diff --git a/test/files/neg/t7014/ThreadSafety.java b/test/files/neg/t7014/ThreadSafety.java new file mode 100644 index 0000000000..ed508804e3 --- /dev/null +++ b/test/files/neg/t7014/ThreadSafety.java @@ -0,0 +1,9 @@ +package t7014; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +@Retention(RetentionPolicy.RUNTIME) // must be exactly RUNTIME retention (those we parse) +public @interface ThreadSafety { + ThreadSafetyLevel level(); +} \ No newline at end of file diff --git a/test/files/neg/t7014/ThreadSafetyLevel.java b/test/files/neg/t7014/ThreadSafetyLevel.java new file mode 100644 index 0000000000..4df1dc787a --- /dev/null +++ b/test/files/neg/t7014/ThreadSafetyLevel.java @@ -0,0 +1,8 @@ +package t7014; // package needed due to other bug in scalac's java parser + +// since we parse eagerly, we have not yet parsed the classfile when parsing the annotation, +// and on doing so, fail to find a symbol for the COMPLETELY_THREADSAFE reference +// from the annotation's argument to the enum's member +// for now, let's just not crash -- should implement lazy completing at some point +@ThreadSafety(level=ThreadSafetyLevel.COMPLETELY_THREADSAFE) +public enum ThreadSafetyLevel { COMPLETELY_THREADSAFE } diff --git a/test/files/neg/t7014/t7014.scala b/test/files/neg/t7014/t7014.scala new file mode 100644 index 0000000000..7c73f700be --- /dev/null +++ b/test/files/neg/t7014/t7014.scala @@ -0,0 +1,3 @@ +package t7014 + +import ThreadSafetyLevel.COMPLETELY_THREADSAFE // refer to annotation so it gets parsed diff --git a/test/files/pos/t5165b.flags b/test/files/pos/t5165b.flags new file mode 100644 index 0000000000..e8fb65d50c --- /dev/null +++ b/test/files/pos/t5165b.flags @@ -0,0 +1 @@ +-Xfatal-warnings \ No newline at end of file diff --git a/test/files/pos/t7014/ThreadSafety.java b/test/files/pos/t7014/ThreadSafety.java deleted file mode 100644 index ed508804e3..0000000000 --- a/test/files/pos/t7014/ThreadSafety.java +++ /dev/null @@ -1,9 +0,0 @@ -package t7014; - -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; - -@Retention(RetentionPolicy.RUNTIME) // must be exactly RUNTIME retention (those we parse) -public @interface ThreadSafety { - ThreadSafetyLevel level(); -} \ No newline at end of file diff --git a/test/files/pos/t7014/ThreadSafetyLevel.java b/test/files/pos/t7014/ThreadSafetyLevel.java deleted file mode 100644 index 4df1dc787a..0000000000 --- a/test/files/pos/t7014/ThreadSafetyLevel.java +++ /dev/null @@ -1,8 +0,0 @@ -package t7014; // package needed due to other bug in scalac's java parser - -// since we parse eagerly, we have not yet parsed the classfile when parsing the annotation, -// and on doing so, fail to find a symbol for the COMPLETELY_THREADSAFE reference -// from the annotation's argument to the enum's member -// for now, let's just not crash -- should implement lazy completing at some point -@ThreadSafety(level=ThreadSafetyLevel.COMPLETELY_THREADSAFE) -public enum ThreadSafetyLevel { COMPLETELY_THREADSAFE } diff --git a/test/files/pos/t7014/t7014.scala b/test/files/pos/t7014/t7014.scala deleted file mode 100644 index 7c73f700be..0000000000 --- a/test/files/pos/t7014/t7014.scala +++ /dev/null @@ -1,3 +0,0 @@ -package t7014 - -import ThreadSafetyLevel.COMPLETELY_THREADSAFE // refer to annotation so it gets parsed diff --git a/test/files/pos/t7551.flags b/test/files/pos/t7551.flags new file mode 100644 index 0000000000..e8fb65d50c --- /dev/null +++ b/test/files/pos/t7551.flags @@ -0,0 +1 @@ +-Xfatal-warnings \ No newline at end of file diff --git a/test/files/pos/t7551/A.java b/test/files/pos/t7551/A.java new file mode 100644 index 0000000000..72aeb40fa0 --- /dev/null +++ b/test/files/pos/t7551/A.java @@ -0,0 +1,9 @@ +package p; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +@Retention(RetentionPolicy.RUNTIME) +public @interface A { + Class subInterface(); +} diff --git a/test/files/pos/t7551/T.scala b/test/files/pos/t7551/T.scala new file mode 100644 index 0000000000..017926e0e2 --- /dev/null +++ b/test/files/pos/t7551/T.scala @@ -0,0 +1,9 @@ +package p + +@A(subInterface = classOf[T.S]) +trait T { +} + +object T { + private[p] trait S extends T { } +} diff --git a/test/files/pos/t7551/Test.scala b/test/files/pos/t7551/Test.scala new file mode 100644 index 0000000000..c1f529c4b1 --- /dev/null +++ b/test/files/pos/t7551/Test.scala @@ -0,0 +1,5 @@ +package p + +object Foo { + def bar(t: T) { } +} -- cgit v1.2.3