Deprecate all mutable methods on RenameMap (#2444)

* Add renamemap.MutableRenameMap which includes these methods without
  deprecation
* Deprecate Stringly typed RenameMap APIs which were accidentally
  undeprecated a while ago

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

src/main/scala/firrtl/annotations/transforms/CleanupNamedTargets.scala

@@ -7,6 +7,7 @@ import firrtl.annotations.{CircuitTarget, ModuleTarget, MultiTargetAnnotation, R
 import firrtl.ir
 import firrtl.options.{Dependency, PreservesAll}
 import firrtl.traversals.Foreachers._
+import firrtl.renamemap.MutableRenameMap
 
 import scala.collection.immutable.{Set => ISet}
 
@@ -31,7 +32,7 @@ class CleanupNamedTargets extends Transform with DependencyAPIMigration {
     statement: ir.Statement
   )(
     implicit references: ISet[ReferenceTarget],
-    renameMap:           RenameMap,
+    renameMap:           MutableRenameMap,
     module:              ModuleTarget
   ): Unit = statement match {
     case ir.DefInstance(_, a, b, _) if references(module.instOf(a, b).asReference) =>
@@ -43,7 +44,7 @@ class CleanupNamedTargets extends Transform with DependencyAPIMigration {
     module: ir.DefModule
   )(
     implicit references: ISet[ReferenceTarget],
-    renameMap:           RenameMap,
+    renameMap:           MutableRenameMap,
     circuit:             CircuitTarget
   ): Unit = {
     implicit val mTarget = circuit.module(module.name)
@@ -60,7 +61,7 @@ class CleanupNamedTargets extends Transform with DependencyAPIMigration {
       case a: ReferenceTarget => a
     }.toSet
 
-    implicit val renameMap = RenameMap()
+    implicit val renameMap = MutableRenameMap()
 
     implicit val cTarget = CircuitTarget(state.circuit.main)
 

src/main/scala/firrtl/annotations/transforms/EliminateTargetPaths.scala

@@ -10,6 +10,7 @@ import firrtl.annotations.analysis.DuplicationHelper
 import firrtl.annotations._
 import firrtl.ir._
 import firrtl.{AnnotationSeq, CircuitState, DependencyAPIMigration, FirrtlInternalException, RenameMap, Transform}
+import firrtl.renamemap.MutableRenameMap
 import firrtl.stage.Forms
 import firrtl.transforms.DedupedResult
 import firrtl.transforms.DedupAnnotationsTransform
@@ -45,7 +46,12 @@ case class NoSuchTargetException(message: String) extends FirrtlInternalExceptio
 
 object EliminateTargetPaths {
 
-  def renameModules(c: Circuit, toRename: Map[String, String], renameMap: RenameMap): Circuit = {
+  @deprecated("Use version that accepts renamemap.MutableRenameMap", "FIRRTL 1.5")
+  def renameModules(c: Circuit, toRename: Map[String, String], renameMap: RenameMap): Circuit =
+    // Cast is safe because RenameMap is sealed trait, MutableRenameMap is only subclass
+    renameModules(c, toRename, renameMap.asInstanceOf[MutableRenameMap])
+
+  def renameModules(c: Circuit, toRename: Map[String, String], renameMap: MutableRenameMap): Circuit = {
     val ct = CircuitTarget(c.main)
     val cx = if (toRename.contains(c.main)) {
       renameMap.record(ct, CircuitTarget(toRename(c.main)))
@@ -159,7 +165,7 @@ class EliminateTargetPaths extends Transform with DependencyAPIMigration {
     lazy val finalModuleSet = finalModuleList.map { case a: DefModule => a.name }.toSet
 
     // Records how targets have been renamed
-    val renameMap = RenameMap()
+    val renameMap = MutableRenameMap()
 
     /* Foreach target, calculate the pathless version and only rename targets that are instantiated. Additionally, rename
      * module targets
@@ -264,7 +270,7 @@ class EliminateTargetPaths extends Transform with DependencyAPIMigration {
       val cache = mutable.Map.empty[String, Boolean]
       mod => cache.getOrElseUpdate(mod, iGraph.findInstancesInHierarchy(mod).size == 1)
     }
-    val firstRenameMap = RenameMap()
+    val firstRenameMap = MutableRenameMap()
     val nonSingletonTargets = targets.foldRight(Seq.empty[IsMember]) {
       case (t: IsComponent, acc) if t.asPath.nonEmpty =>
         val origPath = t.asPath
@@ -298,11 +304,13 @@ class EliminateTargetPaths extends Transform with DependencyAPIMigration {
 
     val (newCircuit, nextRenameMap, newAnnos) = run(state.circuit, nonSingletonTargets, iGraph)
 
-    val renameMap =
+    val renameMap: MutableRenameMap =
       if (firstRenameMap.hasChanges) {
-        firstRenameMap.andThen(nextRenameMap)
+        // Cast is safe because RenameMap is sealed trait, MutableRenameMap is only subclass
+        firstRenameMap.andThen(nextRenameMap).asInstanceOf[MutableRenameMap]
       } else {
-        nextRenameMap
+        // Cast is safe because RenameMap is sealed trait, MutableRenameMap is only subclass
+        nextRenameMap.asInstanceOf[MutableRenameMap]
       }
 
     val iGraphx = InstanceKeyGraph(newCircuit)

src/main/scala/firrtl/backends/experimental/smt/StutteringClockTransform.scala

@@ -11,6 +11,7 @@ import firrtl.passes.PassException
 import firrtl.stage.Forms
 import firrtl.stage.TransformManager.TransformDependency
 import firrtl.transforms.PropagatePresetAnnotations
+import firrtl.renamemap.MutableRenameMap
 
 import scala.collection.mutable
 
@@ -94,7 +95,7 @@ class StutteringClockTransform extends Transform with DependencyAPIMigration {
 
     // rename clocks to clock enable signals
     val mRef = CircuitTarget(state.circuit.main).module(main.name)
-    val renameMap = RenameMap()
+    val renameMap = MutableRenameMap()
     scan.clockToEnable.foreach {
       case (clk, en) =>
         renameMap.record(mRef.ref(clk), mRef.ref(en.name))

src/main/scala/firrtl/passes/Inline.scala

@@ -11,6 +11,7 @@ import firrtl.analyses.InstanceKeyGraph
 import firrtl.graph.{DiGraph, MutableDiGraph}
 import firrtl.stage.{Forms, RunFirrtlTransformAnnotation}
 import firrtl.options.{RegisteredTransform, ShellOption}
+import firrtl.renamemap.MutableRenameMap
 
 // Datastructures
 import scala.collection.mutable
@@ -184,7 +185,7 @@ class InlineInstances extends Transform with DependencyAPIMigration with Registe
       prefix:        String,
       ns:            Namespace,
       renames:       mutable.HashMap[String, String],
-      renameMap:     RenameMap
+      renameMap:     MutableRenameMap
     )(s:             Statement
     ): Statement = {
       def onName(ofModuleOpt: Option[String])(name: String): String = {
@@ -276,10 +277,10 @@ class InlineInstances extends Transform with DependencyAPIMigration with Registe
 
       indexMap match {
         case a if a.isEmpty =>
-          (Map.empty[(OfModule, Instance), RenameMap], Seq.empty[RenameMap])
+          (Map.empty[(OfModule, Instance), MutableRenameMap], Seq.empty[MutableRenameMap])
         case a =>
           val maxIdx = indexMap.values.max
-          val resultSeq = Seq.fill(maxIdx + 1)(RenameMap())
+          val resultSeq = Seq.fill(maxIdx + 1)(MutableRenameMap())
           val resultMap = indexMap.mapValues(idx => resultSeq(maxIdx - idx))
           (resultMap, resultSeq)
       }
@@ -367,7 +368,8 @@ class InlineInstances extends Transform with DependencyAPIMigration with Registe
         Some(m.map(onStmt(ModuleName(m.name, CircuitName(c.main)))))
     })
 
-    val renames = renamesSeq.reduceLeftOption(_ andThen _)
+    // Upcast so reduce works (andThen returns RenameMap)
+    val renames = (renamesSeq: Seq[RenameMap]).reduceLeftOption(_ andThen _)
 
     val cleanedAnnos = annos.filterNot {
       case InlineAnnotation(_) => true

src/main/scala/firrtl/passes/LowerTypes.scala

@@ -28,6 +28,7 @@ import firrtl.{
 import firrtl.ir._
 import firrtl.options.Dependency
 import firrtl.stage.TransformManager.TransformDependency
+import firrtl.renamemap.MutableRenameMap
 
 import scala.annotation.tailrec
 import scala.collection.mutable
@@ -80,7 +81,7 @@ object LowerTypes extends Transform with DependencyAPIMigration {
     // writers. Unfortunately, when you have lots of renames, this is very expensive
     // performance-wise. We use a private internal API that does not run .distinct to improve
     // performance, but we must be careful to not insert any duplicates.
-    val refRenameMap = RenameMap.noDistinct()
+    val refRenameMap = MutableRenameMap.noDistinct()
     val resultAndRenames =
       state.circuit.modules.map(m => onModule(c, m, memInitByModule.getOrElse(m.name, Seq()), refRenameMap))
     val result = state.circuit.copy(modules = resultAndRenames.map(_._1))
@@ -100,7 +101,7 @@ object LowerTypes extends Transform with DependencyAPIMigration {
     c:          CircuitTarget,
     m:          DefModule,
     memoryInit: Seq[MemoryInitAnnotation],
-    renameMap:  RenameMap
+    renameMap:  MutableRenameMap
   ): (DefModule, Map[Instance, Instance], Seq[MemoryInitAnnotation]) = {
     val ref = c.module(m.name)
 
@@ -123,7 +124,7 @@ object LowerTypes extends Transform with DependencyAPIMigration {
   private def lowerPorts(
     ref:       ModuleTarget,
     m:         DefModule,
-    renameMap: RenameMap
+    renameMap: MutableRenameMap
   ): (DefModule, Seq[(String, Seq[Reference])]) = {
     val namespace = mutable.HashSet[String]() ++ m.ports.map(_.name)
     val loweredPortsAndRefs = m.ports.flatMap { p =>
@@ -225,7 +226,7 @@ private class LoweringSymbolTable extends SymbolTable {
 // Lowers types and keeps track of references to lowered types.
 private class LoweringTable(
   table:           LoweringSymbolTable,
-  renameMap:       RenameMap,
+  renameMap:       MutableRenameMap,
   m:               ModuleTarget,
   portNameToExprs: Seq[(String, Seq[Reference])]) {
   private val portNames: Set[String] = portNameToExprs.map(_._2.head.name).toSet
@@ -284,7 +285,7 @@ private object DestructTypes {
     m:         ModuleTarget,
     ref:       Field,
     namespace: Namespace,
-    renameMap: RenameMap,
+    renameMap: MutableRenameMap,
     reserved:  Set[String]
   ): Seq[(Field, String)] = {
     // field renames (uniquify) are computed bottom up
@@ -349,7 +350,7 @@ private object DestructTypes {
     m:         ModuleTarget,
     mem:       DefMemory,
     namespace: Namespace,
-    renameMap: RenameMap,
+    renameMap: MutableRenameMap,
     reserved:  Set[String]
   ): (Seq[DefMemory], Seq[(String, SubField)]) = {
     // Uniquify the lowered memory names: When memories get split up into ground types, the access order is changes.
@@ -425,7 +426,7 @@ private object DestructTypes {
 
   private def recordRenames(
     fieldToRefs: Seq[(Field, Seq[ReferenceTarget])],
-    renameMap:   RenameMap,
+    renameMap:   MutableRenameMap,
     parent:      ParentRef
   ): Unit = {
     // TODO: if we group by ReferenceTarget, we could reduce the number of calls to `record`. Is it worth it?

src/main/scala/firrtl/passes/RemoveCHIRRTL.scala

@@ -9,6 +9,7 @@ import firrtl.ir._
 import firrtl.Utils._
 import firrtl.Mappers._
 import firrtl.options.Dependency
+import firrtl.renamemap.MutableRenameMap
 
 case class MPort(name: String, clk: Expression)
 case class MPorts(readers: ArrayBuffer[MPort], writers: ArrayBuffer[MPort], readwriters: ArrayBuffer[MPort])
@@ -78,6 +79,7 @@ object RemoveCHIRRTL extends Transform with DependencyAPIMigration {
     s.map(collect_smems_and_mports(mports, smems))
   }
 
+  @deprecated("This should never have been public", "FIRRTL 1.5")
   def collect_refs(
     mports:  MPortMap,
     smems:   SeqMemSet,
@@ -86,6 +88,18 @@ object RemoveCHIRRTL extends Transform with DependencyAPIMigration {
     raddrs:  AddrMap,
     renames: RenameMap
   )(s:       Statement
+  ): Statement =
+    // Cast is safe because RenameMap is sealed trait, MutableRenameMap is only subclass
+    collect_refs(mports, smems, types, refs, raddrs, renames.asInstanceOf[MutableRenameMap])(s)
+
+  private def collect_refs(
+    mports:  MPortMap,
+    smems:   SeqMemSet,
+    types:   MPortTypeMap,
+    refs:    DataRefMap,
+    raddrs:  AddrMap,
+    renames: MutableRenameMap
+  )(s:       Statement
   ): Statement = s match {
     case sx: CDefMemory =>
       types(sx.name) = sx.tpe
@@ -285,7 +299,12 @@ object RemoveCHIRRTL extends Transform with DependencyAPIMigration {
     }
   }
 
-  def remove_chirrtl_m(renames: RenameMap)(m: DefModule): DefModule = {
+  @deprecated("This should never have been public", "FIRRTL 1.5")
+  def remove_chirrtl_m(renames: RenameMap)(m: DefModule): DefModule =
+    // Cast is safe because RenameMap is sealed trait, MutableRenameMap is only subclass
+    remove_chirrtl_m(renames.asInstanceOf[MutableRenameMap])(m)
+
+  private def remove_chirrtl_m(renames: MutableRenameMap)(m: DefModule): DefModule = {
     val mports = new MPortMap
     val smems = new SeqMemSet
     val types = new MPortTypeMap
@@ -299,7 +318,7 @@ object RemoveCHIRRTL extends Transform with DependencyAPIMigration {
 
   def execute(state: CircuitState): CircuitState = {
     val c = state.circuit
-    val renames = RenameMap()
+    val renames = MutableRenameMap()
     renames.setCircuit(c.main)
     val result = c.copy(modules = c.modules.map(remove_chirrtl_m(renames)))
     state.copy(circuit = result, renames = Some(renames))

src/main/scala/firrtl/passes/ZeroWidth.scala

@@ -5,6 +5,7 @@ package firrtl.passes
 import firrtl.PrimOps._
 import firrtl.ir._
 import firrtl._
+import firrtl.renamemap.MutableRenameMap
 import firrtl.Mappers._
 import firrtl.options.Dependency
 
@@ -143,7 +144,7 @@ object ZeroWidth extends Transform with DependencyAPIMigration {
         case _                        => e.map(onExp)
       }
   }
-  private def onStmt(renames: RenameMap)(s: Statement): Statement = s match {
+  private def onStmt(renames: MutableRenameMap)(s: Statement): Statement = s match {
     case d @ DefWire(info, name, tpe) =>
       renames.delete(getRemoved(d))
       removeZero(tpe) match {
@@ -181,7 +182,7 @@ object ZeroWidth extends Transform with DependencyAPIMigration {
       }
     case sx => sx.map(onStmt(renames)).map(onExp)
   }
-  private def onModule(renames: RenameMap)(m: DefModule): DefModule = {
+  private def onModule(renames: MutableRenameMap)(m: DefModule): DefModule = {
     renames.setModule(m.name)
     // For each port, record deleted subcomponents
     m.ports.foreach { p => renames.delete(getRemoved(p)) }
@@ -200,7 +201,7 @@ object ZeroWidth extends Transform with DependencyAPIMigration {
     // run executeEmptyMemStmt first to remove zero-width memories
     // then run InferTypes to update widths for addr, en, clk, etc
     val c = InferTypes.run(executeEmptyMemStmt(state).circuit)
-    val renames = RenameMap()
+    val renames = MutableRenameMap()
     renames.setCircuit(c.main)
     val result = c.copy(modules = c.modules.map(onModule(renames)))
     CircuitState(result, outputForm, state.annotations, Some(renames))

src/main/scala/firrtl/passes/memlib/ReplaceMemMacros.scala

@@ -12,6 +12,7 @@ import firrtl.passes.MemPortUtils.{MemPortMap, Modules}
 import firrtl.passes.memlib.MemTransformUtils._
 import firrtl.passes.wiring._
 import firrtl.stage.Forms
+import firrtl.renamemap.MutableRenameMap
 
 import scala.collection.mutable.ListBuffer
 
@@ -244,7 +245,7 @@ class ReplaceMemMacros extends Transform with DependencyAPIMigration {
     memPortMap:              MemPortMap,
     memMods:                 Modules,
     annotatedMemoriesBuffer: ListBuffer[DefAnnotatedMemory],
-    renameMap:               RenameMap,
+    renameMap:               MutableRenameMap,
     circuit:                 String
   )(s:                       Statement
   ): Statement = s match {
@@ -282,7 +283,7 @@ class ReplaceMemMacros extends Transform with DependencyAPIMigration {
     nameMap:                 NameMap,
     memMods:                 Modules,
     annotatedMemoriesBuffer: ListBuffer[DefAnnotatedMemory],
-    renameMap:               RenameMap,
+    renameMap:               MutableRenameMap,
     circuit:                 String
   )(m:                       DefModule
   ) = {
@@ -299,7 +300,7 @@ class ReplaceMemMacros extends Transform with DependencyAPIMigration {
     val memMods = new Modules
     val nameMap = new NameMap
     c.modules.map(m => m.map(constructNameMap(namespace, nameMap, m.name)))
-    val renameMap = RenameMap()
+    val renameMap = MutableRenameMap()
     val modules = c.modules.map(updateMemMods(namespace, nameMap, memMods, annotatedMemoriesBuffer, renameMap, c.main))
     state.copy(
       circuit = c.copy(modules = modules ++ memMods),

src/main/scala/firrtl/transforms/DeadCodeElimination.scala

@@ -11,6 +11,7 @@ import firrtl.analyses.InstanceKeyGraph
 import firrtl.Mappers._
 import firrtl.Utils.{kind, throwInternalError}
 import firrtl.MemoizedHash._
+import firrtl.renamemap.MutableRenameMap
 import firrtl.backends.experimental.smt.random.DefRandom
 import firrtl.options.{Dependency, RegisteredTransform, ShellOption}
 
@@ -213,7 +214,7 @@ class DeadCodeElimination extends Transform with RegisteredTransform with Depend
     instMap:        collection.Map[String, String],
     deadNodes:      collection.Set[LogicNode],
     moduleMap:      collection.Map[String, DefModule],
-    renames:        RenameMap,
+    renames:        MutableRenameMap,
     topName:        String,
     doTouchExtMods: Set[String]
   )(mod:            DefModule
@@ -346,7 +347,7 @@ class DeadCodeElimination extends Transform with RegisteredTransform with Depend
 
     val liveNodes = depGraph.reachableFrom(circuitSink) + circuitSink
     val deadNodes = depGraph.getVertices -- liveNodes
-    val renames = RenameMap()
+    val renames = MutableRenameMap()
     renames.setCircuit(c.main)
 
     // As we delete deadCode, we will delete ports from Modules and somtimes complete modules