From 12a799c9293982ffa285daf0a4720ee90f6ed1d2 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 7 Apr 2014 11:25:27 +0200 Subject: [PATCH 1/3] Add two pending tests for sbt/sbt#1237 Add two scripted tests that illustrate a problem occurring with Scala 2.11-RC3 where some macros have themselves attached as original tree, which causes a stack overflow during dependency extraction. --- .../macro-client/Client.scala | 5 ++++ .../macro-arg-dep-2-11/macro-client/Foo.scala | 5 ++++ .../macro-client/changes/Foo.scala | 3 ++ .../macro-provider/Provider.scala | 12 ++++++++ .../macro-arg-dep-2-11/pending | 12 ++++++++ .../macro-arg-dep-2-11/project/build.scala | 30 +++++++++++++++++++ .../macro-client/Client.scala | 5 ++++ .../macro-client/Foo.scala | 5 ++++ .../macro-client/changes/Foo.scala | 3 ++ .../macro-provider/Provider.scala | 12 ++++++++ .../macro-arg-dep-nested-2-11/pending | 13 ++++++++ .../project/build.scala | 30 +++++++++++++++++++ 12 files changed, 135 insertions(+) create mode 100644 sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/macro-client/Client.scala create mode 100644 sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/macro-client/Foo.scala create mode 100644 sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/macro-client/changes/Foo.scala create mode 100644 sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/macro-provider/Provider.scala create mode 100644 sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/pending create mode 100644 sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/project/build.scala create mode 100644 sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/macro-client/Client.scala create mode 100644 sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/macro-client/Foo.scala create mode 100644 sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/macro-client/changes/Foo.scala create mode 100644 sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/macro-provider/Provider.scala create mode 100644 sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/pending create mode 100644 sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/project/build.scala diff --git a/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/macro-client/Client.scala b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/macro-client/Client.scala new file mode 100644 index 000000000..0ecbe6fce --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/macro-client/Client.scala @@ -0,0 +1,5 @@ +package macros + +object Client { + Provider.printTree(Foo.str) +} diff --git a/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/macro-client/Foo.scala b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/macro-client/Foo.scala new file mode 100644 index 000000000..6f410fca2 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/macro-client/Foo.scala @@ -0,0 +1,5 @@ +package macros + +object Foo { + def str: String = "abc" +} diff --git a/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/macro-client/changes/Foo.scala b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/macro-client/changes/Foo.scala new file mode 100644 index 000000000..4f2a62b39 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/macro-client/changes/Foo.scala @@ -0,0 +1,3 @@ +package macros +object Foo { +} diff --git a/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/macro-provider/Provider.scala b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/macro-provider/Provider.scala new file mode 100644 index 000000000..b39b4c282 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/macro-provider/Provider.scala @@ -0,0 +1,12 @@ +package macros +import scala.language.experimental.macros +import scala.reflect.macros._ + +object Provider { + def printTree(arg: Any) = macro printTreeImpl + def printTreeImpl(c: Context)(arg: c.Expr[Any]): c.Expr[String] = { + val argStr = arg.tree.toString + val literalStr = c.universe.Literal(c.universe.Constant(argStr)) + c.Expr[String](literalStr) + } +} diff --git a/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/pending b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/pending new file mode 100644 index 000000000..183aa6c49 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/pending @@ -0,0 +1,12 @@ +> compile + +# remove `Foo.str` which is an argument to a macro +$ copy-file macro-client/changes/Foo.scala macro-client/Foo.scala + +# we should recompile Foo.scala first and then fail to compile Client.scala due to missing +# `Foo.str` +-> macro-client/compile + +> clean + +-> compile diff --git a/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/project/build.scala b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/project/build.scala new file mode 100644 index 000000000..89cd91ef8 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/project/build.scala @@ -0,0 +1,30 @@ +import sbt._ +import Keys._ + +object build extends Build { + val defaultSettings = Seq( + libraryDependencies <+= scalaVersion("org.scala-lang" % "scala-reflect" % _ ), + incOptions := incOptions.value.withNameHashing(true), + scalaVersion := "2.11.0-RC3" + ) + + lazy val root = Project( + base = file("."), + id = "macro", + aggregate = Seq(macroProvider, macroClient), + settings = Defaults.defaultSettings ++ defaultSettings + ) + + lazy val macroProvider = Project( + base = file("macro-provider"), + id = "macro-provider", + settings = Defaults.defaultSettings ++ defaultSettings + ) + + lazy val macroClient = Project( + base = file("macro-client"), + id = "macro-client", + dependencies = Seq(macroProvider), + settings = Defaults.defaultSettings ++ defaultSettings + ) +} diff --git a/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/macro-client/Client.scala b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/macro-client/Client.scala new file mode 100644 index 000000000..76c16af24 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/macro-client/Client.scala @@ -0,0 +1,5 @@ +package macros + +object Client { + Provider.printTree(Provider.printTree(Foo.str)) +} diff --git a/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/macro-client/Foo.scala b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/macro-client/Foo.scala new file mode 100644 index 000000000..6f410fca2 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/macro-client/Foo.scala @@ -0,0 +1,5 @@ +package macros + +object Foo { + def str: String = "abc" +} diff --git a/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/macro-client/changes/Foo.scala b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/macro-client/changes/Foo.scala new file mode 100644 index 000000000..4f2a62b39 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/macro-client/changes/Foo.scala @@ -0,0 +1,3 @@ +package macros +object Foo { +} diff --git a/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/macro-provider/Provider.scala b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/macro-provider/Provider.scala new file mode 100644 index 000000000..b39b4c282 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/macro-provider/Provider.scala @@ -0,0 +1,12 @@ +package macros +import scala.language.experimental.macros +import scala.reflect.macros._ + +object Provider { + def printTree(arg: Any) = macro printTreeImpl + def printTreeImpl(c: Context)(arg: c.Expr[Any]): c.Expr[String] = { + val argStr = arg.tree.toString + val literalStr = c.universe.Literal(c.universe.Constant(argStr)) + c.Expr[String](literalStr) + } +} diff --git a/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/pending b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/pending new file mode 100644 index 000000000..231939418 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/pending @@ -0,0 +1,13 @@ +> compile + +# remove `Foo.str` which is an argument to a macro +# (this macro itself that is an argument to another macro) +$ copy-file macro-client/changes/Foo.scala macro-client/Foo.scala + +# we should recompile Foo.scala first and then fail to compile Client.scala due to missing +# `Foo.str` +-> macro-client/compile + +> clean + +-> compile diff --git a/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/project/build.scala b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/project/build.scala new file mode 100644 index 000000000..89cd91ef8 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/project/build.scala @@ -0,0 +1,30 @@ +import sbt._ +import Keys._ + +object build extends Build { + val defaultSettings = Seq( + libraryDependencies <+= scalaVersion("org.scala-lang" % "scala-reflect" % _ ), + incOptions := incOptions.value.withNameHashing(true), + scalaVersion := "2.11.0-RC3" + ) + + lazy val root = Project( + base = file("."), + id = "macro", + aggregate = Seq(macroProvider, macroClient), + settings = Defaults.defaultSettings ++ defaultSettings + ) + + lazy val macroProvider = Project( + base = file("macro-provider"), + id = "macro-provider", + settings = Defaults.defaultSettings ++ defaultSettings + ) + + lazy val macroClient = Project( + base = file("macro-client"), + id = "macro-client", + dependencies = Seq(macroProvider), + settings = Defaults.defaultSettings ++ defaultSettings + ) +} From a80966e394b8f119b0875d9a9a7994591bdaf1d8 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 7 Apr 2014 11:33:47 +0200 Subject: [PATCH 2/3] Handle macros that have themselves as original tree It has been reported in sbt/sbt#1237 that stack overflows may occur during the extraction of used names (and later of dependencies between files). This problem has been introduced by sbt/sbt#1163, which was about recording the dependencies of macro arguments. When a macro is expanded, the compiler attaches the tree before expansion to the tree representing the expanded macro. As of Scala 2.11-RC3, some macros have themselves attached as original tree, which caused the same macro to be inspected over and over until a stack overflow. This commit solves this problem by making sure that the original of a macro expansion will be inspected if and only if it is different from the expanded tree. Fixes sbt/sbt#1237 --- compile/interface/src/main/scala/xsbt/Dependency.scala | 7 ++++++- .../interface/src/main/scala/xsbt/ExtractUsedNames.scala | 8 +++++++- .../macro-arg-dep-2-11/{pending => test} | 0 .../macro-arg-dep-nested-2-11/{pending => test} | 0 4 files changed, 13 insertions(+), 2 deletions(-) rename sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/{pending => test} (100%) rename sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/{pending => test} (100%) diff --git a/compile/interface/src/main/scala/xsbt/Dependency.scala b/compile/interface/src/main/scala/xsbt/Dependency.scala index b8a55c8a9..77dd9355f 100644 --- a/compile/interface/src/main/scala/xsbt/Dependency.scala +++ b/compile/interface/src/main/scala/xsbt/Dependency.scala @@ -146,7 +146,12 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile deps.foreach(addDependency) case Template(parents, self, body) => traverseTrees(body) - case MacroExpansionOf(original) => + /* + * 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. + */ + case MacroExpansionOf(original) if original != tree => this.traverse(original) case other => () } diff --git a/compile/interface/src/main/scala/xsbt/ExtractUsedNames.scala b/compile/interface/src/main/scala/xsbt/ExtractUsedNames.scala index 6ab01c9eb..1bcaf125f 100644 --- a/compile/interface/src/main/scala/xsbt/ExtractUsedNames.scala +++ b/compile/interface/src/main/scala/xsbt/ExtractUsedNames.scala @@ -55,7 +55,13 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext } def handleTreeNode(node: Tree): Unit = { - def handleMacroExpansion(original: Tree): Unit = original.foreach(handleTreeNode) + def handleMacroExpansion(original: Tree): Unit = { + // Some macros seem to have themselves registered as original tree. + // In this case, we only need to handle the children of the original tree, + // because we already handled the expanded tree. + if(original == node) original.children.foreach(handleTreeNode) + else original.foreach(handleTreeNode) + } def handleClassicTreeNode(node: Tree): Unit = node match { case _: DefTree | _: Template => () diff --git a/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/pending b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/test similarity index 100% rename from sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/pending rename to sbt/src/sbt-test/source-dependencies/macro-arg-dep-2-11/test diff --git a/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/pending b/sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/test similarity index 100% rename from sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/pending rename to sbt/src/sbt-test/source-dependencies/macro-arg-dep-nested-2-11/test From 062cd1c77634e8faf12c6eb4707a43b939c4f40e Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Tue, 8 Apr 2014 23:18:48 +0200 Subject: [PATCH 3/3] Add link to corresponding issue in Scala issue tracker --- compile/interface/src/main/scala/xsbt/Dependency.scala | 1 + compile/interface/src/main/scala/xsbt/ExtractUsedNames.scala | 1 + 2 files changed, 2 insertions(+) diff --git a/compile/interface/src/main/scala/xsbt/Dependency.scala b/compile/interface/src/main/scala/xsbt/Dependency.scala index 77dd9355f..1edae4ac0 100644 --- a/compile/interface/src/main/scala/xsbt/Dependency.scala +++ b/compile/interface/src/main/scala/xsbt/Dependency.scala @@ -150,6 +150,7 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile * 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) diff --git a/compile/interface/src/main/scala/xsbt/ExtractUsedNames.scala b/compile/interface/src/main/scala/xsbt/ExtractUsedNames.scala index 1bcaf125f..ba8e87a1e 100644 --- a/compile/interface/src/main/scala/xsbt/ExtractUsedNames.scala +++ b/compile/interface/src/main/scala/xsbt/ExtractUsedNames.scala @@ -59,6 +59,7 @@ class ExtractUsedNames[GlobalType <: CallbackGlobal](val global: GlobalType) ext // Some macros seem to have themselves registered as original tree. // In this case, we only need to handle the children of the original tree, // because we already handled the expanded tree. + // See https://issues.scala-lang.org/browse/SI-8486 if(original == node) original.children.foreach(handleTreeNode) else original.foreach(handleTreeNode) }