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>

5 files changed, 61 insertions, 11 deletions

diff --git a/src/test/scala/firrtl/stage/phases/tests/ConvertCompilerAnnotationsSpec.scala b/src/test/scala/firrtl/stage/phases/tests/ConvertCompilerAnnotationsSpec.scala
new file mode 100644
index 00000000..acd37bf5
--- /dev/null
+++ b/src/test/scala/firrtl/stage/phases/tests/ConvertCompilerAnnotationsSpec.scala
@@ -0,0 +1,40 @@
+// See LICENSE for license details.
+
+package firrtl.stage.phases.tests
+
+import firrtl.{HighFirrtlCompiler, HighFirrtlEmitter, LowFirrtlCompiler}
+import firrtl.options.{Dependency, OptionsException}
+import firrtl.stage.{CompilerAnnotation, RunFirrtlTransformAnnotation}
+import firrtl.stage.phases.ConvertCompilerAnnotations
+
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+class ConvertCompilerAnnotationsSpec extends AnyFlatSpec with Matchers {
+
+  class Fixture {
+    val phase = new ConvertCompilerAnnotations
+  }
+
+  behavior.of(classOf[ConvertCompilerAnnotations].getName)
+
+  it should "convert a deprecated CompilerAnnotation to a RunFirrtlTransformAnnotation" in new Fixture {
+    val annotations = Seq(CompilerAnnotation(new HighFirrtlCompiler))
+    phase
+      .transform(annotations)
+      .map {
+        case RunFirrtlTransformAnnotation(a) => Dependency.fromTransform(a)
+      }
+      .toSeq should be(Seq(Dependency[HighFirrtlEmitter]))
+  }
+
+  it should "throw an exception if multiple CompilerAnnotations are specified" in new Fixture {
+    val annotations = Seq(
+      CompilerAnnotation(new HighFirrtlCompiler),
+      CompilerAnnotation(new LowFirrtlCompiler)
+    )
+    intercept[OptionsException] {
+      phase.transform(annotations)
+    }.getMessage should include("Zero or one CompilerAnnotation may be specified")
+  }
+}
diff --git a/src/test/scala/firrtl/testutils/FirrtlSpec.scala b/src/test/scala/firrtl/testutils/FirrtlSpec.scala
index a0c41085..9ee5fdaf 100644
--- a/src/test/scala/firrtl/testutils/FirrtlSpec.scala
+++ b/src/test/scala/firrtl/testutils/FirrtlSpec.scala
@@ -101,7 +101,7 @@ trait FirrtlRunners extends BackendCompilationUtilities {
         InfoModeAnnotation("ignore") +:
         RenameTopAnnotation(topName) +:
         stage.FirrtlCircuitAnnotation(circuit) +:
-        stage.CompilerAnnotation("mverilog") +:
+        stage.RunFirrtlTransformAnnotation.stringToEmitter("mverilog") +:
         stage.OutputFileAnnotation(topName) +:
         toAnnos(baseTransforms)
     }
diff --git a/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala b/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala
index 9274bac6..ee942d85 100644
--- a/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala
+++ b/src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala
@@ -286,6 +286,12 @@ class FirrtlMainSpec
         args = Array("-X", "sverilog", "-E", "sverilog", "-o", "Foo"),
         stdout = defaultStdOut,
         files = Seq("Foo.sv")
+      ),
+      /* Test that an output is generated if no emitter is specified */
+      FirrtlMainTest(
+        args = Array("-X", "verilog", "-o", "Foo"),
+        stdout = defaultStdOut,
+        files = Seq("Foo.v")
       )
     )
       .foreach(runStageExpectFiles)
diff --git a/src/test/scala/firrtlTests/stage/phases/AddDefaultsSpec.scala b/src/test/scala/firrtlTests/stage/phases/AddDefaultsSpec.scala
index 686c42ad..eca7b706 100644
--- a/src/test/scala/firrtlTests/stage/phases/AddDefaultsSpec.scala
+++ b/src/test/scala/firrtlTests/stage/phases/AddDefaultsSpec.scala
@@ -2,11 +2,11 @@
 
 package firrtlTests.stage.phases
 
-import firrtl.NoneCompiler
+import firrtl.ChirrtlEmitter
 import firrtl.annotations.Annotation
 import firrtl.stage.phases.AddDefaults
 import firrtl.transforms.BlackBoxTargetDirAnno
-import firrtl.stage.{CompilerAnnotation, InfoModeAnnotation, RunFirrtlTransformAnnotation}
+import firrtl.stage.{InfoModeAnnotation, RunFirrtlTransformAnnotation}
 import firrtl.options.{Dependency, Phase, TargetDirAnnotation}
 import org.scalatest.flatspec.AnyFlatSpec
 import org.scalatest.matchers.should.Matchers
@@ -32,7 +32,11 @@ class AddDefaultsSpec extends AnyFlatSpec with Matchers {
   }
 
   it should "not overwrite existing annotations" in new Fixture {
-    val input = Seq(BlackBoxTargetDirAnno("foo"), CompilerAnnotation(new NoneCompiler()), InfoModeAnnotation("ignore"))
+    val input = Seq(
+      BlackBoxTargetDirAnno("foo"),
+      RunFirrtlTransformAnnotation(new ChirrtlEmitter),
+      InfoModeAnnotation("ignore")
+    )
 
     phase.transform(input).toSeq should be(input)
   }
diff --git a/src/test/scala/firrtlTests/stage/phases/ChecksSpec.scala b/src/test/scala/firrtlTests/stage/phases/ChecksSpec.scala
index 65516da5..6c0335b9 100644
--- a/src/test/scala/firrtlTests/stage/phases/ChecksSpec.scala
+++ b/src/test/scala/firrtlTests/stage/phases/ChecksSpec.scala
@@ -4,7 +4,7 @@ package firrtlTests.stage.phases
 
 import firrtl.stage._
 
-import firrtl.{AnnotationSeq, ChirrtlEmitter, EmitAllModulesAnnotation, NoneCompiler}
+import firrtl.{AnnotationSeq, ChirrtlEmitter, EmitAllModulesAnnotation}
 import firrtl.options.{OptionsException, OutputAnnotationFileAnnotation, Phase}
 import firrtl.stage.phases.Checks
 import org.scalatest.flatspec.AnyFlatSpec
@@ -18,7 +18,7 @@ class ChecksSpec extends AnyFlatSpec with Matchers {
   val outputFile = OutputFileAnnotation("bar")
   val emitAllModules = EmitAllModulesAnnotation(classOf[ChirrtlEmitter])
   val outputAnnotationFile = OutputAnnotationFileAnnotation("baz")
-  val goodCompiler = CompilerAnnotation(new NoneCompiler())
+  val goodCompiler = RunFirrtlTransformAnnotation(new ChirrtlEmitter)
   val infoMode = InfoModeAnnotation("ignore")
 
   val min = Seq(inputFile, goodCompiler, infoMode)
@@ -54,15 +54,15 @@ class ChecksSpec extends AnyFlatSpec with Matchers {
     checkExceptionMessage(phase, in, "No more than one output file can be specified")
   }
 
-  it should "enforce exactly one compiler" in new Fixture {
+  it should "enforce one or more compilers (at this point these are emitters)" in new Fixture {
     info("0 compilers should throw an exception")
     val inZero = Seq(inputFile, infoMode)
-    checkExceptionMessage(phase, inZero, "Exactly one compiler must be specified, but none found")
+    checkExceptionMessage(phase, inZero, "At least one compiler must be specified")
 
-    info("2 compilers should throw an exception")
-    val c = goodCompiler.compiler
+    info("2 compilers should not throw an exception")
+    val c = goodCompiler
     val inTwo = min :+ goodCompiler
-    checkExceptionMessage(phase, inTwo, s"Exactly one compiler must be specified, but found '$c, $c'")
+    phase.transform(inTwo)
   }
 
   it should "validate info mode names" in new Fixture {