diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2014-11-07 11:10:15 +1000 |
---|---|---|
committer | Jason Zaugg <jzaugg@gmail.com> | 2014-11-07 11:10:15 +1000 |
commit | 7830b4689a5f9962011f117c9b866a939795e58a (patch) | |
tree | cc3e0a2bbf67d350775d44dbe648ea2e37451c1c | |
parent | 000de44e3b1f1c0c4bc30eda8ef155548714a6af (diff) | |
parent | 17992f672f7d5663654a1ea365dfd1dad7061410 (diff) | |
download | scala-7830b4689a5f9962011f117c9b866a939795e58a.tar.gz scala-7830b4689a5f9962011f117c9b866a939795e58a.tar.bz2 scala-7830b4689a5f9962011f117c9b866a939795e58a.zip |
Merge pull request #4083 from retronym/ticket/8947
SI-8947 Avoid cross talk between tag materializers and reify
-rw-r--r-- | src/compiler/scala/reflect/reify/Taggers.scala | 3 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/StdAttachments.scala | 2 | ||||
-rw-r--r-- | src/reflect/scala/reflect/internal/Trees.scala | 8 | ||||
-rw-r--r-- | test/files/pos/t8947/Client_2.scala | 1 | ||||
-rw-r--r-- | test/files/pos/t8947/Macro_1.scala | 41 | ||||
-rw-r--r-- | test/junit/scala/tools/nsc/symtab/CannotHaveAttrsTest.scala | 12 |
6 files changed, 64 insertions, 3 deletions
diff --git a/src/compiler/scala/reflect/reify/Taggers.scala b/src/compiler/scala/reflect/reify/Taggers.scala index 093c2bee22..0863ee38f9 100644 --- a/src/compiler/scala/reflect/reify/Taggers.scala +++ b/src/compiler/scala/reflect/reify/Taggers.scala @@ -79,8 +79,7 @@ abstract class Taggers { try materializer catch { case ReificationException(pos, msg) => - c.error(pos.asInstanceOf[c.Position], msg) // this cast is a very small price for the sanity of exception handling - EmptyTree + c.abort(pos.asInstanceOf[c.Position], msg) // this cast is a very small price for the sanity of exception handling case UnexpectedReificationException(pos, err, cause) if cause != null => throw cause } diff --git a/src/compiler/scala/tools/nsc/typechecker/StdAttachments.scala b/src/compiler/scala/tools/nsc/typechecker/StdAttachments.scala index 57f27a05fd..ea44b9dc39 100644 --- a/src/compiler/scala/tools/nsc/typechecker/StdAttachments.scala +++ b/src/compiler/scala/tools/nsc/typechecker/StdAttachments.scala @@ -61,7 +61,7 @@ trait StdAttachments { val metadata = MacroExpansionAttachment(expandee, expanded) expandee updateAttachment metadata expanded match { - case expanded: Tree => expanded updateAttachment metadata + case expanded: Tree if !expanded.isEmpty => expanded updateAttachment metadata case _ => // do nothing } } diff --git a/src/reflect/scala/reflect/internal/Trees.scala b/src/reflect/scala/reflect/internal/Trees.scala index 2f07cef315..35de3adff6 100644 --- a/src/reflect/scala/reflect/internal/Trees.scala +++ b/src/reflect/scala/reflect/internal/Trees.scala @@ -9,6 +9,7 @@ package internal import Flags._ import scala.collection.{ mutable, immutable } +import scala.reflect.macros.Attachments import util.Statistics trait Trees extends api.Trees { @@ -1074,6 +1075,13 @@ trait Trees extends api.Trees { override def setType(t: Type) = { requireLegal(t, NoType, "tpe"); this } override def tpe_=(t: Type) = setType(t) + // We silently ignore attempts to add attachments to `EmptyTree`. See SI-8947 for an + // example of a bug in macro expansion that this solves. + override def setAttachments(attachments: Attachments {type Pos = Position}): this.type = attachmentWarning() + override def updateAttachment[T: ClassTag](attachment: T): this.type = attachmentWarning() + override def removeAttachment[T: ClassTag]: this.type = attachmentWarning() + private def attachmentWarning(): this.type = {devWarning(s"Attempt to mutate attachments on $self ignored"); this} + private def requireLegal(value: Any, allowed: Any, what: String) = ( if (value != allowed) { log(s"can't set $what for $self to value other than $allowed") diff --git a/test/files/pos/t8947/Client_2.scala b/test/files/pos/t8947/Client_2.scala new file mode 100644 index 0000000000..1a5082a2f9 --- /dev/null +++ b/test/files/pos/t8947/Client_2.scala @@ -0,0 +1 @@ +object Test { X.extractor }
\ No newline at end of file diff --git a/test/files/pos/t8947/Macro_1.scala b/test/files/pos/t8947/Macro_1.scala new file mode 100644 index 0000000000..4a5de3decb --- /dev/null +++ b/test/files/pos/t8947/Macro_1.scala @@ -0,0 +1,41 @@ +import language.experimental.macros +import scala.reflect.macros._ +import blackbox.Context + +object X { + + def classTagOrNull[T](implicit t: reflect.ClassTag[T] = null) = t + // the failed search for ClassTag[T] does not issue a visible + // error as we fall back to the default argument. But, the + // macro engine things we have expanded the macro `materializeClassTag[D]()` + // to `EmptyTree`, and then attaches a backreference from the expansion + // to the expandee. This is the `MacroExpansionAttachment` tree attachment. + def foo[D] = classTagOrNull[D] + + def extractor: Any = macro X.extractorMacro + def extractorMacro(c: Context): c.Expr[Any] = { + // Later, in reify, an unrelated use of `EmptyTree` in the AST representing + // the argument is now treated as a macro expansion which should be rolled + // back in the tree we reify! This ends up generating a call to `implicitly` + // which leads to an ambiguous error. + // + // Any macro call that expands to EmptyTree could have triggered this problem. + c.universe.reify(new { def something(data: Any) = ??? }) + } + + // Workarounds: + // + // 1. Use quasiquotes rather than `reify`. (But, beware to fully qualify all references, e.g. `_root_.scala.Predef.???`) + // 2. Avoid failed ClassTag lookups (e.g. in the original bug report, annotate the type argument to `map`) + // 3. In the macro implementation, just before calling the `reify` macro, you could call another macro + // + // def prepareReify = macro prepareReifyImpl + // def prepareReifyImpl(c: Context) = { + // val symtab = c.universe.asInstanceOf[reflect.internal.SymbolTable] + // symtab.EmptyTree.setAttachments(symtab.NoPosition) + // } + // + // To make this visible to the macro implementaiton, it will need to be compiled in an earlier stage, + // e.g a separate SBT sub-project. + +} diff --git a/test/junit/scala/tools/nsc/symtab/CannotHaveAttrsTest.scala b/test/junit/scala/tools/nsc/symtab/CannotHaveAttrsTest.scala index d424f12710..69931c9e24 100644 --- a/test/junit/scala/tools/nsc/symtab/CannotHaveAttrsTest.scala +++ b/test/junit/scala/tools/nsc/symtab/CannotHaveAttrsTest.scala @@ -64,4 +64,16 @@ class CannotHaveAttrsTest { assertThrows[IllegalArgumentException] { t.setType(tpe) } } } + + class Attach + @Test + def attachmentsAreIgnored = { + attrlessTrees.foreach { t => + t.setAttachments(NoPosition.update(new Attach)) + assert(t.attachments == NoPosition) + t.updateAttachment(new Attach) + assert(t.attachments == NoPosition) + t.removeAttachment[Attach] // no exception + } + } } |