diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2014-02-09 19:06:59 +0100 |
---|---|---|
committer | Jason Zaugg <jzaugg@gmail.com> | 2014-02-09 19:06:59 +0100 |
commit | 9dc60af28e2c17579c17e27168d1e29ef34efaef (patch) | |
tree | 02afe2a92dbff538e42a922837b57860ac527462 /src/compiler | |
parent | 08e51dfec50842253afb87cc5ae3c7400dc18ced (diff) | |
parent | 1adb3379c7ac568317db5739f1d6df628f58b65d (diff) | |
download | scala-9dc60af28e2c17579c17e27168d1e29ef34efaef.tar.gz scala-9dc60af28e2c17579c17e27168d1e29ef34efaef.tar.bz2 scala-9dc60af28e2c17579c17e27168d1e29ef34efaef.zip |
Merge pull request #3485 from xeno-by/topic/reset-all-attrs
kills resetAllAttrs
Diffstat (limited to 'src/compiler')
11 files changed, 47 insertions, 71 deletions
diff --git a/src/compiler/scala/reflect/macros/contexts/Typers.scala b/src/compiler/scala/reflect/macros/contexts/Typers.scala index cd3db74016..c1ab17027f 100644 --- a/src/compiler/scala/reflect/macros/contexts/Typers.scala +++ b/src/compiler/scala/reflect/macros/contexts/Typers.scala @@ -46,9 +46,7 @@ trait Typers { universe.analyzer.inferImplicit(tree, viewTpe, true, callsiteTyper.context, silent, withMacrosDisabled, pos, (pos, msg) => throw TypecheckException(pos, msg)) } - def resetAllAttrs(tree: Tree): Tree = universe.resetAllAttrs(universe.duplicateAndKeepPositions(tree)) + def resetLocalAttrs(tree: Tree): Tree = universe.resetAttrs(universe.duplicateAndKeepPositions(tree)) - def resetLocalAttrs(tree: Tree): Tree = universe.resetLocalAttrs(universe.duplicateAndKeepPositions(tree)) - - def untypecheck(tree: Tree): Tree = universe.resetLocalAttrs(universe.duplicateAndKeepPositions(tree)) + def untypecheck(tree: Tree): Tree = resetLocalAttrs(tree) } diff --git a/src/compiler/scala/reflect/reify/Reifier.scala b/src/compiler/scala/reflect/reify/Reifier.scala index ad0632f93e..6b0a0ee8d7 100644 --- a/src/compiler/scala/reflect/reify/Reifier.scala +++ b/src/compiler/scala/reflect/reify/Reifier.scala @@ -86,7 +86,7 @@ abstract class Reifier extends States throw new Error("reifee %s of type %s is not supported".format(reifee, if (reifee == null) "null" else reifee.getClass.toString)) } - // todo. why do we resetAllAttrs? + // todo. why do we reset attrs? // // typically we do some preprocessing before reification and // the code emitted/moved around during preprocessing is very hard to typecheck, so we leave it as it is @@ -109,19 +109,11 @@ abstract class Reifier extends States // // todo. this is a common problem with non-trivial macros in our current macro system // needs to be solved some day - // maybe try `resetLocalAttrs` once the dust settles - var importantSymbols = Set[Symbol]( - NothingClass, AnyClass, SingletonClass, PredefModule, ScalaRunTimeModule, TypeCreatorClass, TreeCreatorClass, MirrorClass, - ApiUniverseClass, JavaUniverseClass, ReflectRuntimePackage, runDefinitions.ReflectRuntimeCurrentMirror) - importantSymbols ++= importantSymbols map (_.companionSymbol) - importantSymbols ++= importantSymbols map (_.moduleClass) - importantSymbols ++= importantSymbols map (_.linkedClassOfClass) - def isImportantSymbol(sym: Symbol): Boolean = sym != null && sym != NoSymbol && importantSymbols(sym) - val untyped = resetAllAttrs(result, leaveAlone = { + // upd. a new hope: https://groups.google.com/forum/#!topic/scala-internals/TtCTPlj_qcQ + val untyped = resetAttrs(result, leaveAlone = { case ValDef(_, u, _, _) if u == nme.UNIVERSE_SHORT => true case ValDef(_, m, _, _) if m == nme.MIRROR_SHORT => true case tree if symtab.syms contains tree.symbol => true - case tree if isImportantSymbol(tree.symbol) => true case _ => false }) diff --git a/src/compiler/scala/reflect/reify/codegen/GenTrees.scala b/src/compiler/scala/reflect/reify/codegen/GenTrees.scala index f6b3c42ca9..b082796757 100644 --- a/src/compiler/scala/reflect/reify/codegen/GenTrees.scala +++ b/src/compiler/scala/reflect/reify/codegen/GenTrees.scala @@ -140,7 +140,7 @@ trait GenTrees { if (sym == NoSymbol) { // this sometimes happens, e.g. for binds that don't have a body // or for untyped code generated during previous phases - // (see a comment in Reifiers about the latter, starting with "why do we resetAllAttrs?") + // (see a comment in Reifiers about the latter, starting with "why do we reset attrs?") mirrorCall(nme.Ident, reify(name)) } else if (!sym.isLocalToReifee) { diff --git a/src/compiler/scala/tools/nsc/ast/Trees.scala b/src/compiler/scala/tools/nsc/ast/Trees.scala index 24bce0636d..7a7d4ac0b2 100644 --- a/src/compiler/scala/tools/nsc/ast/Trees.scala +++ b/src/compiler/scala/tools/nsc/ast/Trees.scala @@ -176,13 +176,24 @@ trait Trees extends scala.reflect.internal.Trees { self: Global => } } - /** resets symbol and tpe fields in a tree, @see ResetAttrs - */ -// def resetAllAttrs[A<:Tree](x:A): A = { new ResetAttrsTraverser().traverse(x); x } -// def resetLocalAttrs[A<:Tree](x:A): A = { new ResetLocalAttrsTraverser().traverse(x); x } - - def resetAllAttrs(x: Tree, leaveAlone: Tree => Boolean = null): Tree = new ResetAttrs(false, leaveAlone).transform(x) - def resetLocalAttrs(x: Tree, leaveAlone: Tree => Boolean = null): Tree = new ResetAttrs(true, leaveAlone).transform(x) + // Finally, noone resetAllAttrs it anymore, so I'm removing it from the compiler. + // Even though it's with great pleasure I'm doing that, I'll leave its body here to warn future generations about what happened in the past. + // + // So what actually happened in the past is that we used to have two flavors of resetAttrs: resetAllAttrs and resetLocalAttrs. + // resetAllAttrs destroyed all symbols and types in the tree in order to reset its state to something suitable for retypechecking + // and/or embedding into bigger trees / different lexical scopes. (Btw here's some background on why people would want to use + // reset attrs in the first place: https://groups.google.com/forum/#!topic/scala-internals/TtCTPlj_qcQ). + // + // However resetAllAttrs was more of a poison than of a treatment, because along with locally defined symbols that are the cause + // for almost every or maybe even every case of tree corruption, it erased external bindings that sometimes could not be restored. + // This is how we came up with resetLocalAttrs that left external bindings alone, and that was a big step forward. + // Then slowly but steadily we've evicted all usages of resetAllAttrs from our codebase in favor of resetLocalAttrs + // and have been living happily ever after. + // + // def resetAllAttrs(x: Tree, leaveAlone: Tree => Boolean = null): Tree = new ResetAttrs(localOnly = false, leaveAlone).transform(x) + + /** @see ResetAttrs */ + def resetAttrs(x: Tree, leaveAlone: Tree => Boolean = null): Tree = new ResetAttrs(leaveAlone).transform(x) /** A transformer which resets symbol and tpe fields of all nodes in a given tree, * with special treatment of: @@ -193,7 +204,7 @@ trait Trees extends scala.reflect.internal.Trees { self: Global => * * (bq:) This transformer has mutable state and should be discarded after use */ - private class ResetAttrs(localOnly: Boolean, leaveAlone: Tree => Boolean = null, keepLabels: Boolean = false) { + private class ResetAttrs(leaveAlone: Tree => Boolean = null) { // this used to be based on -Ydebug, but the need for logging in this code is so situational // that I've reverted to a hard-coded constant here. val debug = false @@ -277,29 +288,18 @@ trait Trees extends scala.reflect.internal.Trees { self: Global => // vetoXXX local variables declared below describe the conditions under which we cannot erase symbols. // // The first reason to not erase symbols is the threat of non-idempotency (SI-5464). - // Here we take care of labels (SI-5562) and references to package classes (SI-5705). + // Here we take care of references to package classes (SI-5705). // There are other non-idempotencies, but they are not worked around yet. // - // The second reason has to do with the fact that resetAttrs itself has limited usefulness. - // - // First of all, why do we need resetAttrs? Gor one, it's absolutely required to move trees around. - // One cannot just take a typed tree from one lexical context and transplant it somewhere else. - // Most likely symbols defined by those trees will become borked and the compiler will blow up (SI-5797). - // To work around we just erase all symbols and types and then hope that we'll be able to correctly retypecheck. - // For ones who're not affected by scalac Stockholm syndrome, this might seem to be an extremely naive fix, but well... - // - // Of course, sometimes erasing everything won't work, because if a given identifier got resolved to something - // in one lexical scope, it can get resolved to something else. - // - // What do we do in these cases? Enter the workaround for the workaround: resetLocalAttrs, which only destroys - // locally defined symbols, but doesn't touch references to stuff declared outside of a given tree. - // That's what localOnly and vetoScope are for. + // The second reason has to do with the fact that resetAttrs needs to be less destructive. + // Erasing locally-defined symbols is useful to prevent tree corruption, but erasing external bindings is not, + // therefore we want to retain those bindings, especially given that restoring them can be impossible + // if we move these trees into lexical contexts different from their original locations. if (dupl.hasSymbol) { val sym = dupl.symbol - val vetoScope = localOnly && !(locals contains sym) - val vetoLabel = keepLabels && sym.isLabel + val vetoScope = !(locals contains sym) val vetoThis = dupl.isInstanceOf[This] && sym.isPackageClass - if (!(vetoScope || vetoLabel || vetoThis)) dupl.symbol = NoSymbol + if (!(vetoScope || vetoThis)) dupl.symbol = NoSymbol } dupl.clearType() } @@ -308,10 +308,9 @@ trait Trees extends scala.reflect.internal.Trees { self: Global => } def transform(x: Tree): Tree = { - if (localOnly) new MarkLocals().traverse(x) - if (localOnly && debug) { + if (debug) { assert(locals.size == orderedLocals.size) val msg = orderedLocals.toList filter {_ != NoSymbol} map {" " + _} mkString EOL trace("locals (%d total): %n".format(orderedLocals.size))(msg) diff --git a/src/compiler/scala/tools/nsc/typechecker/Namers.scala b/src/compiler/scala/tools/nsc/typechecker/Namers.scala index 645f267a21..632e25aa2e 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Namers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Namers.scala @@ -1205,7 +1205,7 @@ trait Namers extends MethodSynthesis { * flag. */ private def addDefaultGetters(meth: Symbol, ddef: DefDef, vparamss: List[List[ValDef]], tparams: List[TypeDef], overriddenSymbol: => Symbol) { - val DefDef(_, _, rtparams0, rvparamss0, _, _) = resetLocalAttrs(ddef.duplicate) + val DefDef(_, _, rtparams0, rvparamss0, _, _) = resetAttrs(ddef.duplicate) // having defs here is important to make sure that there's no sneaky tree sharing // in methods with multiple default parameters def rtparams = rtparams0.map(_.duplicate) @@ -1292,7 +1292,7 @@ trait Namers extends MethodSynthesis { return // fix #3649 (prevent crash in erroneous source code) } } - val ClassDef(_, _, rtparams, _) = resetLocalAttrs(cdef.duplicate) + val ClassDef(_, _, rtparams, _) = resetAttrs(cdef.duplicate) defTparams = rtparams.map(rt => copyTypeDef(rt)(mods = rt.mods &~ (COVARIANT | CONTRAVARIANT))) nmr } diff --git a/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala b/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala index 46ff98875f..6a4df415ae 100644 --- a/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala +++ b/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala @@ -173,7 +173,7 @@ trait NamesDefaults { self: Analyzer => // setSymbol below is important because the 'selected' function might be overloaded. by // assigning the correct method symbol, typedSelect will just assign the type. the reason // to still call 'typed' is to correctly infer singleton types, SI-5259. - val selectPos = + val selectPos = if(qual.pos.isRange && baseFun.pos.isRange) qual.pos.union(baseFun.pos).withStart(Math.min(qual.pos.end, baseFun.pos.end)) else baseFun.pos val f = blockTyper.typedOperator(Select(newQual, selected).setSymbol(baseFun1.symbol).setPos(selectPos)) @@ -287,7 +287,7 @@ trait NamesDefaults { self: Analyzer => } else { // TODO In 83c9c764b, we tried to a stable type here to fix SI-7234. But the resulting TypeTree over a - // singleton type without an original TypeTree fails to retypecheck after a resetLocalAttrs (SI-7516), + // singleton type without an original TypeTree fails to retypecheck after a resetAttrs (SI-7516), // which is important for (at least) macros. arg.tpe } @@ -310,7 +310,7 @@ trait NamesDefaults { self: Analyzer => new ChangeOwnerTraverser(context.owner, sym) traverse arg // fixes #4502 if (repeated) arg match { case WildcardStarArg(expr) => expr - case _ => blockTyper typed gen.mkSeqApply(resetLocalAttrs(arg)) + case _ => blockTyper typed gen.mkSeqApply(resetAttrs(arg)) } else arg } Some(atPos(body.pos)(ValDef(sym, body).setType(NoType))) diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index f7684b93af..2125e281f0 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -1414,7 +1414,7 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans case TypeRef(pre, sym, args) => tree match { case tt: TypeTree if tt.original == null => // SI-7783 don't warn about inferred types - // FIXME: reconcile this check with one in resetAllAttrs + // FIXME: reconcile this check with one in resetAttrs case _ => checkUndesiredProperties(sym, tree.pos) } if(sym.isJavaDefined) diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 101e1526fe..10fe530445 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -832,7 +832,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper else tpr.typed(withImplicitArgs, mode, pt) } orElse { _ => - val resetTree = resetLocalAttrs(original) + val resetTree = resetAttrs(original) debuglog(s"fallback on implicits: ${tree}/$resetTree") val tree1 = typed(resetTree, mode) // Q: `typed` already calls `pluginsTyped` and `adapt`. the only difference here is that @@ -2307,7 +2307,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper context.scope.unlink(ldef.symbol) val sym2 = namer.enterInScope( context.owner.newLabel(ldef.name, ldef.pos) setInfo MethodType(List(), restpe)) - val rhs2 = typed(resetAllAttrs(ldef.rhs), restpe) + val LabelDef(_, _, rhs1) = resetAttrs(ldef) + val rhs2 = typed(rhs1, restpe) ldef.params foreach (param => param setType param.symbol.tpe) deriveLabelDef(ldef)(_ => rhs2) setSymbol sym2 setType restpe } @@ -2589,7 +2590,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper // // Well behaved trees satisfy the property: // - // typed(tree) == typed(resetLocalAttrs(typed(tree)) + // typed(tree) == typed(resetAttrs(typed(tree)) // // Trees constructed without low-level symbol manipulation get this for free; // references to local symbols are cleared by `ResetAttrs`, but bind to the diff --git a/src/compiler/scala/tools/nsc/typechecker/Unapplies.scala b/src/compiler/scala/tools/nsc/typechecker/Unapplies.scala index ffac29b4b8..cc2d9141ce 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Unapplies.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Unapplies.scala @@ -52,13 +52,13 @@ trait Unapplies extends ast.TreeDSL { } private def constrParamss(cdef: ClassDef): List[List[ValDef]] = { - val ClassDef(_, _, _, Template(_, _, body)) = resetLocalAttrs(cdef.duplicate) + val ClassDef(_, _, _, Template(_, _, body)) = resetAttrs(cdef.duplicate) val DefDef(_, _, _, vparamss, _, _) = treeInfo firstConstructor body vparamss } private def constrTparamsInvariant(cdef: ClassDef): List[TypeDef] = { - val ClassDef(_, _, tparams, _) = resetLocalAttrs(cdef.duplicate) + val ClassDef(_, _, tparams, _) = resetAttrs(cdef.duplicate) val tparamsInvariant = tparams.map(tparam => copyTypeDef(tparam)(mods = tparam.mods &~ (COVARIANT | CONTRAVARIANT))) tparamsInvariant } diff --git a/src/compiler/scala/tools/reflect/ToolBox.scala b/src/compiler/scala/tools/reflect/ToolBox.scala index 4c1bc794bc..02a458214f 100644 --- a/src/compiler/scala/tools/reflect/ToolBox.scala +++ b/src/compiler/scala/tools/reflect/ToolBox.scala @@ -71,12 +71,6 @@ trait ToolBox[U <: scala.reflect.api.Universe] { */ def inferImplicitView(tree: u.Tree, from: u.Type, to: u.Type, silent: Boolean = true, withMacrosDisabled: Boolean = false, pos: u.Position = u.NoPosition): u.Tree - /** Recursively resets symbols and types in a given tree. - * WARNING: Don't use this API, go for [[untypecheck]] instead. - */ - @deprecated("Use `tb.untypecheck` instead", "2.11.0") - def resetAllAttrs(tree: u.Tree): u.Tree - /** Recursively resets locally defined symbols and types in a given tree. * WARNING: Don't use this API, go for [[untypecheck]] instead. */ diff --git a/src/compiler/scala/tools/reflect/ToolBoxFactory.scala b/src/compiler/scala/tools/reflect/ToolBoxFactory.scala index 4a8c91bd1b..541a915adb 100644 --- a/src/compiler/scala/tools/reflect/ToolBoxFactory.scala +++ b/src/compiler/scala/tools/reflect/ToolBoxFactory.scala @@ -236,7 +236,7 @@ abstract class ToolBoxFactory[U <: JavaUniverse](val u: U) { factorySelf => NoPosition)) trace("wrapped: ")(showAttributed(moduledef, true, true, settings.Yshowsymkinds.value)) - val cleanedUp = resetLocalAttrs(moduledef) + val cleanedUp = resetAttrs(moduledef) trace("cleaned up: ")(showAttributed(cleanedUp, true, true, settings.Yshowsymkinds.value)) cleanedUp.asInstanceOf[ModuleDef] } @@ -385,18 +385,10 @@ abstract class ToolBoxFactory[U <: JavaUniverse](val u: U) { factorySelf => uitree } - def resetAllAttrs(tree: u.Tree): u.Tree = withCompilerApi { compilerApi => - import compilerApi._ - val ctree: compiler.Tree = importer.importTree(tree) - val ttree: compiler.Tree = compiler.resetAllAttrs(ctree) - val uttree = exporter.importTree(ttree) - uttree - } - def resetLocalAttrs(tree: u.Tree): u.Tree = withCompilerApi { compilerApi => import compilerApi._ val ctree: compiler.Tree = importer.importTree(tree) - val ttree: compiler.Tree = compiler.resetLocalAttrs(ctree) + val ttree: compiler.Tree = compiler.resetAttrs(ctree) val uttree = exporter.importTree(ttree) uttree } |