From 438206eb5ad5cf018fc968244a0a98468b953608 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 1 Sep 2014 10:07:23 +0200 Subject: [PATCH] Fix SOE with macros in dependencies extraction In some cases, expanded macros report that their original tree and its expansion are the same, thus creating a cyclic chain. This chain may then produce a SOE during dependencies or used names extraction. This kind of problem was already reported in sbt/sbt#1237 and sbt/sbt#1408. Unfortunately, the fix that was applied to the dependencies extraction part was not sufficient. This commit ports the fix applied to the used names extraction part to the dependencies extraction part. --- .../src/main/scala/xsbt/Dependency.scala | 127 ++++++++++-------- 1 file changed, 72 insertions(+), 55 deletions(-) diff --git a/compile/interface/src/main/scala/xsbt/Dependency.scala b/compile/interface/src/main/scala/xsbt/Dependency.scala index b2b4e012d5..6f4a0cace1 100644 --- a/compile/interface/src/main/scala/xsbt/Dependency.scala +++ b/compile/interface/src/main/scala/xsbt/Dependency.scala @@ -92,68 +92,85 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile { } } - private abstract class ExtractDependenciesTraverser extends Traverser { + private abstract class DependenciesExtractor { protected val depBuf = collection.mutable.ArrayBuffer.empty[Symbol] protected def addDependency(dep: Symbol): Unit = depBuf += dep def dependencies: collection.immutable.Set[Symbol] = { // convert to immutable set and remove NoSymbol if we have one depBuf.toSet - NoSymbol } + def extractByTreeWalk(tree: Tree): Unit } - private class ExtractDependenciesByMemberRefTraverser extends ExtractDependenciesTraverser { - override def traverse(tree: Tree): Unit = { - tree match { - case Import(expr, selectors) => - selectors.foreach { - case ImportSelector(nme.WILDCARD, _, null, _) => - // in case of wildcard import we do not rely on any particular name being defined - // on `expr`; all symbols that are being used will get caught through selections - case ImportSelector(name: Name, _, _, _) => - def lookupImported(name: Name) = expr.symbol.info.member(name) - // importing a name means importing both a term and a type (if they exist) - addDependency(lookupImported(name.toTermName)) - addDependency(lookupImported(name.toTypeName)) + private class DependenciesByMemberRefExtractor extends DependenciesExtractor { + override def extractByTreeWalk(tree: Tree): Unit = { + + def handleTreeNode(node: Tree): Unit = { + + def handleMacroExpansion(original: Tree): Unit = { + // Some macros seem to be their own orignal tree, or appear in the children of their + // original tree. To prevent infinite loops, we need to filter out nodes that we already + // handled. + // This is only relevant for Scala 2.10.4 + // See https://issues.scala-lang.org/browse/SI-8486 + original.filter(_ ne node).foreach(handleTreeNode) + } + + def handleClassicTreeNode(node: Tree): Unit = { + node match { + case Import(expr, selectors) => + selectors.foreach { + case ImportSelector(nme.WILDCARD, _, null, _) => + // in case of wildcard import we do not rely on any particular name being defined + // on `expr`; all symbols that are being used will get caught through selections + case ImportSelector(name: Name, _, _, _) => + def lookupImported(name: Name) = expr.symbol.info.member(name) + // importing a name means importing both a term and a type (if they exist) + addDependency(lookupImported(name.toTermName)) + addDependency(lookupImported(name.toTypeName)) + } + case select: Select => + addDependency(select.symbol) + /* + * Idents are used in number of situations: + * - to refer to local variable + * - to refer to a top-level package (other packages are nested selections) + * - to refer to a term defined in the same package as an enclosing class; + * this looks fishy, see this thread: + * https://groups.google.com/d/topic/scala-internals/Ms9WUAtokLo/discussion + */ + case ident: Ident => + addDependency(ident.symbol) + case typeTree: TypeTree => + val typeSymbolCollector = new CollectTypeTraverser({ + case tpe if !tpe.typeSymbol.isPackage => tpe.typeSymbol + }) + typeSymbolCollector.traverse(typeTree.tpe) + val deps = typeSymbolCollector.collected.toSet + deps.foreach(addDependency) + case Template(parents, self, body) => + body foreach extractByTreeWalk + case other => () } - case select: Select => - addDependency(select.symbol) - /* - * Idents are used in number of situations: - * - to refer to local variable - * - to refer to a top-level package (other packages are nested selections) - * - to refer to a term defined in the same package as an enclosing class; - * this looks fishy, see this thread: - * https://groups.google.com/d/topic/scala-internals/Ms9WUAtokLo/discussion - */ - case ident: Ident => - addDependency(ident.symbol) - case typeTree: TypeTree => - val typeSymbolCollector = new CollectTypeTraverser({ - case tpe if !tpe.typeSymbol.isPackage => tpe.typeSymbol - }) - typeSymbolCollector.traverse(typeTree.tpe) - val deps = typeSymbolCollector.collected.toSet - deps.foreach(addDependency) - case Template(parents, self, body) => - traverseTrees(body) - /* - * Some macros appear to contain themselves as original tree - * In this case, we don't need to inspect the original tree because - * we already inspected its expansion, which is equal. - * See https://issues.scala-lang.org/browse/SI-8486 - */ - case MacroExpansionOf(original) if original != tree => - this.traverse(original) - case other => () + } + + node match { + case MacroExpansionOf(original) => + handleClassicTreeNode(node) + handleMacroExpansion(original) + case _ => + handleClassicTreeNode(node) + } } - super.traverse(tree) + + tree.foreach(handleTreeNode) } } private def extractDependenciesByMemberRef(unit: CompilationUnit): collection.immutable.Set[Symbol] = { - val traverser = new ExtractDependenciesByMemberRefTraverser - traverser.traverse(unit.body) - val dependencies = traverser.dependencies + val extractor = new DependenciesByMemberRefExtractor + extractor.extractByTreeWalk(unit.body) + val dependencies = extractor.dependencies dependencies.map(enclosingTopLevelClass) } @@ -163,23 +180,23 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile { log(msg) } - private final class ExtractDependenciesByInheritanceTraverser extends ExtractDependenciesTraverser { - override def traverse(tree: Tree): Unit = tree match { + private final class DependenciesByInheritanceExtractor extends DependenciesExtractor { + override def extractByTreeWalk(tree: Tree): Unit = tree match { case Template(parents, self, body) => // we are using typeSymbol and not typeSymbolDirect because we want // type aliases to be expanded val parentTypeSymbols = parents.map(parent => parent.tpe.typeSymbol).toSet debuglog("Parent type symbols for " + tree.pos + ": " + parentTypeSymbols.map(_.fullName)) parentTypeSymbols.foreach(addDependency) - traverseTrees(body) - case tree => super.traverse(tree) + body foreach extractByTreeWalk + case tree => tree.children foreach extractByTreeWalk } } private def extractDependenciesByInheritance(unit: CompilationUnit): collection.immutable.Set[Symbol] = { - val traverser = new ExtractDependenciesByInheritanceTraverser - traverser.traverse(unit.body) - val dependencies = traverser.dependencies + val extractor = new DependenciesByInheritanceExtractor + extractor.extractByTreeWalk(unit.body) + val dependencies = extractor.dependencies dependencies.map(enclosingTopLevelClass) }