Add safer Chisel annotation API, deprecate old ones

core/src/main/scala/chisel3/Annotation.scala

@@ -3,6 +3,7 @@
 package chisel3.experimental
 
 import scala.language.existentials
+import scala.annotation.nowarn
 import chisel3.internal.Builder
 import chisel3.{Data, InstanceId, RawModule}
 import firrtl.annotations._
@@ -13,6 +14,10 @@ import firrtl.transforms.{DedupGroupAnnotation, NoDedupAnnotation}
   *
   * Defines a conversion to a corresponding FIRRTL Annotation
   */
+@deprecated(
+  "Avoid custom annotations. If you must use annotations, new annotate.apply method that takes Data",
+  "Chisel 7.0"
+)
 trait ChiselAnnotation {
 
   /** Conversion to FIRRTL Annotation */
@@ -23,16 +28,42 @@ trait ChiselAnnotation {
   *
   *  Defines a conversion to corresponding FIRRTL Annotation(s)
   */
+@deprecated(
+  "Avoid custom annotations. If you must use annotations, new annotate.apply method that takes Data",
+  "Chisel 7.0"
+)
 trait ChiselMultiAnnotation {
   def toFirrtl: Seq[Annotation]
 }
 
+@nowarn("msg=Avoid custom annotations")
 object annotate {
-  def apply(anno: ChiselAnnotation): Unit = {
-    Builder.annotations += anno
-  }
-  def apply(annos: ChiselMultiAnnotation): Unit = {
-    Builder.newAnnotations += annos
+  @deprecated(
+    "Avoid custom annotations. If you must use annotations, new annotate.apply method that takes Data",
+    "Chisel 7.0"
+  )
+  def apply(anno: ChiselAnnotation): Unit = apply()(Seq(anno.toFirrtl))
+
+  @deprecated(
+    "Avoid custom annotations. If you must use annotations, new annotate.apply method that takes Data",
+    "Chisel 7.0"
+  )
+  def apply(annos: ChiselMultiAnnotation): Unit = apply()(annos.toFirrtl)
+
+  /** Create annotations.
+    *
+    * Avoid this API if possible.
+    *
+    * Anything being annotated must be passed as arguments so that Chisel can do safety checks.
+    * The caller is still responsible for calling .toTarget on those arguments in mkAnnos.
+    */
+  def apply(targets: InstanceId*)(mkAnnos: => Seq[Annotation]): Unit = {
+    targets.foreach {
+      case d: Data => requireIsAnnotatable(d, "Data marked with annotation")
+      case _ => ()
+    }
+    // TODO record views that need to be renamed
+    Builder.annotations += (() => mkAnnos)
   }
 }
 
@@ -76,7 +107,7 @@ object doNotDedup {
     * @return Unmodified signal `module`
     */
   def apply[T <: RawModule](module: T): Unit = {
-    annotate(new ChiselAnnotation { def toFirrtl: NoDedupAnnotation = NoDedupAnnotation(module.toNamed) })
+    annotate(module)(Seq(NoDedupAnnotation(module.toNamed)))
   }
 }
 
@@ -89,6 +120,6 @@ object dedupGroup {
     * @return Unmodified signal `module`
     */
   def apply[T <: BaseModule](module: T, group: String): Unit = {
-    annotate(new ChiselAnnotation { def toFirrtl: DedupGroupAnnotation = DedupGroupAnnotation(module.toTarget, group) })
+    annotate(module)(Seq(DedupGroupAnnotation(module.toTarget, group)))
   }
 }

core/src/main/scala/chisel3/ChiselEnumImpl.scala

@@ -3,6 +3,7 @@
 package chisel3
 
 import scala.language.existentials
+import scala.annotation.nowarn
 import scala.collection.mutable
 import chisel3.experimental.{annotate, requireIsHardware, ChiselAnnotation, SourceInfo, UnlocatableSourceInfo}
 import chisel3.internal.Builder.pushOp
@@ -13,6 +14,8 @@ import chisel3.internal.binding.{Binding, ChildBinding, ConstrainedBinding}
 
 import chisel3.experimental.EnumAnnotations._
 
+// Rather than refactoring the annotation work here, we should just remove ChiselEnum annotations
+@nowarn("msg=Avoid custom annotations")
 private[chisel3] abstract class EnumTypeImpl(private[chisel3] val factory: ChiselEnum, selfAnnotating: Boolean = true)
     extends Element { self: EnumType =>
 
@@ -231,10 +234,14 @@ private[chisel3] abstract class EnumTypeImpl(private[chisel3] val factory: Chise
   }
 }
 
+// Rather than refactoring the annotation work here, we should just remove ChiselEnum annotations
+@nowarn("msg=Avoid custom annotations")
 private[chisel3] object ChiselEnumImpl {
   private[chisel3] case object CacheKey extends BuilderContextCache.Key[mutable.HashSet[ChiselAnnotation]]
 }
 
+// Rather than refactoring the annotation work here, we should just remove ChiselEnum annotations
+@nowarn("msg=Avoid custom annotations")
 private[chisel3] trait ChiselEnumImpl { self: ChiselEnum =>
   class Type extends EnumType(this)
   object Type {

core/src/main/scala/chisel3/ModuleImpl.scala

@@ -1018,10 +1018,7 @@ private object ModulePrefixAnnotation {
   def annotate[T <: HasId](target: T): Unit = {
     val prefix = Builder.getModulePrefix
     if (prefix != "") {
-      val annotation: ChiselAnnotation = new ChiselAnnotation {
-        def toFirrtl: Annotation = ModulePrefixAnnotation(target.toTarget, prefix)
-      }
-      chisel3.experimental.annotate(annotation)
+      chisel3.experimental.annotate(target)(Seq(ModulePrefixAnnotation(target.toTarget, prefix)))
     }
   }
 }

core/src/main/scala/chisel3/dontTouch.scala

@@ -39,8 +39,7 @@ object dontTouch {
       case d if DataMirror.hasProbeTypeModifier(d) => ()
       case _:   Property[_] => ()
       case agg: Aggregate   => agg.getElements.foreach(dontTouch.apply)
-      case _:   Element =>
-        annotate(new ChiselAnnotation { def toFirrtl: DontTouchAnnotation = DontTouchAnnotation(data.toNamed) })
+      case _:   Element     => annotate(data)(Seq(DontTouchAnnotation(data.toNamed)))
       case _ => throw new ChiselException("Non-hardware dontTouch")
     }
     data

core/src/main/scala/chisel3/experimental/EnumAnnotations.scala

@@ -2,9 +2,12 @@
 
 package chisel3.experimental
 
+import scala.annotation.nowarn
 import chisel3._
 import firrtl.annotations._
 
+// Rather than refactoring the annotation work here, we should just remove ChiselEnum annotations
+@nowarn("msg=Avoid custom annotations")
 object EnumAnnotations {
 
   /** An annotation for strong enum instances that are ''not'' inside of Vecs
@@ -16,6 +19,10 @@ object EnumAnnotations {
     def duplicate(n: Named): EnumComponentAnnotation = this.copy(target = n)
   }
 
+  @deprecated(
+    "Avoid custom annotations. If you must use annotations, new annotate.apply method that takes Data",
+    "Chisel 7.0"
+  )
   case class EnumComponentChiselAnnotation(target: InstanceId, enumTypeName: String) extends ChiselAnnotation {
     def toFirrtl: EnumComponentAnnotation = EnumComponentAnnotation(target.toNamed, enumTypeName)
   }
@@ -45,6 +52,10 @@ object EnumAnnotations {
     def duplicate(n: Named): EnumVecAnnotation = this.copy(target = n)
   }
 
+  @deprecated(
+    "Avoid custom annotations. If you must use annotations, new annotate.apply method that takes Data",
+    "Chisel 7.0"
+  )
   case class EnumVecChiselAnnotation(target: InstanceId, typeName: String, fields: Seq[Seq[String]])
       extends ChiselAnnotation {
     override def toFirrtl: EnumVecAnnotation = EnumVecAnnotation(target.toNamed, typeName, fields)
@@ -57,6 +68,10 @@ object EnumAnnotations {
     */
   case class EnumDefAnnotation(typeName: String, definition: Map[String, BigInt]) extends NoTargetAnnotation
 
+  @deprecated(
+    "Avoid custom annotations. If you must use annotations, new annotate.apply method that takes Data",
+    "Chisel 7.0"
+  )
   case class EnumDefChiselAnnotation(typeName: String, definition: Map[String, BigInt]) extends ChiselAnnotation {
     override def toFirrtl: Annotation = EnumDefAnnotation(typeName, definition)
   }

core/src/main/scala/chisel3/experimental/Trace.scala

@@ -23,9 +23,7 @@ object Trace {
 
   /** Trace a Instance name. */
   def traceName(x: RawModule): Unit = {
-    annotate(new ChiselAnnotation {
-      def toFirrtl: Annotation = TraceAnnotation(x.toAbsoluteTarget, x.toAbsoluteTarget)
-    })
+    annotate(x)(Seq(TraceAnnotation(x.toAbsoluteTarget, x.toAbsoluteTarget)))
   }
 
   /** Trace a Data name. This does NOT add "don't touch" semantics to the traced data. If you want this behavior, use an explicit [[chisel3.dontTouch]]. */
@@ -34,9 +32,7 @@ object Trace {
       case aggregate: Aggregate =>
         aggregate.elementsIterator.foreach(traceName)
       case element: Element =>
-        annotate(new ChiselAnnotation {
-          def toFirrtl: Annotation = TraceAnnotation(element.toAbsoluteTarget, element.toAbsoluteTarget)
-        })
+        annotate(element)(Seq(TraceAnnotation(element.toAbsoluteTarget, element.toAbsoluteTarget)))
     }
   }
 

core/src/main/scala/chisel3/internal/Builder.scala

@@ -23,7 +23,7 @@ import chisel3.internal.Builder.Prefix
 import logger.{LazyLogging, LoggerOption}
 
 import scala.collection.mutable
-import scala.annotation.tailrec
+import scala.annotation.{nowarn, tailrec}
 import java.io.File
 import scala.util.control.NonFatal
 import chisel3.ChiselException
@@ -476,6 +476,7 @@ private[chisel3] class ChiselContext() {
   var modulePrefixStack: List[(String, Boolean)] = Nil
 }
 
+@nowarn("msg=Avoid custom annotations")
 private[chisel3] class DynamicContext(
   val annotationSeq:       AnnotationSeq,
   val throwOnFirstError:   Boolean,
@@ -541,8 +542,7 @@ private[chisel3] class DynamicContext(
   }
 
   val components = ArrayBuffer[Component]()
-  val annotations = ArrayBuffer[ChiselAnnotation]()
-  val newAnnotations = ArrayBuffer[ChiselMultiAnnotation]()
+  val annotations = ArrayBuffer[() => Seq[Annotation]]()
   val layers = mutable.LinkedHashSet[layer.Layer]()
   val options = mutable.LinkedHashSet[choice.Case]()
   var currentModule: Option[BaseModule] = None
@@ -570,6 +570,7 @@ private[chisel3] class DynamicContext(
   var inDefinition: Boolean = false
 }
 
+@nowarn("msg=Avoid custom annotations")
 private[chisel3] object Builder extends LazyLogging {
 
   // Represents the current state of the prefixes given
@@ -625,16 +626,13 @@ private[chisel3] object Builder extends LazyLogging {
   def components:  ArrayBuffer[Component] = dynamicContext.components
   def definitions: ArrayBuffer[Definition[_]] = dynamicContext.definitions
 
-  def annotations: ArrayBuffer[ChiselAnnotation] = dynamicContext.annotations
+  def annotations: ArrayBuffer[() => Seq[Annotation]] = dynamicContext.annotations
 
   def layers:  mutable.LinkedHashSet[layer.Layer] = dynamicContext.layers
   def options: mutable.LinkedHashSet[choice.Case] = dynamicContext.options
 
   def contextCache: BuilderContextCache = dynamicContext.contextCache
 
-  // TODO : Unify this with annotations in the future - done this way for backward compatability
-  def newAnnotations: ArrayBuffer[ChiselMultiAnnotation] = dynamicContext.newAnnotations
-
   def annotationSeq:         AnnotationSeq = dynamicContext.annotationSeq
   def namingStack:           NamingStack = dynamicContext.namingStack
   def importedDefinitionMap: Map[String, String] = dynamicContext.importedDefinitionMap
@@ -1160,7 +1158,6 @@ private[chisel3] object Builder extends LazyLogging {
           components.toSeq,
           annotations.toSeq,
           makeViewRenameMap,
-          newAnnotations.toSeq,
           typeAliases,
           layerAdjacencyList(layer.Layer.Root).map(foldLayers).toSeq,
           optionDefs

core/src/main/scala/chisel3/internal/firrtl/IR.scala

@@ -568,50 +568,16 @@ private[chisel3] object ir {
   case class DefClass(id: Class, name: String, ports: Seq[Port], block: Block) extends Component
 
   case class Circuit(
-    name:           String,
-    components:     Seq[Component],
-    annotations:    Seq[ChiselAnnotation],
-    renames:        RenameMap,
-    newAnnotations: Seq[ChiselMultiAnnotation],
-    typeAliases:    Seq[DefTypeAlias],
-    layers:         Seq[Layer],
-    options:        Seq[DefOption]
+    name:        String,
+    components:  Seq[Component],
+    annotations: Seq[() => Seq[Annotation]],
+    renames:     RenameMap,
+    typeAliases: Seq[DefTypeAlias],
+    layers:      Seq[Layer],
+    options:     Seq[DefOption]
   ) {
 
-    def this(
-      name:        String,
-      components:  Seq[Component],
-      annotations: Seq[ChiselAnnotation],
-      renames:     RenameMap,
-      typeAliases: Seq[DefTypeAlias],
-      layers:      Seq[Layer],
-      options:     Seq[DefOption]
-    ) =
-      this(name, components, annotations, renames, Seq.empty, typeAliases, layers, options)
-
     def firrtlAnnotations: Iterable[Annotation] =
-      annotations.flatMap(_.toFirrtl.update(renames)) ++ newAnnotations.flatMap(
-        _.toFirrtl.flatMap(_.update(renames))
-      )
-  }
-
-  object Circuit
-      extends scala.runtime.AbstractFunction7[String, Seq[Component], Seq[ChiselAnnotation], RenameMap, Seq[
-        DefTypeAlias
-      ], Seq[Layer], Seq[DefOption], Circuit] {
-    def unapply(c: Circuit): Option[(String, Seq[Component], Seq[ChiselAnnotation], RenameMap, Seq[DefTypeAlias])] = {
-      Some((c.name, c.components, c.annotations, c.renames, c.typeAliases))
-    }
-
-    def apply(
-      name:        String,
-      components:  Seq[Component],
-      annotations: Seq[ChiselAnnotation],
-      renames:     RenameMap,
-      typeAliases: Seq[DefTypeAlias] = Seq.empty,
-      layers:      Seq[Layer] = Seq.empty,
-      options:     Seq[DefOption] = Seq.empty
-    ): Circuit =
-      new Circuit(name, components, annotations, renames, typeAliases, layers, options)
+      annotations.flatMap(_().flatMap(_.update(renames)))
   }
 }

src/main/scala/chisel3/util/AttributeAnnotation.scala

@@ -35,9 +35,6 @@ object addAttribute {
     * @param annoString attribute string to add to target.
     */
   def apply(target: Data, annoString: String): Unit = {
-    requireIsAnnotatable(target, "target must be annotatable")
-    annotate(new ChiselAnnotation {
-      def toFirrtl = AttributeAnnotation(target.toNamed, annoString)
-    })
+    annotate(target)(Seq(AttributeAnnotation(target.toNamed, annoString)))
   }
 }

src/main/scala/chisel3/util/BlackBoxUtils.scala

@@ -45,10 +45,7 @@ trait HasBlackBoxResource extends BlackBox {
     * }}}
     */
   def addResource(blackBoxResource: String): Unit = {
-    val anno = new ChiselAnnotation {
-      def toFirrtl = BlackBoxInlineAnno.fromResource(blackBoxResource, self.toNamed)
-    }
-    chisel3.experimental.annotate(anno)
+    chisel3.experimental.annotate(self)(Seq(BlackBoxInlineAnno.fromResource(blackBoxResource, self.toNamed)))
   }
 }
 
@@ -61,10 +58,7 @@ trait HasBlackBoxInline extends BlackBox {
     * @param blackBoxInline The black box contents
     */
   def setInline(blackBoxName: String, blackBoxInline: String): Unit = {
-    val anno = new ChiselAnnotation {
-      def toFirrtl = BlackBoxInlineAnno(self.toNamed, blackBoxName, blackBoxInline)
-    }
-    chisel3.experimental.annotate(anno)
+    chisel3.experimental.annotate(self)(Seq(BlackBoxInlineAnno(self.toNamed, blackBoxName, blackBoxInline)))
   }
 }
 
@@ -78,9 +72,6 @@ trait HasBlackBoxPath extends BlackBox {
     * target directory.
     */
   def addPath(blackBoxPath: String): Unit = {
-    val anno = new ChiselAnnotation {
-      def toFirrtl = BlackBoxPathAnno(self.toNamed, blackBoxPath)
-    }
-    chisel3.experimental.annotate(anno)
+    chisel3.experimental.annotate(self)(Seq(BlackBoxPathAnno(self.toNamed, blackBoxPath)))
   }
 }