Merge branch '3.5.x' into 3.5-release

5 files changed, 177 insertions, 71 deletions

diff --git a/core/src/main/scala/chisel3/internal/BiConnect.scala b/core/src/main/scala/chisel3/internal/BiConnect.scala
index 2d6c9e4a..e8fb2361 100644
--- a/core/src/main/scala/chisel3/internal/BiConnect.scala
+++ b/core/src/main/scala/chisel3/internal/BiConnect.scala
@@ -55,6 +55,24 @@ private[chisel3] object BiConnect {
     * There is some cleverness in the use of internal try-catch to catch exceptions
     * during the recursive decent and then rethrow them with extra information added.
     * This gives the user a 'path' to where in the connections things went wrong.
+    *
+    * == Chisel Semantics and how they emit to firrtl ==
+    *
+    * 1. Strict Bi-Connect (all fields as seen by firrtl must match exactly)
+    *   `a <= b`
+    *
+    * 2. Strict Bi-Connect (implemented as being field-blasted because we know all firrtl fields would not match exactly)
+    *   `a.foo <= b.foo, b.bar <= a.bar`
+    *
+    * 3. Not-Strict Bi-Connect (firrtl will allow fields to not match exactly)
+    *   `a <- b`
+    *
+    * 4. Mixed Semantic Bi-Connect (some fields need to be handled differently)
+    *   `a.foo <= b.foo` (case 2),  `b.bar <- a.bar` (case 3)
+    *
+    * - The decision on 1 vs 2 is based on structural type -- if same type once emitted to firrtl, emit 1, otherwise emit 2
+    * - 1/2 vs 3 is based on CompileOptions at connection point e.g. at `<>` , emit 3 if `emitStrictConnects = false` for either side
+    * - 4 is a special case of 2 turning into 3 for some subfields, when either side's subfield at `extends Bundle/Record` has `emitStrictConnects = false`
     */
   def connect(
     sourceInfo:            SourceInfo,
@@ -140,8 +158,8 @@ private[chisel3] object BiConnect {
       // Handle Records defined in Chisel._ code by emitting a FIRRTL bulk
       // connect when possible and a partial connect otherwise
       case pair @ (left_r: Record, right_r: Record) =>
-        val notStrict =
-          Seq(left_r.compileOptions, right_r.compileOptions).contains(ExplicitCompileOptions.NotStrict)
+        val emitStrictConnects: Boolean =
+          left_r.compileOptions.emitStrictConnects && right_r.compileOptions.emitStrictConnects
 
         // chisel3 <> is commutative but FIRRTL <- is not
         val flipConnection =
@@ -161,40 +179,38 @@ private[chisel3] object BiConnect {
           )
         ) {
           pushCommand(Connect(sourceInfo, leftReified.get.lref, rightReified.get.lref))
-        } else if (notStrict) {
-          newLeft.bulkConnect(newRight)(sourceInfo, ExplicitCompileOptions.NotStrict)
+        } else if (!emitStrictConnects) {
+          newLeft.legacyConnect(newRight)(sourceInfo)
         } else {
           recordConnect(sourceInfo, connectCompileOptions, left_r, right_r, context_mod)
         }
 
-      // Handle Records connected to DontCare (change to NotStrict)
+      // Handle Records connected to DontCare
       case (left_r: Record, DontCare) =>
-        left_r.compileOptions match {
-          case ExplicitCompileOptions.NotStrict =>
-            left.bulkConnect(right)(sourceInfo, ExplicitCompileOptions.NotStrict)
-          case _ =>
-            // For each field in left, descend with right
-            for ((field, left_sub) <- left_r.elements) {
-              try {
-                connect(sourceInfo, connectCompileOptions, left_sub, right, context_mod)
-              } catch {
-                case BiConnectException(message) => throw BiConnectException(s".$field$message")
-              }
+        if (!left_r.compileOptions.emitStrictConnects) {
+          left.legacyConnect(right)(sourceInfo)
+        } else {
+          // For each field in left, descend with right
+          for ((field, left_sub) <- left_r.elements) {
+            try {
+              connect(sourceInfo, connectCompileOptions, left_sub, right, context_mod)
+            } catch {
+              case BiConnectException(message) => throw BiConnectException(s".$field$message")
             }
+          }
         }
       case (DontCare, right_r: Record) =>
-        right_r.compileOptions match {
-          case ExplicitCompileOptions.NotStrict =>
-            left.bulkConnect(right)(sourceInfo, ExplicitCompileOptions.NotStrict)
-          case _ =>
-            // For each field in left, descend with right
-            for ((field, right_sub) <- right_r.elements) {
-              try {
-                connect(sourceInfo, connectCompileOptions, left, right_sub, context_mod)
-              } catch {
-                case BiConnectException(message) => throw BiConnectException(s".$field$message")
-              }
+        if (!right_r.compileOptions.emitStrictConnects) {
+          left.legacyConnect(right)(sourceInfo)
+        } else {
+          // For each field in left, descend with right
+          for ((field, right_sub) <- right_r.elements) {
+            try {
+              connect(sourceInfo, connectCompileOptions, left, right_sub, context_mod)
+            } catch {
+              case BiConnectException(message) => throw BiConnectException(s".$field$message")
             }
+          }
         }
 
       // Left and right are different subtypes of Data so fail
diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala
index 4180f580..61f94f8f 100644
--- a/core/src/main/scala/chisel3/internal/Builder.scala
+++ b/core/src/main/scala/chisel3/internal/Builder.scala
@@ -6,7 +6,7 @@ import scala.util.DynamicVariable
 import scala.collection.mutable.ArrayBuffer
 import chisel3._
 import chisel3.experimental._
-import chisel3.experimental.hierarchy.{Clone, Instance}
+import chisel3.experimental.hierarchy.{Clone, ImportDefinitionAnnotation, Instance}
 import chisel3.internal.firrtl._
 import chisel3.internal.naming._
 import _root_.firrtl.annotations.{CircuitName, ComponentName, IsMember, ModuleName, Named, ReferenceTarget}
@@ -137,6 +137,17 @@ private[chisel3] trait HasId extends InstanceId {
     this
   }
 
+  // Private internal version of suggestName that tells you if the name changed
+  // Returns Some(old name, old prefix) if name changed, None otherwise
+  private[chisel3] def _suggestNameCheck(seed: => String): Option[(String, Prefix)] = {
+    val oldSeed = this.seedOpt
+    val oldPrefix = this.naming_prefix
+    suggestName(seed)
+    if (oldSeed.nonEmpty && (oldSeed != this.seedOpt || oldPrefix != this.naming_prefix)) {
+      Some(oldSeed.get -> oldPrefix)
+    } else None
+  }
+
   /** Takes the first seed suggested. Multiple calls to this function will be ignored.
     * If the final computed name conflicts with another name, it may get uniquified by appending
     * a digit at the end.
@@ -166,37 +177,13 @@ private[chisel3] trait HasId extends InstanceId {
   }
 
   /** Computes the name of this HasId, if one exists
-    * @param defaultPrefix Optionally provide a default prefix for computing the name
     * @param defaultSeed Optionally provide default seed for computing the name
     * @return the name, if it can be computed
     */
-  private[chisel3] def _computeName(defaultPrefix: Option[String], defaultSeed: Option[String]): Option[String] = {
-
-    /** Computes a name of this signal, given the seed and prefix
-      * @param seed
-      * @param prefix
-      * @return
-      */
-    def buildName(seed: String, prefix: Prefix): String = {
-      val builder = new StringBuilder()
-      prefix.foreach { p =>
-        builder ++= p
-        builder += '_'
-      }
-      builder ++= seed
-      builder.toString
-    }
-
-    if (hasSeed) {
-      Some(buildName(seedOpt.get, naming_prefix.reverse))
-    } else {
-      defaultSeed.map { default =>
-        defaultPrefix match {
-          case Some(p) => buildName(default, p :: naming_prefix.reverse)
-          case None    => buildName(default, naming_prefix.reverse)
-        }
-      }
-    }
+  private[chisel3] def _computeName(defaultSeed: Option[String]): Option[String] = {
+    seedOpt
+      .orElse(defaultSeed)
+      .map(name => buildName(name, naming_prefix.reverse))
   }
 
   /** This resolves the precedence of [[autoSeed]] and [[suggestName]]
@@ -216,9 +203,9 @@ private[chisel3] trait HasId extends InstanceId {
   // Uses a namespace to convert suggestion into a true name
   // Will not do any naming if the reference already assigned.
   // (e.g. tried to suggest a name to part of a Record)
-  private[chisel3] def forceName(prefix: Option[String], default: => String, namespace: Namespace): Unit =
+  private[chisel3] def forceName(default: => String, namespace: Namespace): Unit =
     if (_ref.isEmpty) {
-      val candidate_name = _computeName(prefix, Some(default)).get
+      val candidate_name = _computeName(Some(default)).get
       val available_name = namespace.name(candidate_name)
       setRef(Ref(available_name))
       // Clear naming prefix to free memory
@@ -240,7 +227,18 @@ private[chisel3] trait HasId extends InstanceId {
 
   private def refName(c: Component): String = _ref match {
     case Some(arg) => arg.fullName(c)
-    case None      => _computeName(None, None).get
+    case None      =>
+      // This is super hacky but this is just for a short term deprecation
+      // These accesses occur after Chisel elaboration so we cannot use the normal
+      // Builder.deprecated mechanism, we have to create our own one off ErrorLog and print the
+      // warning right away.
+      val errors = new ErrorLog
+      val logger = new _root_.logger.Logger(this.getClass.getName)
+      val msg = "Accessing the .instanceName or .toTarget of non-hardware Data is deprecated. " +
+        "This will become an error in Chisel 3.6."
+      errors.deprecated(msg, None)
+      errors.checkpoint(logger)
+      _computeName(None).get
   }
 
   // Helper for reifying views if they map to a single Target
@@ -363,10 +361,50 @@ private[chisel3] class ChiselContext() {
   val viewNamespace = Namespace.empty
 }
 
-private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq, val throwOnFirstError: Boolean) {
+private[chisel3] class DynamicContext(
+  val annotationSeq:        AnnotationSeq,
+  val throwOnFirstError:    Boolean,
+  val warnReflectiveNaming: Boolean) {
+  val importDefinitionAnnos = annotationSeq.collect { case a: ImportDefinitionAnnotation[_] => a }
+
+  // Map holding the actual names of extModules
+  // Pick the definition name by default in case not passed through annotation.
+  val importDefinitionMap = importDefinitionAnnos
+    .map(a => a.definition.proto.name -> a.overrideDefName.getOrElse(a.definition.proto.name))
+    .toMap
+
+  // Helper function which does 2 things
+  // 1. Ensure there are no repeated names for imported Definitions - both Proto Names as well as ExtMod Names
+  // 2. Return the distinct definition / extMod names
+  private def checkAndGeDistinctProtoExtModNames() = {
+    val importAllDefinitionProtoNames = importDefinitionAnnos.map { a => a.definition.proto.name }
+    val importDistinctDefinitionProtoNames = importDefinitionMap.keys.toSeq
+    val importAllDefinitionExtModNames = importDefinitionMap.toSeq.map(_._2)
+    val importDistinctDefinitionExtModNames = importAllDefinitionExtModNames.distinct
+
+    if (importDistinctDefinitionProtoNames.length < importAllDefinitionProtoNames.length) {
+      val duplicates = importAllDefinitionProtoNames.diff(importDistinctDefinitionProtoNames).mkString(", ")
+      throwException(s"Expected distinct imported Definition names but found duplicates for: $duplicates")
+    }
+    if (importDistinctDefinitionExtModNames.length < importAllDefinitionExtModNames.length) {
+      val duplicates = importAllDefinitionExtModNames.diff(importDistinctDefinitionExtModNames).mkString(", ")
+      throwException(s"Expected distinct overrideDef names but found duplicates for: $duplicates")
+    }
+    (importAllDefinitionProtoNames ++ importAllDefinitionExtModNames).distinct
+  }
+
   val globalNamespace = Namespace.empty
+
+  // Ensure imported Definitions emit as ExtModules with the correct name so
+  // that instantiations will also use the correct name and prevent any name
+  // conflicts with Modules/Definitions in this elaboration
+  checkAndGeDistinctProtoExtModNames().foreach {
+    globalNamespace.name(_)
+  }
+
   val components = ArrayBuffer[Component]()
   val annotations = ArrayBuffer[ChiselAnnotation]()
+  val newAnnotations = ArrayBuffer[ChiselMultiAnnotation]()
   var currentModule: Option[BaseModule] = None
 
   /** Contains a mapping from a elaborated module to their aspect
@@ -432,8 +470,13 @@ private[chisel3] object Builder extends LazyLogging {
   def globalNamespace: Namespace = dynamicContext.globalNamespace
   def components:      ArrayBuffer[Component] = dynamicContext.components
   def annotations:     ArrayBuffer[ChiselAnnotation] = dynamicContext.annotations
-  def annotationSeq:   AnnotationSeq = dynamicContext.annotationSeq
-  def namingStack:     NamingStack = dynamicContext.namingStack
+
+  // 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 importDefinitionMap: Map[String, String] = dynamicContext.importDefinitionMap
 
   def unnamedViews:  ArrayBuffer[Data] = dynamicContext.unnamedViews
   def viewNamespace: Namespace = chiselContext.get.viewNamespace
@@ -480,7 +523,11 @@ private[chisel3] object Builder extends LazyLogging {
       }
     }
     buildAggName(d).map { name =>
-      pushPrefix(name)
+      if (isTemp(name)) {
+        pushPrefix(name.tail)
+      } else {
+        pushPrefix(name)
+      }
     }.isDefined
   }
 
@@ -621,6 +668,8 @@ private[chisel3] object Builder extends LazyLogging {
     throwException("Error: No implicit reset.")
   )
 
+  def warnReflectiveNaming: Boolean = dynamicContext.warnReflectiveNaming
+
   // TODO(twigg): Ideally, binding checks and new bindings would all occur here
   // However, rest of frontend can't support this yet.
   def pushCommand[T <: Command](c: T): T = {
@@ -662,8 +711,9 @@ private[chisel3] object Builder extends LazyLogging {
       throwException(m)
     }
   }
-  def warning(m:    => String): Unit = if (dynamicContextVar.value.isDefined) errors.warning(m)
-  def deprecated(m: => String, location: Option[String] = None): Unit =
+  def warning(m:      => String): Unit = if (dynamicContextVar.value.isDefined) errors.warning(m)
+  def warningNoLoc(m: => String): Unit = if (dynamicContextVar.value.isDefined) errors.warningNoLoc(m)
+  def deprecated(m:   => String, location: Option[String] = None): Unit =
     if (dynamicContextVar.value.isDefined) errors.deprecated(m, location)
 
   /** Record an exception as an error, and throw it.
@@ -720,12 +770,12 @@ private[chisel3] object Builder extends LazyLogging {
       logger.info("Elaborating design...")
       val mod = f
       if (forceModName) { // This avoids definition name index skipping with D/I
-        mod.forceName(None, mod.name, globalNamespace)
+        mod.forceName(mod.name, globalNamespace)
       }
       errors.checkpoint(logger)
       logger.info("Done elaborating.")
 
-      (Circuit(components.last.name, components.toSeq, annotations.toSeq, makeViewRenameMap), mod)
+      (Circuit(components.last.name, components.toSeq, annotations.toSeq, makeViewRenameMap, newAnnotations.toSeq), mod)
     }
   }
   initializeSingletons()
diff --git a/core/src/main/scala/chisel3/internal/Error.scala b/core/src/main/scala/chisel3/internal/Error.scala
index 62086870..3b0846eb 100644
--- a/core/src/main/scala/chisel3/internal/Error.scala
+++ b/core/src/main/scala/chisel3/internal/Error.scala
@@ -186,6 +186,11 @@ private[chisel3] class ErrorLog {
   def warning(m: => String): Unit =
     errors += new Warning(m, getUserLineNumber)
 
+  /** Log a warning message without a source locator */
+  def warningNoLoc(m: => String): Unit = {
+    errors += new Warning(m, None)
+  }
+
   /** Emit an informational message */
   @deprecated("This method will be removed in 3.5", "3.4")
   def info(m: String): Unit =
diff --git a/core/src/main/scala/chisel3/internal/MonoConnect.scala b/core/src/main/scala/chisel3/internal/MonoConnect.scala
index 40056c89..31364804 100644
--- a/core/src/main/scala/chisel3/internal/MonoConnect.scala
+++ b/core/src/main/scala/chisel3/internal/MonoConnect.scala
@@ -210,7 +210,9 @@ private[chisel3] object MonoConnect {
         }
 
       // Source is DontCare - it may be connected to anything. It generates a defInvalid for the sink.
-      case (sink, DontCare) => pushCommand(DefInvalid(sourceInfo, sink.lref))
+      case (_sink: Element, DontCare) =>
+        val sink = reify(_sink) // Handle views
+        pushCommand(DefInvalid(sourceInfo, sink.lref))
       // DontCare as a sink is illegal.
       case (DontCare, _) => throw DontCareCantBeSink
       // Analog is illegal in mono connections.
diff --git a/core/src/main/scala/chisel3/internal/firrtl/IR.scala b/core/src/main/scala/chisel3/internal/firrtl/IR.scala
index 1ee8842f..dc9ab027 100644
--- a/core/src/main/scala/chisel3/internal/firrtl/IR.scala
+++ b/core/src/main/scala/chisel3/internal/firrtl/IR.scala
@@ -94,7 +94,7 @@ object Arg {
     case Some(arg)                           => arg.name
     case None =>
       id match {
-        case data: Data => data._computeName(None, Some("?")).get
+        case data: Data => data._computeName(Some("?")).get
         case _ => "?"
       }
   }
@@ -861,7 +861,40 @@ case class DefBlackBox(
   params: Map[String, Param])
     extends Component
 
-case class Circuit(name: String, components: Seq[Component], annotations: Seq[ChiselAnnotation], renames: RenameMap) {
-  def firrtlAnnotations: Iterable[Annotation] = annotations.flatMap(_.toFirrtl.update(renames))
+case class Circuit(
+  name:       String,
+  components: Seq[Component],
+  @deprecated("Do not use annotations val of Circuit directly - use firrtlAnnotations instead. Will be removed in a future release",
+    "Chisel 3.5")
+  annotations: Seq[ChiselAnnotation],
+  renames:     RenameMap,
+  @deprecated("Do not use newAnnotations val of Circuit directly - use firrtlAnnotations instead. Will be removed in a future release",
+    "Chisel 3.5")
 
+  newAnnotations: Seq[ChiselMultiAnnotation]) {
+
+  def this(name: String, components: Seq[Component], annotations: Seq[ChiselAnnotation], renames: RenameMap) =
+    this(name, components, annotations, renames, Seq.empty)
+
+  def firrtlAnnotations: Iterable[Annotation] =
+    annotations.flatMap(_.toFirrtl.update(renames)) ++ newAnnotations.flatMap(
+      _.toFirrtl.flatMap(_.update(renames))
+    )
+
+  def copy(
+    name:        String = name,
+    components:  Seq[Component] = components,
+    annotations: Seq[ChiselAnnotation] = annotations,
+    renames:     RenameMap = renames
+  ) = Circuit(name, components, annotations, renames, newAnnotations)
+
+}
+object Circuit
+    extends scala.runtime.AbstractFunction4[String, Seq[Component], Seq[ChiselAnnotation], RenameMap, Circuit] {
+  def unapply(c: Circuit): Option[(String, Seq[Component], Seq[ChiselAnnotation], RenameMap)] = {
+    Some((c.name, c.components, c.annotations, c.renames))
+  }
+
+  def apply(name: String, components: Seq[Component], annotations: Seq[ChiselAnnotation], renames: RenameMap): Circuit =
+    new Circuit(name, components, annotations, renames)
 }