Fix ChiselStage and Builder handling of logging (backport #3895)

core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala

@@ -103,7 +103,20 @@ object Definition extends SourceInfoDoc {
   ): Definition[T] = {
     val dynamicContext = {
       val context = Builder.captureContext()
+<<<<<<< HEAD
       new DynamicContext(Nil, context.throwOnFirstError, context.warningsAsErrors, context.sourceRoots)
+=======
+      new DynamicContext(
+        Nil,
+        context.throwOnFirstError,
+        context.legacyShiftRightWidth,
+        context.warningFilters,
+        context.sourceRoots,
+        Some(context.globalNamespace),
+        Builder.allDefinitions,
+        context.loggerOptions
+      )
+>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895))
     }
     Builder.globalNamespace.copyTo(dynamicContext.globalNamespace)
     dynamicContext.inDefinition = true

core/src/main/scala/chisel3/internal/Builder.scala

@@ -9,19 +9,24 @@ import chisel3.experimental._
 import chisel3.experimental.hierarchy.core.{Clone, ImportDefinitionAnnotation, Instance}
 import chisel3.internal.firrtl._
 import chisel3.internal.naming._
-import _root_.firrtl.annotations.{CircuitName, ComponentName, IsMember, ModuleName, Named, ReferenceTarget}
+import _root_.firrtl.annotations.{Annotation, CircuitName, ComponentName, IsMember, ModuleName, Named, ReferenceTarget}
 import _root_.firrtl.annotations.AnnotationUtils.validComponentName
 import _root_.firrtl.AnnotationSeq
 import _root_.firrtl.renamemap.MutableRenameMap
 import _root_.firrtl.util.BackendCompilationUtilities._
 import chisel3.experimental.dataview.{reify, reifySingleData}
 import chisel3.internal.Builder.Prefix
-import logger.LazyLogging
+import logger.{LazyLogging, LoggerOption}
 
 import scala.collection.mutable
 import scala.annotation.tailrec
 import java.io.File
 import scala.util.control.NonFatal
+<<<<<<< HEAD
+=======
+import chisel3.ChiselException
+import logger.LoggerOptions
+>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895))
 
 private[chisel3] class Namespace(keywords: Set[String]) {
   // This HashMap is compressed, not every name in the namespace is present here.
@@ -425,11 +430,24 @@ private[chisel3] class ChiselContext() {
 }
 
 private[chisel3] class DynamicContext(
+<<<<<<< HEAD
   val annotationSeq:     AnnotationSeq,
   val throwOnFirstError: Boolean,
   val warningsAsErrors:  Boolean,
   val sourceRoots:       Seq[File]) {
   val importDefinitionAnnos = annotationSeq.collect { case a: ImportDefinitionAnnotation[_] => a }
+=======
+  val annotationSeq:         AnnotationSeq,
+  val throwOnFirstError:     Boolean,
+  val legacyShiftRightWidth: Boolean,
+  val warningFilters:        Seq[WarningFilter],
+  val sourceRoots:           Seq[File],
+  val defaultNamespace:      Option[Namespace],
+  // Definitions from other scopes in the same elaboration, use allDefinitions below
+  val outerScopeDefinitions: List[Iterable[Definition[_]]],
+  val loggerOptions:         LoggerOptions) {
+  val importedDefinitionAnnos = annotationSeq.collect { case a: ImportDefinitionAnnotation[_] => a }
+>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895))
 
   // Map holding the actual names of extModules
   // Pick the definition name by default in case not passed through annotation.
@@ -836,6 +854,16 @@ private[chisel3] object Builder extends LazyLogging {
     f:              => T,
     dynamicContext: DynamicContext,
     forceModName:   Boolean = true
+  ): (Circuit, T) = {
+    // Logger has its own context separate from Chisel's dynamic context
+    _root_.logger.Logger.makeScope(dynamicContext.loggerOptions) {
+      buildImpl(f, dynamicContext)
+    }
+  }
+
+  private def buildImpl[T <: BaseModule](
+    f:              => T,
+    dynamicContext: DynamicContext
   ): (Circuit, T) = {
     dynamicContextVar.withValue(Some(dynamicContext)) {
       ViewParent: Unit // Must initialize the singleton in a Builder context or weird things can happen

docs/src/cookbooks/cookbook.md

@@ -784,8 +784,86 @@ If the index is too narrow you can use `.pad` to increase the width.
 ```scala mdoc:silent
 import chisel3.util.log2Up
 
+<<<<<<< HEAD
 class TooNarrow(extracteeWidth: Int, indexWidth: Int) {
   val extractee = Wire(UInt(extracteeWidth.W))
+=======
+Chisel will warn if a dynamic index is not the correctly-sized width for indexing a Vec or UInt.
+"Correctly-sized" means that the width of the index should be the log2 of the size of the indexee.
+If the indexee is a non-power-of-2 size, use the ceiling of the log2 result.
+
+```scala mdoc:invisible:reset
+import chisel3._
+// Helper to throw away return value so it doesn't show up in mdoc
+def compile(gen: => chisel3.RawModule): Unit = {
+  circt.stage.ChiselStage.emitCHIRRTL(gen)
+}
+```
+
+When the index does not have enough bits to address all entries or bits in the extractee, you can `.pad` the index to increase the width.
+
+```scala mdoc
+class TooNarrow extends RawModule {
+  val extractee = Wire(UInt(7.W))
+  val index = Wire(UInt(2.W))
+  extractee(index)
+}
+compile(new TooNarrow)
+```
+
+This can be fixed with `pad`:
+
+```scala mdoc
+class TooNarrowFixed extends RawModule {
+  val extractee = Wire(UInt(7.W))
+  val index = Wire(UInt(2.W))
+  extractee(index.pad(3))
+}
+compile(new TooNarrowFixed)
+```
+
+#### Use bit extraction when the index is too wide
+
+```scala mdoc
+class TooWide extends RawModule {
+  val extractee = Wire(Vec(8, UInt(32.W)))
+  val index = Wire(UInt(4.W))
+  extractee(index)
+}
+compile(new TooWide)
+```
+
+This can be fixed with bit extraction:
+
+```scala mdoc
+class TooWideFixed extends RawModule {
+  val extractee = Wire(Vec(8, UInt(32.W)))
+  val index = Wire(UInt(4.W))
+  extractee(index(2, 0))
+}
+compile(new TooWideFixed)
+```
+
+Note that size 1 `Vecs` and `UInts` should be indexed by a zero-width `UInt`:
+
+```scala mdoc
+class SizeOneVec extends RawModule {
+  val extractee = Wire(Vec(1, UInt(32.W)))
+  val index = Wire(UInt(0.W))
+  extractee(index)
+}
+compile(new SizeOneVec)
+```
+
+Because `pad` only pads if the desired width is less than the current width of the argument,
+you can use `pad` in conjunction with bit extraction when the widths may be too wide or too
+narrow under different circumstances
+
+```scala mdoc
+import chisel3.util.log2Ceil
+class TooWideOrNarrow(extracteeSize: Int, indexWidth: Int) extends Module {
+  val extractee = Wire(Vec(extracteeSize, UInt(8.W)))
+>>>>>>> 88d147d90 (Fix ChiselStage and Builder handling of logging (#3895))
   val index = Wire(UInt(indexWidth.W))
   extractee(index.pad(log2Up(extracteeWidth)))
 }

docs/src/explanations/warnings.md

@@ -0,0 +1,145 @@
+---
+layout: docs
+title:  "Warnings"
+section: "chisel3"
+---
+
+# Warnings
+
+Warnings in Chisel are used for deprecating old APIs or semantics for later removal.
+As a matter of good software practice, Chisel users are encouraged to treat warnings as errors with `--warnings-as-errors`;
+however, the coarse-grained nature of this option can be problematic when bumping Chisel which may introduce many warnings.
+See [Warning Configuration](#warning-configuration) below for techniques to help deal with large numbers of warnings.
+
+## Warning Configuration
+
+Inspired by `-Wconf` [in Scala](https://www.scala-lang.org/2021/01/12/configuring-and-suppressing-warnings.html),
+Chisel supports fine-grain control of warning behavior via the CLI options `--warn-conf` and `--warn-conf-file`.
+
+### Basic Operation
+
+`--warn-conf` accepts a comma-separated sequence of `<filter>:<action>` pairs.
+When a warning is hit in Chisel, the sequence of pairs are checked from left-to-right to see if the `filter` matches the warning.
+The `action` associated with the first matching `filter` is the one used for the specific warning.
+If no `filters` match, then the default behavior is to issue the warning.
+
+`--warn-conf` can be specified any number of times.
+Earlier uses of `--warn-conf` take priority over later ones in the same left-to-right decreasing priority as the `filters` are checked within a single `--warn-conf`.
+As a mental model, the user can pretend that all `--warn-conf` arguments concatenated together (separated by `,`) into a single argument.
+
+### Warning Configuration Files
+
+`--warn-conf-file` accepts a file which contains the same format of `<filter>:<action>` pairs, separated by newlines.
+Lines starting with `#` will be treated as comments and ignored.
+`filters` are checked in decreasing priority from top-to-bottom of the file.
+
+A single command-line can contain any number of `--warn-conf-file` and any number of `--warn-conf` arguments.
+The filters from all `--warn-conf*` arguments will be applied in the same left-to-right decreasing priority order.
+
+### Filters
+
+The supported filters are:
+
+* `any` - matches all warnings
+* `id=<integer>` - matches warnings with the integer id
+* `src=<glob>` - matches warnings when `<glob>` matches the source locator filename where the warning occurs
+
+`id` and `src` filters can be combined with `&`.
+Any filter can have at most one `id` and at most one `src` listed.
+`any` cannot be combined with any other filters.
+
+### Actions
+
+The supported actions are:
+
+* `:s` - suppress matching warnings
+* `:w` - report matching warnings as warnings (default behavior)
+* `:e` - error on matching warnings
+
+### Examples
+
+The following example issues a warning when elaborated normally
+
+```scala mdoc:invisible:reset
+// Helper to throw away return value so it doesn't show up in mdoc
+def compile(gen: => chisel3.RawModule, args: Array[String] = Array()): Unit = {
+  circt.stage.ChiselStage.emitCHIRRTL(gen, args = args)
+}
+```
+
+```scala mdoc
+import circt.stage.ChiselStage.emitSystemVerilog
+import chisel3._
+class TooWideIndexModule extends RawModule {
+  val in = IO(Input(Vec(4, UInt(8.W))))
+  val idx = IO(Input(UInt(8.W))) // This index is wider than necessary
+  val out = IO(Output(UInt(8.W)))
+
+  out := in(idx)
+}
+compile(new TooWideIndexModule)
+```
+
+As shown in the warning, this warning is `W004` (which can be fixed [as described below](#w004-dynamic-index-too-wide)), we can suppress it with an `id` filter which will suppress all instances of this warning in the elaboration run.
+
+```scala mdoc
+compile(new TooWideIndexModule, args = Array("--warn-conf", "id=4:s"))
+```
+
+It is generally advisable to make warning suppressions as precise as possible, so we could combine this `id` filter with a `src` glob filter for just this file:
+
+```scala mdoc
+compile(new TooWideIndexModule, args = Array("--warn-conf", "id=4&src=**warnings.md:s"))
+```
+
+Finally, users are encouraged to treat warnings as errors to the extend possible,
+so they should always end any warning configuration with `any:e` to elevate all unmatched warnings to errors:
+
+```scala mdoc
+compile(new TooWideIndexModule, args = Array("--warn-conf", "id=4&src=**warnings.md:s,any:e"))
+// Or
+compile(new TooWideIndexModule, args = Array("--warn-conf", "id=4&src=**warnings.md:s", "--warn-conf", "any:e"))
+// Or
+compile(new TooWideIndexModule, args = Array("--warn-conf", "id=4&src=**warnings.md:s", "--warnings-as-errors"))
+```
+
+## Warning Glossary
+
+Chisel warnings have a unique identifier number to make them easier to lookup as well as so they can be configured as described above.
+
+### [W001] Unsafe UInt cast to ChiselEnum
+
+This warning occurs when casting a `UInt` to a `ChiselEnum` when there are values the `UInt` could take that are not legal states in the enumeration.
+See the [ChiselEnum explanation](chisel-enum#casting) for more information and how to fix this warning.
+
+**Note:** This is the only warning that is not currently scheduled for become an error.
+
+### [W002] Dynamic bit select too wide
+
+This warning occurs when dynamically indexing a `UInt` or an `SInt` with an index that is wider than necessary to address all bits in the indexee.
+It indicates that some of the high-bits of the index are ignored by the indexing operation.
+It can be fixed as described in the [Cookbook](../cookbooks/cookbook#dynamic-index-too-wide-narrow).
+
+### [W003] Dynamic bit select too narrow
+
+This warning occurs when dynamically indexing a `UInt` or an `SInt` with an index that is to small to address all bits in the indexee.
+It indicates that some bits of the indexee cannot be reached by the indexing operation.
+It can be fixed as described in the [Cookbook](../cookbooks/cookbook#dynamic-index-too-wide-narrow).
+
+### [W004] Dynamic index too wide
+
+This warning occurs when dynamically indexing a `Vec` with an index that is wider than necessary to address all elements of the `Vec`.
+It indicates that some of the high-bits of the index are ignored by the indexing operation.
+It can be fixed as described in the [Cookbook](../cookbooks/cookbook#dynamic-index-too-wide-narrow).
+
+### [W005] Dynamic index too narrow
+
+This warning occurs when dynamically indexing a `Vec` with an index that is to small to address all elements in the `Vec`.
+It indicates that some elements of the `Vec` cannot be reached by the indexing operation.
+It can be fixed as described in the [Cookbook](../cookbooks/cookbook#dynamic-index-too-wide-narrow).
+
+
+### [W006] Extract from Vec of size 0
+
+This warning occurs when indexing a `Vec` with no elements.
+It can be fixed by removing the indexing operation for the size zero `Vec` (perhaps via guarding with an `if-else` or `Option.when`).