SI-8947 Additional layers of defence against EmptyTree mutation

As suggested in review:

  - Use `abort` rather than `{error; EmptyTree} when we hit an
    error in reification or tag materialization.
  - Explicitly avoid adding the `MacroExpansionAttachment` to the
    macro expansion if it an `EmptyTree`
  - Emit a `-Xdev` warning if any other code paths find a way to
    mutate attachments in places they shouldn't.

1 files changed, 4 insertions, 3 deletions

diff --git a/src/reflect/scala/reflect/internal/Trees.scala b/src/reflect/scala/reflect/internal/Trees.scala
index 26e9c7f89d..af533b21bc 100644
--- a/src/reflect/scala/reflect/internal/Trees.scala
+++ b/src/reflect/scala/reflect/internal/Trees.scala
@@ -1078,9 +1078,10 @@ trait Trees extends api.Trees {
 
     // 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 = this
-    override def updateAttachment[T: ClassTag](attachment: T): this.type = this
-    override def removeAttachment[T: ClassTag]: this.type = this
+    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) {