Deprecate CompilerAnnotation (#1870)

* CompilerAnnotation$ emits RunFirrtlTransform

Change the CompilerAnnotation object to emit
RunFirrtlTransformAnnotations containing the associated emitter.

This requires a fix in the Driver compatibility layer to know how to
enable one-file-per module emission if either a CompilerAnnotation or
a RunFirrtlTransformAnnotation(_: Emitter) is present.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>

* Add ConvertCompilerAnnotation phase

Add a phase, ConvertCompilerAnnotation, that converts a
CompilerAnnotation to a RunFirrtlTransformAnnotation. This provides a
warning to the user if this path is taken.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>

* Add test of ConvertCompilerAnnotation

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>

* Deprecate CompilerAnnotation$, move helper methods

Deprecate the CompilerAnnotation companion object and move it's
private utility inside the RunFirrtlTransformAnnotation companion
object.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>

* Make ConvertCompilerAnnotations private[firrtl]

Make this phase private to avoid adding a deprecation warning. Also,
remove an unused string value.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>

* Fix incorrect string in test

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>

* Add test that '-X verilog', no emitter yields file

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>

8 files changed, 82 insertions, 32 deletions

diff --git a/src/main/scala/firrtl/ExecutionOptionsManager.scala b/src/main/scala/firrtl/ExecutionOptionsManager.scala
index 50fb30a6..e673a5af 100644
--- a/src/main/scala/firrtl/ExecutionOptionsManager.scala
+++ b/src/main/scala/firrtl/ExecutionOptionsManager.scala
@@ -343,7 +343,7 @@ case class FirrtlExecutionOptions(
     List() ++ (if (inputFileNameOverride.nonEmpty) Seq(FirrtlFileAnnotation(inputFileNameOverride)) else Seq()) ++
       (if (outputFileNameOverride.nonEmpty) { Some(OutputFileAnnotation(outputFileNameOverride)) }
        else { None }) ++
-      Some(CompilerAnnotation(compilerName)) ++
+      Some(RunFirrtlTransformAnnotation.stringToEmitter(compilerName)) ++
       Some(InfoModeAnnotation(infoModeName)) ++
       firrtlSource.map(FirrtlSourceAnnotation(_)) ++
       customTransforms.map(t => RunFirrtlTransformAnnotation(t)) ++
diff --git a/src/main/scala/firrtl/stage/FirrtlAnnotations.scala b/src/main/scala/firrtl/stage/FirrtlAnnotations.scala
index d37d2881..3390e632 100644
--- a/src/main/scala/firrtl/stage/FirrtlAnnotations.scala
+++ b/src/main/scala/firrtl/stage/FirrtlAnnotations.scala
@@ -152,28 +152,16 @@ object FirrtlSourceAnnotation extends HasShellOptions {
   *  - If unset, a [[CompilerAnnotation]] with the default [[VerilogCompiler]]
   * @param compiler compiler name
   */
+@deprecated("Use a RunFirrtlTransformAnnotation targeting a specific Emitter.", "FIRRTL 1.4.0")
 case class CompilerAnnotation(compiler: Compiler = new VerilogCompiler()) extends NoTargetAnnotation with FirrtlOption
 
+@deprecated("Use a RunFirrtlTransformAnnotation targeting a specific Emitter.", "FIRRTL 1.4.0")
 object CompilerAnnotation extends HasShellOptions {
 
-  private[firrtl] def apply(compilerName: String): CompilerAnnotation = {
-    val c = compilerName match {
-      case "none"     => new NoneCompiler()
-      case "high"     => new HighFirrtlCompiler()
-      case "low"      => new LowFirrtlCompiler()
-      case "middle"   => new MiddleFirrtlCompiler()
-      case "verilog"  => new VerilogCompiler()
-      case "mverilog" => new MinimumVerilogCompiler()
-      case "sverilog" => new SystemVerilogCompiler()
-      case _          => throw new OptionsException(s"Unknown compiler name '$compilerName'! (Did you misspell it?)")
-    }
-    CompilerAnnotation(c)
-  }
-
   val options = Seq(
     new ShellOption[String](
       longOption = "compiler",
-      toAnnotationSeq = a => Seq(CompilerAnnotation(a)),
+      toAnnotationSeq = a => Seq(RunFirrtlTransformAnnotation.stringToEmitter(a)),
       helpText = "The FIRRTL compiler to use (default: verilog)",
       shortOption = Some("X"),
       helpValueName = Some("<none|high|middle|low|verilog|mverilog|sverilog>")
@@ -194,6 +182,20 @@ object RunFirrtlTransformAnnotation extends HasShellOptions {
   def apply(transform: TransformDependency): RunFirrtlTransformAnnotation =
     RunFirrtlTransformAnnotation(transform.getObject)
 
+  private[firrtl] def stringToEmitter(a: String): RunFirrtlTransformAnnotation = {
+    val emitter = a match {
+      case "none"     => new ChirrtlEmitter
+      case "high"     => new HighFirrtlEmitter
+      case "low"      => new LowFirrtlEmitter
+      case "middle"   => new MiddleFirrtlEmitter
+      case "verilog"  => new VerilogEmitter
+      case "mverilog" => new MinimumVerilogEmitter
+      case "sverilog" => new SystemVerilogEmitter
+      case _          => throw new OptionsException(s"Unknown compiler name '$a'! (Did you misspell it?)")
+    }
+    RunFirrtlTransformAnnotation(emitter)
+  }
+
   val options = Seq(
     new ShellOption[Seq[String]](
       longOption = "custom-transforms",
@@ -225,6 +227,13 @@ object RunFirrtlTransformAnnotation extends HasShellOptions {
       },
       helpText = "Convert all FIRRTL names to a specific case",
       helpValueName = Some("<lower|upper>")
+    ),
+    new ShellOption[String](
+      longOption = "compiler",
+      toAnnotationSeq = a => Seq(stringToEmitter(a)),
+      helpText = "The FIRRTL compiler to use (default: verilog)",
+      shortOption = Some("X"),
+      helpValueName = Some("<none|high|middle|low|verilog|mverilog|sverilog>")
     )
   )
 
diff --git a/src/main/scala/firrtl/stage/FirrtlCli.scala b/src/main/scala/firrtl/stage/FirrtlCli.scala
index fb5aa09f..c33980dd 100644
--- a/src/main/scala/firrtl/stage/FirrtlCli.scala
+++ b/src/main/scala/firrtl/stage/FirrtlCli.scala
@@ -16,7 +16,6 @@ trait FirrtlCli { this: Shell =>
     OutputFileAnnotation,
     InfoModeAnnotation,
     FirrtlSourceAnnotation,
-    CompilerAnnotation,
     RunFirrtlTransformAnnotation,
     firrtl.EmitCircuitAnnotation,
     firrtl.EmitAllModulesAnnotation,
diff --git a/src/main/scala/firrtl/stage/FirrtlStage.scala b/src/main/scala/firrtl/stage/FirrtlStage.scala
index 58d07e43..b92fc4ef 100644
--- a/src/main/scala/firrtl/stage/FirrtlStage.scala
+++ b/src/main/scala/firrtl/stage/FirrtlStage.scala
@@ -7,7 +7,13 @@ import firrtl.options.{Dependency, Phase, PhaseManager, Shell, Stage, StageMain}
 import firrtl.options.phases.DeletedWrapper
 import firrtl.stage.phases.CatchExceptions
 
-class FirrtlPhase extends PhaseManager(targets = Seq(Dependency[firrtl.stage.phases.Compiler])) {
+class FirrtlPhase
+    extends PhaseManager(
+      targets = Seq(
+        Dependency[firrtl.stage.phases.Compiler],
+        Dependency[firrtl.stage.phases.ConvertCompilerAnnotations]
+      )
+    ) {
 
   override def invalidates(a: Phase) = false
 
diff --git a/src/main/scala/firrtl/stage/phases/AddDefaults.scala b/src/main/scala/firrtl/stage/phases/AddDefaults.scala
index 9f4163cc..b296b59d 100644
--- a/src/main/scala/firrtl/stage/phases/AddDefaults.scala
+++ b/src/main/scala/firrtl/stage/phases/AddDefaults.scala
@@ -6,7 +6,7 @@ import firrtl.{AnnotationSeq, VerilogEmitter}
 import firrtl.options.{Dependency, Phase, TargetDirAnnotation}
 import firrtl.stage.TransformManager.TransformDependency
 import firrtl.transforms.BlackBoxTargetDirAnno
-import firrtl.stage.{CompilerAnnotation, FirrtlOptions, InfoModeAnnotation, RunFirrtlTransformAnnotation}
+import firrtl.stage.{FirrtlOptions, InfoModeAnnotation, RunFirrtlTransformAnnotation}
 
 /** [[firrtl.options.Phase Phase]] that adds default [[FirrtlOption]] [[firrtl.annotations.Annotation Annotation]]s.
   * This is a part of the preprocessing done by [[FirrtlStage]].
@@ -23,10 +23,9 @@ class AddDefaults extends Phase {
 
   /** Append any missing default annotations to an annotation sequence */
   def transform(annotations: AnnotationSeq): AnnotationSeq = {
-    var bb, c, em, im = true
+    var bb, em, im = true
     annotations.foreach {
       case _: BlackBoxTargetDirAnno => bb = false
-      case _: CompilerAnnotation    => c = false
       case _: InfoModeAnnotation    => im = false
       case RunFirrtlTransformAnnotation(_: firrtl.Emitter) => em = false
       case _ =>
@@ -39,7 +38,7 @@ class AddDefaults extends Phase {
 
     (if (bb) Seq(BlackBoxTargetDirAnno(targetDir)) else Seq()) ++
       // if there is no compiler or emitter specified, add the default emitter
-      (if (c && em) Seq(RunFirrtlTransformAnnotation(DefaultEmitterTarget)) else Seq()) ++
+      (if (em) Seq(RunFirrtlTransformAnnotation(DefaultEmitterTarget)) else Seq()) ++
       (if (im) Seq(InfoModeAnnotation()) else Seq()) ++
       annotations
   }
diff --git a/src/main/scala/firrtl/stage/phases/Checks.scala b/src/main/scala/firrtl/stage/phases/Checks.scala
index 24cfa0a3..44828ae6 100644
--- a/src/main/scala/firrtl/stage/phases/Checks.scala
+++ b/src/main/scala/firrtl/stage/phases/Checks.scala
@@ -31,14 +31,13 @@ class Checks extends Phase {
     * @throws firrtl.options.OptionsException if any checks fail
     */
   def transform(annos: AnnotationSeq): AnnotationSeq = {
-    val inF, inS, eam, ec, outF, comp, emitter, im, inC = collection.mutable.ListBuffer[Annotation]()
+    val inF, inS, eam, ec, outF, emitter, im, inC = collection.mutable.ListBuffer[Annotation]()
     annos.foreach(_ match {
       case a: FirrtlFileAnnotation     => a +=: inF
       case a: FirrtlSourceAnnotation   => a +=: inS
       case a: EmitAllModulesAnnotation => a +=: eam
       case a: EmitCircuitAnnotation    => a +=: ec
       case a: OutputFileAnnotation     => a +=: outF
-      case a: CompilerAnnotation       => a +=: comp
       case a: InfoModeAnnotation       => a +=: im
       case a: FirrtlCircuitAnnotation  => a +=: inC
       case a @ RunFirrtlTransformAnnotation(_: firrtl.Emitter) => a +=: emitter
@@ -81,13 +80,13 @@ class Checks extends Phase {
       )
     }
 
-    /* One mandatory compiler (or emitter) must be specified */
-    if (comp.size != 1 && emitter.isEmpty) {
-      val x = comp.map { case CompilerAnnotation(x) => x }
-      val (msg, suggest) = if (comp.size == 0) { ("none found", "forget one of") }
-      else { (s"""found '${x.mkString(", ")}'""", "use multiple of") }
-      throw new OptionsException(s"""|Exactly one compiler must be specified, but $msg. Did you $suggest the following?
-                                     |    - an option or annotation: -X, --compiler, CompilerAnnotation""".stripMargin)
+    /* At least one emitter must be specified */
+    if (emitter.isEmpty) {
+      throw new OptionsException(
+        s"""|At least one compiler must be specified, but none found. Specify a compiler via:
+            |    - a RunFirrtlTransformAnnotation targeting a specific emitter, e.g., VerilogEmitter
+            |    - a command line option: -X, --compiler""".stripMargin
+      )
     }
 
     /* One mandatory info mode must be specified */
diff --git a/src/main/scala/firrtl/stage/phases/ConvertCompilerAnnotations.scala b/src/main/scala/firrtl/stage/phases/ConvertCompilerAnnotations.scala
new file mode 100644
index 00000000..b4743cde
--- /dev/null
+++ b/src/main/scala/firrtl/stage/phases/ConvertCompilerAnnotations.scala
@@ -0,0 +1,35 @@
+// See LICENSE for license details.
+
+package firrtl.stage.phases
+
+import firrtl.AnnotationSeq
+import firrtl.options.{Dependency, OptionsException, Phase}
+import firrtl.stage.{CompilerAnnotation, RunFirrtlTransformAnnotation}
+
+private[firrtl] class ConvertCompilerAnnotations extends Phase {
+
+  override def prerequisites = Seq.empty
+  override def optionalPrerequisites = Seq.empty
+  override def optionalPrerequisiteOf = Seq(Dependency[AddDefaults], Dependency[Checks])
+  override def invalidates(a: Phase) = false
+
+  override def transform(annotations: AnnotationSeq): AnnotationSeq = {
+    annotations.collect {
+      case a: CompilerAnnotation => a
+    } match {
+      case a if a.size > 1 =>
+        throw new OptionsException(
+          s"Zero or one CompilerAnnotation may be specified, but found '${a.mkString(", ")}'.".stripMargin
+        )
+      case _ =>
+    }
+    annotations.map {
+      case CompilerAnnotation(a) =>
+        val suggestion = s"RunFirrtlTransformAnnotation(new ${a.emitter.getClass.getName})"
+        logger.warn(s"CompilerAnnotation is deprecated since FIRRTL 1.4.0. Please use '$suggestion' instead.")
+        RunFirrtlTransformAnnotation(a.emitter)
+      case a => a
+    }
+  }
+
+}
diff --git a/src/main/scala/firrtl/stage/phases/DriverCompatibility.scala b/src/main/scala/firrtl/stage/phases/DriverCompatibility.scala
index 0b558cc0..529f9ca3 100644
--- a/src/main/scala/firrtl/stage/phases/DriverCompatibility.scala
+++ b/src/main/scala/firrtl/stage/phases/DriverCompatibility.scala
@@ -4,7 +4,7 @@ package firrtl.stage.phases
 
 import firrtl.stage._
 
-import firrtl.{AnnotationSeq, EmitAllModulesAnnotation, EmitCircuitAnnotation, FirrtlExecutionResult, Parser}
+import firrtl.{AnnotationSeq, EmitAllModulesAnnotation, EmitCircuitAnnotation, Emitter, FirrtlExecutionResult, Parser}
 import firrtl.annotations.NoTargetAnnotation
 import firrtl.FileUtils
 import firrtl.proto.FromProto
@@ -227,6 +227,9 @@ object DriverCompatibility {
           val b = RunFirrtlTransformAnnotation(a.compiler.emitter)
           if (splitModules) { Seq(a, b, EmitAllModulesAnnotation(c.emitter.getClass)) }
           else { Seq(a, b, EmitCircuitAnnotation(c.emitter.getClass)) }
+        case a @ RunFirrtlTransformAnnotation(e: Emitter) =>
+          if (splitModules) { Seq(a, EmitAllModulesAnnotation(e.getClass)) }
+          else { Seq(a, EmitCircuitAnnotation(e.getClass)) }
         case a => Seq(a)
       }
     }