Skip to content

Commit

Permalink
Fix SOE with macros in dependencies extraction
Browse files Browse the repository at this point in the history
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#1237 and
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.
  • Loading branch information
Martin Duhem committed Sep 1, 2014
1 parent 6860227 commit 438206e
Showing 1 changed file with 72 additions and 55 deletions.
127 changes: 72 additions & 55 deletions compile/interface/src/main/scala/xsbt/Dependency.scala
Expand Up @@ -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)
}

Expand All @@ -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)
}

Expand Down

0 comments on commit 438206e

Please sign in to comment.