Refactor Annotations (#767)

* Generalize ChiselAnnotation

This allows us to delay creation of Annotations till elaboration is
complete. Also update all annotation-related code.

* Add RunFirrtlTransform

Use a Chisel-specific RunFirrtlTransform API to preserve behavior of old
ChiselAnnotation (now called ChiselLegacyAnnotation)

* Use unique test directories in ChiselRunners.compile

7 files changed, 119 insertions, 75 deletions

diff --git a/chiselFrontend/src/main/scala/chisel3/core/Annotation.scala b/chiselFrontend/src/main/scala/chisel3/core/Annotation.scala
new file mode 100644
index 00000000..cfee67cb
--- /dev/null
+++ b/chiselFrontend/src/main/scala/chisel3/core/Annotation.scala
@@ -0,0 +1,90 @@
+// See LICENSE for license details.
+
+package chisel3.core
+
+import scala.language.existentials
+
+import chisel3.internal.{Builder, InstanceId}
+import firrtl.Transform
+import firrtl.annotations.{Annotation, CircuitName, ComponentName, ModuleName}
+import firrtl.transforms.DontTouchAnnotation
+
+/** Interface for Annotations in Chisel
+  *
+  * Defines a conversion to a corresponding FIRRTL Annotation
+  */
+trait ChiselAnnotation {
+  /** Conversion to FIRRTL Annotation */
+  def toFirrtl: Annotation
+}
+object ChiselAnnotation {
+  @deprecated("Write a custom ChiselAnnotation subclass instead", "3.1")
+  def apply(component: InstanceId, transformClass: Class[_ <: Transform], value: String) =
+    ChiselLegacyAnnotation(component, transformClass, value)
+  @deprecated("Write a custom ChiselAnnotation subclass instead", "3.1")
+  def unapply(anno: ChiselAnnotation): Option[(InstanceId, Class[_ <: Transform], String)] =
+    anno match {
+      case ChiselLegacyAnnotation(c, t, v) => Some(c, t, v)
+      case _ => None
+    }
+}
+
+/** Mixin for [[ChiselAnnotation]] that instantiates an associated FIRRTL Transform when this
+  * Annotation is present during a run of [[chisel3.Driver.execute]]. Automatic Transform
+  * instantiation is *not* supported when the Circuit and Annotations are serialized before invoking
+  * FIRRTL.
+  */
+// TODO There should be a FIRRTL API for this instead
+trait RunFirrtlTransform extends ChiselAnnotation {
+  def transformClass: Class[_ <: Transform]
+}
+
+// This exists for implementation reasons, we don't want people using this type directly
+final case class ChiselLegacyAnnotation private[chisel3] (
+    component: InstanceId,
+    transformClass: Class[_ <: Transform],
+    value: String) extends ChiselAnnotation with RunFirrtlTransform {
+  def toFirrtl: Annotation = Annotation(component.toNamed, transformClass, value)
+}
+private[chisel3] object ChiselLegacyAnnotation
+
+object annotate { // scalastyle:ignore object.name
+  def apply(anno: ChiselAnnotation): Unit = {
+    Builder.annotations += anno
+  }
+}
+
+/** Marks that a signal should not be removed by Chisel and Firrtl optimization passes
+  *
+  * @example {{{
+  * class MyModule extends Module {
+  *   val io = IO(new Bundle {
+  *     val a = Input(UInt(32.W))
+  *     val b = Output(UInt(32.W))
+  *   })
+  *   io.b := io.a
+  *   val dead = io.a +% 1.U // normally dead would be pruned by DCE
+  *   dontTouch(dead) // Marking it as such will preserve it
+  * }
+  * }}}
+  *
+  * @note Calling this on [[Data]] creates an annotation that Chisel emits to a separate annotations
+  * file. This file must be passed to FIRRTL independently of the `.fir` file. The execute methods
+  * in [[chisel3.Driver]] will pass the annotations to FIRRTL automatically.
+  */
+object dontTouch { // scalastyle:ignore object.name
+  /** Marks a signal to be preserved in Chisel and Firrtl
+    *
+    * @note Requires the argument to be bound to hardware
+    * @param data The signal to be marked
+    * @return Unmodified signal `data`
+    */
+  def apply[T <: Data](data: T)(implicit compileOptions: CompileOptions): T = {
+    if (compileOptions.checkSynthesizable) {
+      requireIsHardware(data, "Data marked dontTouch")
+    }
+    annotate(new ChiselAnnotation { def toFirrtl = DontTouchAnnotation(data.toNamed) })
+    data
+  }
+}
+
diff --git a/chiselFrontend/src/main/scala/chisel3/core/ChiselAnnotation.scala b/chiselFrontend/src/main/scala/chisel3/core/ChiselAnnotation.scala
deleted file mode 100644
index 98253ce5..00000000
--- a/chiselFrontend/src/main/scala/chisel3/core/ChiselAnnotation.scala
+++ /dev/null
@@ -1,69 +0,0 @@
-// See LICENSE for license details.
-
-package chisel3.core
-
-import scala.language.existentials
-
-import chisel3.internal.{Builder, InstanceId}
-import firrtl.Transform
-import firrtl.annotations.{Annotation, CircuitName, ComponentName, ModuleName}
-
-/**
-  * This is a stand-in for the firrtl.Annotations.Annotation because at the time this annotation
-  * is created the component cannot be resolved, into a targetString.  Resolution can only
-  * happen after the circuit is elaborated
-  * @param component       A chisel thingy to be annotated, could be module, wire, reg, etc.
-  * @param transformClass  A fully-qualified class name of the transformation pass
-  * @param value           A string value to be used by the transformation pass
-  */
-case class ChiselAnnotation(component: InstanceId, transformClass: Class[_ <: Transform], value: String) {
-  def toFirrtl: Annotation = {
-    val circuitName = CircuitName(component.pathName.split("""\.""").head)
-    component match {
-      case m: BaseModule =>
-        Annotation(
-          ModuleName(m.name, circuitName), transformClass, value)
-      case _ =>
-        Annotation(
-          ComponentName(
-            component.instanceName, ModuleName(component.parentModName, circuitName)), transformClass, value)
-    }
-  }
-}
-
-/** Marks that a signal should not be removed by Chisel and Firrtl optimization passes
-  *
-  * @example {{{
-  * class MyModule extends Module {
-  *   val io = IO(new Bundle {
-  *     val a = Input(UInt(32.W))
-  *     val b = Output(UInt(32.W))
-  *   })
-  *   io.b := io.a
-  *   val dead = io.a +% 1.U // normally dead would be pruned by DCE
-  *   dontTouch(dead) // Marking it as such will preserve it
-  * }
-  * }}}
-  *
-  * @note Calling this on [[Data]] creates an annotation that Chisel emits to a separate annotations
-  * file. This file must be passed to FIRRTL independently of the `.fir` file. The execute methods
-  * in [[chisel3.Driver]] will pass the annotations to FIRRTL automatically.
-  */
-object dontTouch { // scalastyle:ignore object.name
-  /** Marks a signal to be preserved in Chisel and Firrtl
-    *
-    * @note Requires the argument to be bound to hardware
-    * @param data The signal to be marked
-    * @return Unmodified signal `data`
-    */
-  def apply[T <: Data](data: T)(implicit compileOptions: CompileOptions): T = {
-    if (compileOptions.checkSynthesizable) {
-      requireIsHardware(data, "Data marked dontTouch")
-    }
-    // TODO unify with firrtl.transforms.DontTouchAnnotation
-    val anno = ChiselAnnotation(data, classOf[firrtl.Transform], "DONTtouch!")
-    Builder.annotations += anno
-    data
-  }
-}
-
diff --git a/chiselFrontend/src/main/scala/chisel3/core/Data.scala b/chiselFrontend/src/main/scala/chisel3/core/Data.scala
index 40781490..65f89cc1 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Data.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Data.scala
@@ -198,7 +198,7 @@ object Flipped {
   * time) of bits, and must have methods to pack / unpack structured data to /
   * from bits.
   */
-abstract class Data extends HasId {
+abstract class Data extends HasId with NamedComponent {
   // This is a bad API that punches through object boundaries.
   @deprecated("pending removal once all instances replaced", "chisel3")
   private[chisel3] def flatten: IndexedSeq[Element] = {
diff --git a/chiselFrontend/src/main/scala/chisel3/core/Mem.scala b/chiselFrontend/src/main/scala/chisel3/core/Mem.scala
index c9208030..3fe78010 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Mem.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Mem.scala
@@ -31,7 +31,7 @@ object Mem {
   }
 }
 
-sealed abstract class MemBase[T <: Data](t: T, val length: Int) extends HasId {
+sealed abstract class MemBase[T <: Data](t: T, val length: Int) extends HasId with NamedComponent {
   // REVIEW TODO: make accessors (static/dynamic, read/write) combinations consistent.
 
   /** Creates a read accessor into the memory with static addressing. See the
diff --git a/chiselFrontend/src/main/scala/chisel3/core/Module.scala b/chiselFrontend/src/main/scala/chisel3/core/Module.scala
index 5ba6dbc8..5a4f6abb 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Module.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Module.scala
@@ -13,6 +13,8 @@ import chisel3.internal.Builder._
 import chisel3.internal.firrtl._
 import chisel3.internal.sourceinfo.{InstTransform, SourceInfo}
 
+import _root_.firrtl.annotations.{CircuitName, ModuleName}
+
 object Module {
   /** A wrapper method that all Module instantiations must be wrapped in
     * (necessary to help Chisel track internal state).
@@ -139,6 +141,11 @@ abstract class BaseModule extends HasId {
   /** Legalized name of this module. */
   final val name = Builder.globalNamespace.name(desiredName)
 
+  /** Returns a FIRRTL ModuleName that references this object
+    * @note Should not be called until circuit elaboration is complete
+    */
+  final def toNamed: ModuleName = ModuleName(this.name, CircuitName(this.circuitName))
+
   /** Called at the Module.apply(...) level after this Module has finished elaborating.
     * Returns a map of nodes -> names, for named nodes.
     *
@@ -196,6 +203,7 @@ abstract class BaseModule extends HasId {
   //
   // BaseModule User API functions
   //
+  @deprecated("Use chisel3.experimental.annotate instead", "3.1")
   protected def annotate(annotation: ChiselAnnotation): Unit = {
     Builder.annotations += annotation
   }
diff --git a/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala b/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala
index 5c5c690e..2cb206d4 100644
--- a/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala
+++ b/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala
@@ -8,6 +8,7 @@ import scala.collection.mutable.{ArrayBuffer, HashMap}
 import chisel3._
 import core._
 import firrtl._
+import _root_.firrtl.annotations.{CircuitName, ComponentName, ModuleName, Named}
 
 private[chisel3] class Namespace(keywords: Set[String]) {
   private val names = collection.mutable.HashMap[String, Long]()
@@ -66,6 +67,9 @@ trait InstanceId {
   def pathName: String
   def parentPathName: String
   def parentModName: String
+  /** Returns a FIRRTL Named that refers to this object in the elaborated hardware graph */
+  def toNamed: Named
+
 }
 
 private[chisel3] trait HasId extends InstanceId {
@@ -129,6 +133,11 @@ private[chisel3] trait HasId extends InstanceId {
     case Some(p) => p.name
     case None => throwException(s"$instanceName doesn't have a parent")
   }
+  // TODO Should this be public?
+  protected def circuitName: String = _parent match {
+    case None => instanceName
+    case Some(p) => p.circuitName
+  }
 
   private[chisel3] def getPublicFields(rootClass: Class[_]): Seq[java.lang.reflect.Method] = {
     // Suggest names to nodes using runtime reflection
@@ -142,6 +151,14 @@ private[chisel3] trait HasId extends InstanceId {
     this.getClass.getMethods.sortWith(_.getName < _.getName).filter(isPublicVal(_))
   }
 }
+/** Holds the implementation of toNamed for Data and MemBase */
+private[chisel3] trait NamedComponent extends HasId {
+  /** Returns a FIRRTL ComponentName that references this object
+    * @note Should not be called until circuit elaboration is complete
+    */
+  final def toNamed: ComponentName =
+    ComponentName(this.instanceName, ModuleName(this.parentModName, CircuitName(this.circuitName)))
+}
 
 private[chisel3] class DynamicContext() {
   val idGen = new IdGen
@@ -277,7 +294,7 @@ private[chisel3] object Builder {
       errors.checkpoint()
       errors.info("Done elaborating.")
 
-      Circuit(components.last.name, components, annotations.map(_.toFirrtl))
+      Circuit(components.last.name, components, annotations)
     }
   }
   initializeSingletons()
diff --git a/chiselFrontend/src/main/scala/chisel3/internal/firrtl/IR.scala b/chiselFrontend/src/main/scala/chisel3/internal/firrtl/IR.scala
index b499c2b1..6b555a82 100644
--- a/chiselFrontend/src/main/scala/chisel3/internal/firrtl/IR.scala
+++ b/chiselFrontend/src/main/scala/chisel3/internal/firrtl/IR.scala
@@ -7,8 +7,6 @@ import core._
 import chisel3.internal._
 import chisel3.internal.sourceinfo.{SourceInfo, NoSourceInfo}
 
-import _root_.firrtl.annotations.Annotation
-
 case class PrimOp(val name: String) {
   override def toString: String = name
 }
@@ -278,4 +276,4 @@ abstract class Component extends Arg {
 case class DefModule(id: UserModule, name: String, ports: Seq[Port], commands: Seq[Command]) extends Component
 case class DefBlackBox(id: BaseBlackBox, name: String, ports: Seq[Port], topDir: SpecifiedDirection, params: Map[String, Param]) extends Component
 
-case class Circuit(name: String, components: Seq[Component], annotations: Seq[Annotation] = Seq.empty)
+case class Circuit(name: String, components: Seq[Component], annotations: Seq[ChiselAnnotation] = Seq.empty)