From 9a7945fd86fcad02da0556d8f4a30daa4b005f9d Mon Sep 17 00:00:00 2001
From: mergify[bot]
Date: Tue, 10 Jan 2023 07:19:45 +0000
Subject: Check for Vec subaccess in NamedComponent and throw a nicer error.
 (backport #2907) (#2928)

* Check for Vec subaccess in NamedComponent and throw a nicer error. (#2907)

This would previously end up throwing an exception later, when trying
to create a component name and realizing that it was invalid. Instead,
this detects Vec subaccesses early, and gives a more precise error and
suggestion.

(cherry picked from commit d8c30961c7b293ee19024a487698630367ee71c6)

# Conflicts:
#	core/src/main/scala/chisel3/internal/Builder.scala

* Resolve backport conflicts

Co-authored-by: Mike Urbach <mikeurbach@gmail.com>---
 core/src/main/scala/chisel3/internal/Builder.scala | 46 ++++++++++++++++------
 1 file changed, 35 insertions(+), 11 deletions(-)

(limited to 'core/src/main/scala/chisel3')

diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala
index fe7d7bea..be2c4cfb 100644
--- a/core/src/main/scala/chisel3/internal/Builder.scala
+++ b/core/src/main/scala/chisel3/internal/Builder.scala
@@ -299,25 +299,31 @@ private[chisel3] trait HasId extends InstanceId {
       // Builder.deprecated mechanism, we have to create our own one off ErrorLog and print the
       // warning right away.
       // It's especially bad because --warnings-as-errors does not work with these warnings
-      val nameGuess = _computeName(None) match {
-        case Some(name) => s": '$name'"
-        case None       => ""
-      }
-      val parentGuess = _parent match {
-        case Some(ViewParent) => s", in module '${reifyParent.pathName}'"
-        case Some(p)          => s", in module '${p.pathName}'"
-        case None             => ""
-      }
       val errors = new ErrorLog(false)
       val logger = new _root_.logger.Logger(this.getClass.getName)
       val msg =
-        "Accessing the .instanceName or .toTarget of non-hardware Data is deprecated" + nameGuess + parentGuess + ". " +
+        "Accessing the .instanceName or .toTarget of non-hardware Data is deprecated" + _errorContext + ". " +
           "This will become an error in Chisel 3.6."
       errors.deprecated(msg, None)
       errors.checkpoint(logger)
       _computeName(None).get
   }
 
+  private[chisel3] def _errorContext: String = {
+    val nameGuess: String = _computeName(None) match {
+      case Some(name) => s": '$name'"
+      case None       => ""
+    }
+
+    val parentGuess: String = _parent match {
+      case Some(ViewParent) => s", in module '${reifyParent.pathName}'"
+      case Some(p)          => s", in module '${p.pathName}'"
+      case None             => ""
+    }
+
+    nameGuess + parentGuess
+  }
+
   // Helper for reifying views if they map to a single Target
   private[chisel3] def reifyTarget: Option[Data] = this match {
     case d: Data => reifySingleData(d) // Only Data can be views
@@ -393,13 +399,16 @@ private[chisel3] trait NamedComponent extends HasId {
   /** Returns a FIRRTL ComponentName that references this object
     * @note Should not be called until circuit elaboration is complete
     */
-  final def toNamed: ComponentName =
+  final def toNamed: ComponentName = {
+    assertValidTarget()
     ComponentName(this.instanceName, ModuleName(this.parentModName, CircuitName(this.circuitName)))
+  }
 
   /** Returns a FIRRTL ReferenceTarget that references this object
     * @note Should not be called until circuit elaboration is complete
     */
   final def toTarget: ReferenceTarget = {
+    assertValidTarget()
     val name = this.instanceName
     if (!validComponentName(name)) throwException(s"Illegal component name: $name (note: literals are illegal)")
     import _root_.firrtl.annotations.{Target, TargetToken}
@@ -423,6 +432,21 @@ private[chisel3] trait NamedComponent extends HasId {
       case None             => localTarget
     }
   }
+
+  private def assertValidTarget(): Unit = {
+    val isVecSubaccess = getOptionRef.map {
+      case Index(_, _: ULit) => true // Vec literal indexing
+      case Index(_, _: Node) => true // Vec dynamic indexing
+      case _ => false
+    }.getOrElse(false)
+
+    if (isVecSubaccess) {
+      throwException(
+        s"You cannot target Vec subaccess" + _errorContext +
+          ". Instead, assign it to a temporary (for example, with WireInit) and target the temporary."
+      )
+    }
+  }
 }
 
 // Mutable global state for chisel that can appear outside a Builder context
-- 
cgit v1.2.3