From 70503355299263f95a3447701bb483375bf46665 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Mon, 2 Jul 2012 17:25:18 +0200 Subject: Allow attachments for symbols, just like for trees. Removes the two global hash maps in Namers, and the one in NamesDefaults. Also fixes SI-5975. --- .../scala/tools/nsc/typechecker/Namers.scala | 73 ++++++++-------------- .../tools/nsc/typechecker/NamesDefaults.scala | 15 ++++- .../scala/tools/nsc/typechecker/Typers.scala | 1 - .../scala/tools/nsc/typechecker/Unapplies.scala | 8 +++ .../scala/reflect/internal/StdAttachments.scala | 17 ++++- src/reflect/scala/reflect/internal/Symbols.scala | 9 +-- src/reflect/scala/reflect/internal/Trees.scala | 11 +--- src/reflect/scala/reflect/makro/Universe.scala | 24 +++---- test/files/presentation/ide-t1000976.check | 1 + test/files/presentation/ide-t1000976.flags | 1 + test/files/presentation/ide-t1000976/Test.scala | 30 +++++++++ test/files/presentation/ide-t1000976/src/a/A.scala | 7 +++ test/files/presentation/ide-t1000976/src/b/B.scala | 7 +++ test/files/presentation/ide-t1000976/src/c/C.scala | 3 + test/files/presentation/ide-t1000976/src/d/D.scala | 7 +++ 15 files changed, 140 insertions(+), 74 deletions(-) create mode 100644 test/files/presentation/ide-t1000976.check create mode 100644 test/files/presentation/ide-t1000976.flags create mode 100644 test/files/presentation/ide-t1000976/Test.scala create mode 100644 test/files/presentation/ide-t1000976/src/a/A.scala create mode 100644 test/files/presentation/ide-t1000976/src/b/B.scala create mode 100644 test/files/presentation/ide-t1000976/src/c/C.scala create mode 100644 test/files/presentation/ide-t1000976/src/d/D.scala diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index decd18b599..0738bb51d1 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -52,27 +52,6 @@ trait Namers extends MethodSynthesis { def newNamerFor(context: Context, tree: Tree): Namer = newNamer(context.makeNewScope(tree, tree.symbol)) - // In the typeCompleter (templateSig) of a case class (resp it's module), - // synthetic `copy` (reps `apply`, `unapply`) methods are added. To compute - // their signatures, the corresponding ClassDef is needed. - // During naming, for each case class module symbol, the corresponding ClassDef - // is stored in this map. The map is cleared lazily, i.e. when the new symbol - // is created with the same name, the old one (if present) is wiped out, or the - // entry is deleted when it is used and no longer needed. - private val classOfModuleClass = perRunCaches.newWeakMap[Symbol, WeakReference[ClassDef]]() - - // Default getters of constructors are added to the companion object in the - // typeCompleter of the constructor (methodSig). To compute the signature, - // we need the ClassDef. To create and enter the symbols into the companion - // object, we need the templateNamer of that module class. - // This map is extended during naming of classes, the Namer is added in when - // it's available, i.e. in the type completer (templateSig) of the module class. - private[typechecker] val classAndNamerOfModule = perRunCaches.newMap[Symbol, (ClassDef, Namer)]() - - def resetNamer() { - classAndNamerOfModule.clear() - } - abstract class Namer(val context: Context) extends MethodSynth with NamerContextErrors { thisNamer => import NamerErrorGen._ @@ -621,7 +600,7 @@ trait Namers extends MethodSynthesis { MaxParametersCaseClassError(tree) val m = ensureCompanionObject(tree, caseModuleDef) - classOfModuleClass(m.moduleClass) = new WeakReference(tree) + m.moduleClass.addAttachment(new ClassForCaseCompanionAttachment(tree)) } val hasDefault = impl.body exists { case DefDef(_, nme.CONSTRUCTOR, _, vparamss, _, _) => mexists(vparamss)(_.mods.hasDefault) @@ -629,7 +608,7 @@ trait Namers extends MethodSynthesis { } if (hasDefault) { val m = ensureCompanionObject(tree) - classAndNamerOfModule(m) = (tree, null) + m.addAttachment(new ConstructorDefaultsAttachment(tree, null)) } val owner = tree.symbol.owner if (settings.lint.value && owner.isPackageObjectClass && !mods.isImplicit) { @@ -660,7 +639,8 @@ trait Namers extends MethodSynthesis { if (sym.isLazy) sym.lazyAccessor andAlso enterIfNotThere - defaultParametersOfMethod(sym) foreach { symRef => enterIfNotThere(symRef()) } + for (defAtt <- sym.attachments.get[DefaultsOfLocalMethodAttachment]) + defAtt.defaultGetters foreach enterIfNotThere } this.context } @@ -849,23 +829,20 @@ trait Namers extends MethodSynthesis { // add apply and unapply methods to companion objects of case classes, // unless they exist already; here, "clazz" is the module class if (clazz.isModuleClass) { - Namers.this.classOfModuleClass get clazz foreach { cdefRef => - val cdef = cdefRef() - if (cdef.mods.isCase) addApplyUnapply(cdef, templateNamer) - classOfModuleClass -= clazz + clazz.attachments.get[ClassForCaseCompanionAttachment] foreach { cma => + val cdef = cma.caseClass + assert(cdef.mods.isCase, "expected case class: "+ cdef) + addApplyUnapply(cdef, templateNamer) } } // add the copy method to case classes; this needs to be done here, not in SyntheticMethods, because // the namer phase must traverse this copy method to create default getters for its parameters. // here, clazz is the ClassSymbol of the case class (not the module). - // @check: this seems to work only if the type completer of the class runs before the one of the - // module class: the one from the module class removes the entry from classOfModuleClass (see above). if (clazz.isClass && !clazz.hasModuleFlag) { val modClass = companionSymbolOf(clazz, context).moduleClass - Namers.this.classOfModuleClass get modClass map { cdefRef => - val cdef = cdefRef() - + modClass.attachments.get[ClassForCaseCompanionAttachment] foreach { cma => + val cdef = cma.caseClass def hasCopy(decls: Scope) = (decls lookup nme.copy) != NoSymbol if (cdef.mods.isCase && !hasCopy(decls) && !parents.exists(p => hasCopy(p.typeSymbol.info.decls)) && @@ -877,9 +854,8 @@ trait Namers extends MethodSynthesis { // if default getters (for constructor defaults) need to be added to that module, here's the namer // to use. clazz is the ModuleClass. sourceModule works also for classes defined in methods. val module = clazz.sourceModule - classAndNamerOfModule get module foreach { - case (cdef, _) => - classAndNamerOfModule(module) = (cdef, templateNamer) + for (cda <- module.attachments.get[ConstructorDefaultsAttachment]) { + cda.companionModuleClassNamer = templateNamer } ClassInfoType(parents, decls, clazz) } @@ -1100,13 +1076,15 @@ trait Namers extends MethodSynthesis { val module = companionSymbolOf(clazz, context) module.initialize // call type completer (typedTemplate), adds the // module's templateNamer to classAndNamerOfModule - classAndNamerOfModule get module match { - case s @ Some((cdef, nmr)) if nmr != null => - moduleNamer = s - (cdef, nmr) + module.attachments.get[ConstructorDefaultsAttachment] match { + // by martin: the null case can happen in IDE; this is really an ugly hack on top of an ugly hack but it seems to work + // later by lukas: disabled when fixing SI-5975, i think it cannot happen anymore + case Some(cda) /*if cma.companionModuleClassNamer == null*/ => + val p = (cda.classWithDefault, cda.companionModuleClassNamer) + moduleNamer = Some(p) + p case _ => return // fix #3649 (prevent crash in erroneous source code) - // nmr == null can happen in IDE; this is really an ugly hack on top[ of an ugly hack but it seems to work } } deftParams = cdef.tparams map copyUntypedInvariant @@ -1144,11 +1122,14 @@ trait Namers extends MethodSynthesis { clazz.resetFlag(INTERFACE) // there's a concrete member now val default = parentNamer.enterSyntheticSym(defaultTree) if (forInteractive && default.owner.isTerm) { - // enter into map from method symbols to default arguments. - // if compiling the same local block several times (which can happen in interactive mode) - // we might otherwise not find the default symbol, because the second time it the - // method symbol will be re-entered in the scope but the default parameter will not. - defaultParametersOfMethod(meth) += new WeakReference(default) + // save the default getters as attachments in the method symbol. if compiling the + // same local block several times (which can happen in interactive mode) we might + // otherwise not find the default symbol, because the second time it the method + // symbol will be re-entered in the scope but the default parameter will not. + val att = meth.attachments.get[DefaultsOfLocalMethodAttachment] match { + case Some(att) => att.defaultGetters += default + case None => meth.addAttachment(new DefaultsOfLocalMethodAttachment(default)) + } } } else if (baseHasDefault) { // the parameter does not have a default itself, but the diff --git a/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala b/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala index 61443faba0..a0c1342026 100644 --- a/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala +++ b/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala @@ -21,8 +21,19 @@ trait NamesDefaults { self: Analyzer => import definitions._ import NamesDefaultsErrorsGen._ - val defaultParametersOfMethod = - perRunCaches.newWeakMap[Symbol, Set[WeakReference[Symbol]]]() withDefaultValue Set() + // Default getters of constructors are added to the companion object in the + // typeCompleter of the constructor (methodSig). To compute the signature, + // we need the ClassDef. To create and enter the symbols into the companion + // object, we need the templateNamer of that module class. These two are stored + // as an attachment in the companion module symbol + class ConstructorDefaultsAttachment(val classWithDefault: ClassDef, var companionModuleClassNamer: Namer) + + // To attach the default getters of local (term-owned) methods to the method symbol. + // Used in Namer.enterExistingSym: it needs to re-enter the method symbol and also + // default getters, which could not be found otherwise. + class DefaultsOfLocalMethodAttachment(val defaultGetters: mutable.Set[Symbol]) { + def this(default: Symbol) = this(mutable.Set(default)) + } case class NamedApplyInfo( qual: Option[Tree], diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 52cce11deb..f44b134fc6 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -47,7 +47,6 @@ trait Typers extends Modes with Adaptations with Tags { def resetTyper() { //println("resetTyper called") resetContexts() - resetNamer() resetImplicits() transformed.clear() } diff --git a/src/compiler/scala/tools/nsc/typechecker/Unapplies.scala b/src/compiler/scala/tools/nsc/typechecker/Unapplies.scala index 4c20d14406..e70a6685bd 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Unapplies.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Unapplies.scala @@ -23,6 +23,14 @@ trait Unapplies extends ast.TreeDSL private val unapplyParamName = nme.x_0 + + // In the typeCompleter (templateSig) of a case class (resp it's module), + // synthetic `copy` (reps `apply`, `unapply`) methods are added. To compute + // their signatures, the corresponding ClassDef is needed. During naming (in + // `enterClassDef`), the case class ClassDef is added as an attachment to the + // moduleClass symbol of the companion module. + class ClassForCaseCompanionAttachment(val caseClass: ClassDef) + /** returns type list for return type of the extraction */ def unapplyTypeList(ufn: Symbol, ufntpe: Type) = { assert(ufn.isMethod, ufn) diff --git a/src/reflect/scala/reflect/internal/StdAttachments.scala b/src/reflect/scala/reflect/internal/StdAttachments.scala index 4ea9b27da9..60b3a6f436 100644 --- a/src/reflect/scala/reflect/internal/StdAttachments.scala +++ b/src/reflect/scala/reflect/internal/StdAttachments.scala @@ -4,9 +4,24 @@ package internal trait StdAttachments { self: SymbolTable => + /** + * Common code between reflect-internal Symbol and Tree related to Attachments. + */ + trait Attachable { + protected var rawatt: base.Attachments { type Pos = Position } = NoPosition + def attachments = rawatt + def addAttachment(attachment: Any): this.type = { rawatt = rawatt.add(attachment); this } + def removeAttachment[T: ClassTag]: this.type = { rawatt = rawatt.remove[T]; this } + + // cannot be final due to SynchronizedSymbols + def pos: Position = rawatt.pos + def pos_=(pos: Position): Unit = rawatt = (rawatt withPos pos) + def setPos(newpos: Position): this.type = { pos = newpos; this } + } + case object BackquotedIdentifierAttachment case class CompoundTypeTreeOriginalAttachment(parents: List[Tree], stats: List[Tree]) case class MacroExpansionAttachment(original: Tree) -} \ No newline at end of file +} diff --git a/src/reflect/scala/reflect/internal/Symbols.scala b/src/reflect/scala/reflect/internal/Symbols.scala index 79041924a8..f0c6252dd9 100644 --- a/src/reflect/scala/reflect/internal/Symbols.scala +++ b/src/reflect/scala/reflect/internal/Symbols.scala @@ -10,6 +10,7 @@ import scala.collection.{ mutable, immutable } import scala.collection.mutable.ListBuffer import util.Statistics import Flags._ +import base.Attachments trait Symbols extends api.Symbols { self: SymbolTable => import definitions._ @@ -154,7 +155,8 @@ trait Symbols extends api.Symbols { self: SymbolTable => abstract class Symbol protected[Symbols] (initOwner: Symbol, initPos: Position, initName: Name) extends SymbolContextApiImpl with HasFlags - with Annotatable[Symbol] { + with Annotatable[Symbol] + with Attachable { type AccessBoundaryType = Symbol type AnnotationType = AnnotationInfo @@ -176,7 +178,7 @@ trait Symbols extends api.Symbols { self: SymbolTable => def rawowner = _rawowner def rawflags = _rawflags - private var rawpos = initPos + rawatt = initPos val id = nextId() // identity displayed when -uniqid //assert(id != 3390, initName) @@ -189,8 +191,6 @@ trait Symbols extends api.Symbols { self: SymbolTable => def validTo = _validTo def validTo_=(x: Period) { _validTo = x} - def pos = rawpos - def setPos(pos: Position): this.type = { this.rawpos = pos; this } def setName(name: Name): this.type = { this.name = asNameType(name) ; this } // Update the surrounding scopes @@ -1616,6 +1616,7 @@ trait Symbols extends api.Symbols { self: SymbolTable => setInfo (this.info cloneInfo clone) setAnnotations this.annotations ) + this.attachments.all.foreach(clone.addAttachment) if (clone.thisSym != clone) clone.typeOfThis = (clone.typeOfThis cloneInfo clone) diff --git a/src/reflect/scala/reflect/internal/Trees.scala b/src/reflect/scala/reflect/internal/Trees.scala index dd13dd4c4c..e92d644f4a 100644 --- a/src/reflect/scala/reflect/internal/Trees.scala +++ b/src/reflect/scala/reflect/internal/Trees.scala @@ -15,20 +15,13 @@ trait Trees extends api.Trees { self: SymbolTable => private[scala] var nodeCount = 0 - abstract class Tree extends TreeContextApiImpl with Product { + abstract class Tree extends TreeContextApiImpl with Attachable with Product { val id = nodeCount // TODO: add to attachment? nodeCount += 1 Statistics.incCounter(TreesStats.nodeByType, getClass) - @inline final def pos: Position = rawatt.pos - def pos_=(pos: Position): Unit = rawatt = (rawatt withPos pos) - def setPos(newpos: Position): this.type = { pos = newpos; this } - - private var rawatt: Attachments { type Pos = Position } = NoPosition - def attachments = rawatt - def addAttachment(attachment: Any): this.type = { rawatt = rawatt.add(attachment); this } - def removeAttachment[T: ClassTag]: this.type = { rawatt = rawatt.remove[T]; this } + @inline final override def pos: Position = rawatt.pos private[this] var rawtpe: Type = _ @inline final def tpe = rawtpe diff --git a/src/reflect/scala/reflect/makro/Universe.scala b/src/reflect/scala/reflect/makro/Universe.scala index 98046be555..a676f7f1de 100644 --- a/src/reflect/scala/reflect/makro/Universe.scala +++ b/src/reflect/scala/reflect/makro/Universe.scala @@ -5,13 +5,24 @@ abstract class Universe extends scala.reflect.api.Universe { val treeBuild: TreeBuilder { val global: Universe.this.type } + trait AttachableApi { + /** ... */ + def attachments: base.Attachments { type Pos = Position } + + /** ... */ + def addAttachment(attachment: Any): AttachableApi.this.type + + /** ... */ + def removeAttachment[T: ClassTag]: AttachableApi.this.type + } + // Symbol extensions --------------------------------------------------------------- override type Symbol >: Null <: SymbolContextApi /** The extended API of symbols that's supported in macro context universes */ - trait SymbolContextApi extends SymbolApi { this: Symbol => + trait SymbolContextApi extends SymbolApi with AttachableApi { this: Symbol => // [Eugene++ to Martin] should we also add mutability methods here (similarly to what's done below for trees)? // I'm talking about `setAnnotations` and friends @@ -23,7 +34,7 @@ abstract class Universe extends scala.reflect.api.Universe { /** The extended API of trees that's supported in macro context universes */ - trait TreeContextApi extends TreeApi { this: Tree => + trait TreeContextApi extends TreeApi with AttachableApi { this: Tree => /** ... */ def pos_=(pos: Position): Unit @@ -62,15 +73,6 @@ abstract class Universe extends scala.reflect.api.Universe { /** ... */ def setSymbol(sym: Symbol): this.type - - /** ... */ - def attachments: base.Attachments { type Pos = Position } - - /** ... */ - def addAttachment(attachment: Any): this.type - - /** ... */ - def removeAttachment[T: ClassTag]: this.type } override type SymTree >: Null <: Tree with SymTreeContextApi diff --git a/test/files/presentation/ide-t1000976.check b/test/files/presentation/ide-t1000976.check new file mode 100644 index 0000000000..d58f86d6c6 --- /dev/null +++ b/test/files/presentation/ide-t1000976.check @@ -0,0 +1 @@ +Test OK \ No newline at end of file diff --git a/test/files/presentation/ide-t1000976.flags b/test/files/presentation/ide-t1000976.flags new file mode 100644 index 0000000000..9a1a05a4f6 --- /dev/null +++ b/test/files/presentation/ide-t1000976.flags @@ -0,0 +1 @@ +-sourcepath src \ No newline at end of file diff --git a/test/files/presentation/ide-t1000976/Test.scala b/test/files/presentation/ide-t1000976/Test.scala new file mode 100644 index 0000000000..722259d3a1 --- /dev/null +++ b/test/files/presentation/ide-t1000976/Test.scala @@ -0,0 +1,30 @@ +import scala.tools.nsc.interactive.tests.InteractiveTest +import scala.reflect.internal.util.SourceFile +import scala.tools.nsc.interactive.Response + +object Test extends InteractiveTest { + override def execute(): Unit = { + loadSourceAndWaitUntilTypechecked("A.scala") + val sourceB = loadSourceAndWaitUntilTypechecked("B.scala") + checkErrors(sourceB) + } + + private def loadSourceAndWaitUntilTypechecked(sourceName: String): SourceFile = { + val sourceFile = sourceFiles.find(_.file.name == sourceName).head + compiler.askToDoFirst(sourceFile) + val res = new Response[Unit] + compiler.askReload(List(sourceFile), res) + res.get + askLoadedTyped(sourceFile).get + sourceFile + } + + private def checkErrors(source: SourceFile): Unit = compiler.getUnitOf(source) match { + case Some(unit) => + val problems = unit.problems.toList + if(problems.isEmpty) reporter.println("Test OK") + else problems.foreach(problem => reporter.println(problem.msg)) + + case None => reporter.println("No compilation unit found for " + source.file.name) + } +} diff --git a/test/files/presentation/ide-t1000976/src/a/A.scala b/test/files/presentation/ide-t1000976/src/a/A.scala new file mode 100644 index 0000000000..fcfef8b525 --- /dev/null +++ b/test/files/presentation/ide-t1000976/src/a/A.scala @@ -0,0 +1,7 @@ +package a + +import d.D._ + +object A { + Seq.empty[Byte].toArray.toSeq +} diff --git a/test/files/presentation/ide-t1000976/src/b/B.scala b/test/files/presentation/ide-t1000976/src/b/B.scala new file mode 100644 index 0000000000..628348cac1 --- /dev/null +++ b/test/files/presentation/ide-t1000976/src/b/B.scala @@ -0,0 +1,7 @@ +package b + +import c.C + +class B { + new C("") +} diff --git a/test/files/presentation/ide-t1000976/src/c/C.scala b/test/files/presentation/ide-t1000976/src/c/C.scala new file mode 100644 index 0000000000..cc23e3eef1 --- /dev/null +++ b/test/files/presentation/ide-t1000976/src/c/C.scala @@ -0,0 +1,3 @@ +package c + +class C(key: String = "", componentStates: String = "") diff --git a/test/files/presentation/ide-t1000976/src/d/D.scala b/test/files/presentation/ide-t1000976/src/d/D.scala new file mode 100644 index 0000000000..d7a48f98d5 --- /dev/null +++ b/test/files/presentation/ide-t1000976/src/d/D.scala @@ -0,0 +1,7 @@ +package d + +import c.C + +object D { + implicit def c2s(c: C): String = "" +} -- cgit v1.2.3