From 25ecde037f22ff92df5459aaa5360a9760f1f05f Mon Sep 17 00:00:00 2001 From: Paul Phillips Date: Wed, 23 Nov 2011 03:11:53 +0000 Subject: Reworked AnnotationInfo patch. Took a more ambitious swing based on input from martin. Eliminated the external map and gave annotations a more useful inheritance hierarchy. Eliminated AnnotationInfoBase and made LazyAnnotationInfo an AnnotationInfo (just like LazyType is a Type.) Review by odersky. --- .../scala/reflect/internal/AnnotationInfos.scala | 79 ++++++++++++++-------- src/compiler/scala/reflect/internal/Symbols.scala | 39 ++--------- .../scala/reflect/internal/TreePrinters.scala | 8 +-- .../scala/tools/nsc/backend/jvm/GenJVM.scala | 14 +--- .../nsc/symtab/classfile/ClassfileParser.scala | 5 +- .../scala/tools/nsc/symtab/classfile/Pickler.scala | 29 ++++---- .../tools/nsc/typechecker/MethodSynthesis.scala | 2 +- .../scala/tools/nsc/typechecker/Namers.scala | 8 +-- test/files/neg/t3222.check | 14 ++-- 9 files changed, 84 insertions(+), 114 deletions(-) diff --git a/src/compiler/scala/reflect/internal/AnnotationInfos.scala b/src/compiler/scala/reflect/internal/AnnotationInfos.scala index a86853b453..68d487f520 100644 --- a/src/compiler/scala/reflect/internal/AnnotationInfos.scala +++ b/src/compiler/scala/reflect/internal/AnnotationInfos.scala @@ -11,11 +11,7 @@ import pickling.ByteCodecs /** AnnotationInfo and its helpers */ trait AnnotationInfos extends api.AnnotationInfos { self: SymbolTable => - import definitions.{ ThrowsClass, isMetaAnnotation } - - // Annotations which are en route from Modifiers to a Symbol. - // They are removed from this map when the Symbol comes to claim them. - val pendingSymbolAnnotations = perRunCaches.newMap[Symbol, List[AnnotationInfoBase]]() + import definitions.{ ThrowsClass, StaticAnnotationClass, isMetaAnnotation } // Common annotation code between Symbol and Type. // For methods altering the annotation list, on Symbol it mutates @@ -90,8 +86,7 @@ trait AnnotationInfos extends api.AnnotationInfos { self: SymbolTable => } /** Represents a nested classfile annotation */ - case class NestedAnnotArg(annInfo: AnnotationInfo) - extends ClassfileAnnotArg { + case class NestedAnnotArg(annInfo: AnnotationInfo) extends ClassfileAnnotArg { // The nested annotation should not have any Scala annotation arguments assert(annInfo.args.isEmpty, annInfo.args) override def toString = annInfo.toString @@ -99,7 +94,46 @@ trait AnnotationInfos extends api.AnnotationInfos { self: SymbolTable => object NestedAnnotArg extends NestedAnnotArgExtractor - class AnnotationInfoBase + object AnnotationInfo extends AnnotationInfoExtractor { + def lazily(lazyInfo: => AnnotationInfo) = + new LazyAnnotationInfo(lazyInfo) + + def apply(atp: Type, args: List[Tree], assocs: List[(Name, ClassfileAnnotArg)]): AnnotationInfo = + new CompleteAnnotationInfo(atp, args, assocs) + + def unapply(info: AnnotationInfo): Option[(Type, List[Tree], List[(Name, ClassfileAnnotArg)])] = + Some((info.atp, info.args, info.assocs)) + } + + class CompleteAnnotationInfo( + val atp: Type, + val args: List[Tree], + val assocs: List[(Name, ClassfileAnnotArg)] + ) extends AnnotationInfo { + // Classfile annot: args empty. Scala annot: assocs empty. + assert(args.isEmpty || assocs.isEmpty, atp) + + override def toString = ( + atp + + (if (!args.isEmpty) args.mkString("(", ", ", ")") else "") + + (if (!assocs.isEmpty) (assocs map { case (x, y) => x+" = "+y } mkString ("(", ", ", ")")) else "") + ) + } + + /** Symbol annotations parsed in `Namer` (typeCompleter of + * definitions) have to be lazy (#1782) + */ + final class LazyAnnotationInfo(lazyInfo: => AnnotationInfo) extends AnnotationInfo { + private var forced = false + private lazy val forcedInfo = try lazyInfo finally forced = true + + def atp: Type = forcedInfo.atp + def args: List[Tree] = forcedInfo.args + def assocs: List[(Name, ClassfileAnnotArg)] = forcedInfo.assocs + + // We should always be able to print things without forcing them. + override def toString = if (forced) forcedInfo.toString else "@" + } /** Typed information about an annotation. It can be attached to either * a symbol or an annotated type. @@ -117,12 +151,13 @@ trait AnnotationInfos extends api.AnnotationInfos { self: SymbolTable => * * `assocs` stores arguments to classfile annotations as name-value pairs. */ - case class AnnotationInfo(atp: Type, args: List[Tree], - assocs: List[(Name, ClassfileAnnotArg)]) - extends AnnotationInfoBase { + sealed abstract class AnnotationInfo { + def atp: Type + def args: List[Tree] + def assocs: List[(Name, ClassfileAnnotArg)] - // Classfile annot: args empty. Scala annot: assocs empty. - assert(args.isEmpty || assocs.isEmpty, atp) + // see annotationArgRewriter + lazy val isTrivial = atp.isTrivial && !hasArgWhich(_.isInstanceOf[This]) private var rawpos: Position = NoPosition def pos = rawpos @@ -169,15 +204,11 @@ trait AnnotationInfos extends api.AnnotationInfos { self: SymbolTable => // All subtrees of all args are considered. def hasArgWhich(p: Tree => Boolean) = args exists (_ exists p) - lazy val isTrivial: Boolean = atp.isTrivial && !hasArgWhich(_.isInstanceOf[This]) // see annotationArgRewriter - - override def toString: String = atp + - (if (!args.isEmpty) args.mkString("(", ", ", ")") else "") + - (if (!assocs.isEmpty) (assocs map { case (x, y) => x+" = "+y } mkString ("(", ", ", ")")) else "") - /** Check whether the type or any of the arguments are erroneous */ def isErroneous = atp.isErroneous || args.exists(_.isErroneous) + def isStatic = symbol isNonBottomSubClass StaticAnnotationClass + /** Check whether any of the arguments mention a symbol */ def refsSymbol(sym: Symbol) = hasArgWhich(_.symbol == sym) @@ -204,16 +235,8 @@ trait AnnotationInfos extends api.AnnotationInfos { self: SymbolTable => if (index < args.size) Some(args(index)) else None } - object AnnotationInfo extends AnnotationInfoExtractor - lazy val classfileAnnotArgManifest: ClassManifest[ClassfileAnnotArg] = reflect.ClassManifest.classType(classOf[ClassfileAnnotArg]) - /** Symbol annotations parsed in `Namer` (typeCompleter of - * definitions) have to be lazy (#1782) - */ - case class LazyAnnotationInfo(annot: () => AnnotationInfo) - extends AnnotationInfoBase - - object UnmappableAnnotation extends AnnotationInfo(NoType, Nil, Nil) + object UnmappableAnnotation extends CompleteAnnotationInfo(NoType, Nil, Nil) } diff --git a/src/compiler/scala/reflect/internal/Symbols.scala b/src/compiler/scala/reflect/internal/Symbols.scala index d744cff75a..2fc3eb3652 100644 --- a/src/compiler/scala/reflect/internal/Symbols.scala +++ b/src/compiler/scala/reflect/internal/Symbols.scala @@ -1181,56 +1181,27 @@ trait Symbols extends api.Symbols { self: SymbolTable => // ----- annotations ------------------------------------------------------------ // null is a marker that they still need to be obtained. - private var _annotations: List[AnnotationInfo] = null - // Namer has stored the annotations waiting for us to come calling. - private def obtainAnnotations() { - // .initialize: the type completer of the symbol parses the annotations, - // see "def typeSig" in Namers. - initialize - _annotations = pendingSymbolAnnotations remove this match { - case Some(rawAnnots) => - rawAnnots map { - case x: LazyAnnotationInfo => x.annot() - case x: AnnotationInfo => x - } filterNot (_.atp.isError) - case _ => - Nil - } - } - // Gets _annotations without forcing initialization/obtainment. - def rawAnnotations = if (_annotations eq null) Nil else _annotations - // Used in namer to check whether annotations were already assigned or not. - def hasAssignedAnnotations = (_annotations ne null) && _annotations.nonEmpty + private var _annotations: List[AnnotationInfo] = Nil - @deprecated("This method will be removed", "2.10.0") - def setRawAnnotations(annots: List[AnnotationInfoBase]): this.type = { - // Just in case this is still in use somewhere. - pendingSymbolAnnotations(this) = annots - _annotations = null - this - } + def annotationsString = if (annotations.isEmpty) "" else annotations.mkString("(", ", ", ")") /** After the typer phase (before, look at the definition's Modifiers), contains * the annotations attached to member a definition (class, method, type, field). */ - def annotations: List[AnnotationInfo] = { - if (_annotations eq null) - obtainAnnotations() - _annotations - } + def annotations: List[AnnotationInfo] = _annotations def setAnnotations(annots: List[AnnotationInfo]): this.type = { _annotations = annots this } def withAnnotations(annots: List[AnnotationInfo]): this.type = - setAnnotations(annots ::: rawAnnotations) + setAnnotations(annots ::: annotations) def withoutAnnotations: this.type = setAnnotations(Nil) def addAnnotation(annot: AnnotationInfo): this.type = - setAnnotations(annot :: rawAnnotations) + setAnnotations(annot :: annotations) // Convenience for the overwhelmingly common case def addAnnotation(sym: Symbol, args: Tree*): this.type = diff --git a/src/compiler/scala/reflect/internal/TreePrinters.scala b/src/compiler/scala/reflect/internal/TreePrinters.scala index c86cd9b29c..dcc395ddd2 100644 --- a/src/compiler/scala/reflect/internal/TreePrinters.scala +++ b/src/compiler/scala/reflect/internal/TreePrinters.scala @@ -149,10 +149,10 @@ trait TreePrinters extends api.TreePrinters { self: SymbolTable => } def printAnnotations(tree: Tree) { - val annots = - if (tree.symbol.hasAssignedAnnotations) tree.symbol.annotations - else tree.asInstanceOf[MemberDef].mods.annotations - + val annots = tree.symbol.annotations match { + case Nil => tree.asInstanceOf[MemberDef].mods.annotations + case anns => anns + } annots foreach (annot => print("@"+annot+" ")) } diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala index 37dfd10b02..9d5cc9e0a6 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala @@ -860,7 +860,7 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid with } addGenericSignature(jmethod, m.symbol, clasz.symbol) - val (excs, others) = splitAnnotations(m.symbol.annotations, ThrowsClass) + val (excs, others) = m.symbol.annotations partition (_.symbol == ThrowsClass) addExceptionsAttribute(jmethod, excs) addAnnotations(jmethod, others) addParamAnnotations(jmethod, m.params.map(_.sym.annotations)) @@ -888,16 +888,6 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid with } } - /** Return a pair of lists of annotations, first one containing all - * annotations for the given symbol, and the rest. - */ - private def splitAnnotations(annotations: List[AnnotationInfo], annotSym: Symbol): (List[AnnotationInfo], List[AnnotationInfo]) = { - annotations.partition { a => a match { - case AnnotationInfo(tp, _, _) if tp.typeSymbol == annotSym => true - case _ => false - }} - } - private def isClosureApply(sym: Symbol): Boolean = { (sym.name == nme.apply) && sym.owner.isSynthetic && @@ -1050,7 +1040,7 @@ abstract class GenJVM extends SubComponent with GenJVMUtil with GenAndroid with if (!m.isDeferred) addGenericSignature(mirrorMethod, m, module) - val (throws, others) = splitAnnotations(m.annotations, ThrowsClass) + val (throws, others) = m.annotations partition (_.symbol == ThrowsClass) addExceptionsAttribute(mirrorMethod, throws) addAnnotations(mirrorMethod, others) addParamAnnotations(mirrorMethod, m.info.params.map(_.annotations)) diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala index 7dc9b25015..ef1bcdd739 100644 --- a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala +++ b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala @@ -905,10 +905,7 @@ abstract class ClassfileParser { case None => throw new RuntimeException("Scala class file does not contain Scala annotation") } - debuglog("[class] << " + sym.fullName + ( - if (sym.rawAnnotations.isEmpty) "" - else sym.rawAnnotations.mkString("(", ", ", ")")) - ) + debuglog("[class] << " + sym.fullName + sym.annotationsString) } else in.skip(attrLen) diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala b/src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala index 89a770310f..e7d08ef849 100644 --- a/src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala +++ b/src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala @@ -106,10 +106,6 @@ abstract class Pickler extends SubComponent { sym.isParameter || isLocal(sym.owner)) - private def staticAnnotations(annots: List[AnnotationInfo]) = - annots filter(ann => - ann.symbol isNonBottomSubClass definitions.StaticAnnotationClass) - // Phase 1 methods: Populate entries/index ------------------------------------ /** Store entry e in index at next available position unless @@ -156,9 +152,10 @@ abstract class Pickler extends SubComponent { } putChildren(sym, children.toList sortBy (_.sealedSortName)) } - for (annot <- staticAnnotations(sym.annotations.reverse)) + for (annot <- sym.annotations filter (ann => ann.isStatic && !ann.isErroneous) reverse) putAnnotation(sym, annot) - } else if (sym != NoSymbol) { + } + else if (sym != NoSymbol) { putEntry(if (sym.isModuleClass) sym.name.toTermName else sym.name) if (!sym.owner.isRoot) putSymbol(sym.owner) } @@ -220,7 +217,7 @@ abstract class Pickler extends SubComponent { case AnnotatedType(annotations, underlying, selfsym) => putType(underlying) if (settings.selfInAnnots.value) putSymbol(selfsym) - putAnnotations(staticAnnotations(annotations)) + putAnnotations(annotations filter (_.isStatic)) case _ => throw new FatalError("bad type: " + tp + "(" + tp.getClass + ")") } @@ -616,17 +613,15 @@ abstract class Pickler extends SubComponent { else if (c.tag == EnumTag) writeRef(c.symbolValue) LITERAL + c.tag // also treats UnitTag, NullTag; no value required case AnnotatedType(annotations, tp, selfsym) => - val staticAnnots = staticAnnotations(annotations) - if (staticAnnots isEmpty) { - writeBody(tp) // write the underlying type if there are no annotations - } else { - if (settings.selfInAnnots.value && selfsym != NoSymbol) - writeRef(selfsym) - writeRef(tp) - writeRefs(staticAnnots) - ANNOTATEDtpe + annotations filter (_.isStatic) match { + case Nil => writeBody(tp) // write the underlying type if there are no annotations + case staticAnnots => + if (settings.selfInAnnots.value && selfsym != NoSymbol) + writeRef(selfsym) + writeRef(tp) + writeRefs(staticAnnots) + ANNOTATEDtpe } - // annotations attached to a symbol (i.e. annots on terms) case (target: Symbol, annot@AnnotationInfo(_, _, _)) => writeRef(target) diff --git a/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala b/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala index 0d93d982d2..d75e119fd7 100644 --- a/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala +++ b/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala @@ -74,7 +74,7 @@ trait MethodSynthesis { def finishGetterSetter(typer: Typer, stat: Tree): List[Tree] = stat match { case vd @ ValDef(mods, name, tpt, rhs) if !noFinishGetterSetter(vd) => // If we don't save the annotations, they seem to wander off. - val annotations = stat.symbol.annotations + val annotations = stat.symbol.initialize.annotations val trees = ( allValDefDerived(vd) map (acc => atPos(vd.pos.focus)(acc derive annotations)) diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index 960919905d..d503371f5d 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -1189,14 +1189,14 @@ trait Namers extends MethodSynthesis { // parse the annotations only once. if (!annotated.isInitialized) tree match { case defn: MemberDef => - val ainfos = defn.mods.annotations filter { _ != null } map { ann => + val ainfos = defn.mods.annotations filterNot (_ eq null) map { ann => // need to be lazy, #1782 - LazyAnnotationInfo(() => typer.typedAnnotation(ann)) + AnnotationInfo lazily (typer typedAnnotation ann) } if (ainfos.nonEmpty) { - pendingSymbolAnnotations(annotated) = ainfos + annotated setAnnotations ainfos if (annotated.isTypeSkolem) - pendingSymbolAnnotations(annotated.deSkolemize) = ainfos + annotated.deSkolemize setAnnotations ainfos } case _ => } diff --git a/test/files/neg/t3222.check b/test/files/neg/t3222.check index 6170827cc9..e724024f45 100644 --- a/test/files/neg/t3222.check +++ b/test/files/neg/t3222.check @@ -1,13 +1,7 @@ -t3222.scala:1: error: not found: type B -@throws(classOf[B]) - ^ t3222.scala:4: error: not found: type D def foo(@throws(classOf[D]) x: Int) {} ^ -t3222.scala:3: error: not found: type C - @throws(classOf[C]) - ^ -t3222.scala:6: error: not found: type E - @throws(classOf[E]) - ^ -four errors found +t3222.scala:1: error: not found: type B +@throws(classOf[B]) + ^ +two errors found -- cgit v1.2.3