Skip to content

Commit

Permalink
Add option to treat warnings as errors (backport #2676) (#2677)
Browse files Browse the repository at this point in the history
* Add option to treat warnings as errors (#2676)

Add --warnings-as-errors option

(cherry picked from commit 4989466)

# Conflicts:
#	core/src/main/scala/chisel3/experimental/hierarchy/Definition.scala
#	core/src/main/scala/chisel3/internal/Builder.scala
#	src/main/scala/chisel3/aop/injecting/InjectingAspect.scala
#	src/main/scala/chisel3/stage/ChiselOptions.scala
#	src/main/scala/chisel3/stage/package.scala
#	src/main/scala/chisel3/stage/phases/Elaborate.scala

* Resolve backport conflicts

Co-authored-by: Zachary Yedidia <zyedidia@gmail.com>
Co-authored-by: Jack Koenig <koenig@sifive.com>
  • Loading branch information
3 people authored Aug 13, 2022
1 parent d344e8a commit c4dec94
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ object Definition extends SourceInfoDoc {
): Definition[T] = {
val dynamicContext = {
val context = Builder.captureContext()
new DynamicContext(Nil, context.throwOnFirstError, context.warnReflectiveNaming)
new DynamicContext(Nil, context.throwOnFirstError, context.warnReflectiveNaming, context.warningsAsErrors)
}
Builder.globalNamespace.copyTo(dynamicContext.globalNamespace)
dynamicContext.inDefinition = true
Expand Down
8 changes: 5 additions & 3 deletions core/src/main/scala/chisel3/internal/Builder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,8 @@ private[chisel3] trait HasId extends InstanceId {
// 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
// It's especially bad because --warnings-as-errors does not work with these warnings
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. " +
"This will become an error in Chisel 3.6."
Expand Down Expand Up @@ -376,7 +377,8 @@ private[chisel3] class ChiselContext() {
private[chisel3] class DynamicContext(
val annotationSeq: AnnotationSeq,
val throwOnFirstError: Boolean,
val warnReflectiveNaming: Boolean) {
val warnReflectiveNaming: Boolean,
val warningsAsErrors: Boolean) {
val importDefinitionAnnos = annotationSeq.collect { case a: ImportDefinitionAnnotation[_] => a }

// Map holding the actual names of extModules
Expand Down Expand Up @@ -433,7 +435,7 @@ private[chisel3] class DynamicContext(
var whenStack: List[WhenContext] = Nil
var currentClock: Option[Clock] = None
var currentReset: Option[Reset] = None
val errors = new ErrorLog
val errors = new ErrorLog(warningsAsErrors)
val namingStack = new NamingStack

// Used to indicate if this is the top-level module of full elaboration, or from a Definition
Expand Down
9 changes: 6 additions & 3 deletions core/src/main/scala/chisel3/internal/Error.scala
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ private[chisel3] object ErrorLog {
val errTag = s"[${Console.RED}error${Console.RESET}]"
}

private[chisel3] class ErrorLog {
private[chisel3] class ErrorLog(warningsAsErrors: Boolean) {
def getLoc(loc: Option[StackTraceElement]): String = {
loc match {
case Some(elt: StackTraceElement) => s"${elt.getFileName}:${elt.getLineNumber}"
Expand All @@ -190,17 +190,20 @@ private[chisel3] class ErrorLog {
errors += (((m, getLoc(loc)), new Error(m, loc)))
}

private def warn(m: => String, loc: Option[StackTraceElement]): LogEntry =
if (warningsAsErrors) new Error(m, loc) else new Warning(m, loc)

/** Log a warning message */
def warning(m: => String): Unit = {
val loc = getUserLineNumber
errors += (((m, getLoc(loc)), new Warning(m, loc)))
errors += (((m, getLoc(loc)), warn(m, loc)))
}

/** Log a warning message without a source locator. This is used when the
* locator wouldn't be helpful (e.g., due to lazy values).
*/
def warningNoLoc(m: => String): Unit =
errors += (((m, ""), new Warning(m, None)))
errors += (((m, ""), warn(m, None)))

/** Emit an informational message */
@deprecated("This method will be removed in 3.5", "3.4")
Expand Down
7 changes: 6 additions & 1 deletion src/main/scala/chisel3/aop/injecting/InjectingAspect.scala
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,12 @@ abstract class InjectorAspect[T <: RawModule, M <: RawModule](
RunFirrtlTransformAnnotation(new InjectingTransform) +: modules.map { module =>
val chiselOptions = view[ChiselOptions](annotationsInAspect)
val dynamicContext =
new DynamicContext(annotationsInAspect, chiselOptions.throwOnFirstError, chiselOptions.warnReflectiveNaming)
new DynamicContext(
annotationsInAspect,
chiselOptions.throwOnFirstError,
chiselOptions.warnReflectiveNaming,
chiselOptions.warningsAsErrors
)
// Add existing module names into the namespace. If injection logic instantiates new modules
// which would share the same name, they will get uniquified accordingly
moduleNames.foreach { n =>
Expand Down
18 changes: 18 additions & 0 deletions src/main/scala/chisel3/stage/ChiselAnnotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,24 @@ case object ThrowOnFirstErrorAnnotation

}

/** When enabled, warnings will be treated as errors.
*/
case object WarningsAsErrorsAnnotation
extends NoTargetAnnotation
with ChiselOption
with HasShellOptions
with Unserializable {

val options = Seq(
new ShellOption[Unit](
longOption = "warnings-as-errors",
toAnnotationSeq = _ => Seq(WarningsAsErrorsAnnotation),
helpText = "Treat warnings as errors"
)
)

}

/** Warn when reflective naming changes names of signals */
case object WarnReflectiveNamingAnnotation
extends NoTargetAnnotation
Expand Down
1 change: 1 addition & 0 deletions src/main/scala/chisel3/stage/ChiselCli.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ trait ChiselCli { this: Shell =>
NoRunFirrtlCompilerAnnotation,
PrintFullStackTraceAnnotation,
ThrowOnFirstErrorAnnotation,
WarningsAsErrorsAnnotation,
WarnReflectiveNamingAnnotation,
ChiselOutputFileAnnotation,
ChiselGeneratorAnnotation
Expand Down
3 changes: 3 additions & 0 deletions src/main/scala/chisel3/stage/ChiselOptions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class ChiselOptions private[stage] (
val printFullStackTrace: Boolean = false,
val throwOnFirstError: Boolean = false,
val warnReflectiveNaming: Boolean = false,
val warningsAsErrors: Boolean = false,
val outputFile: Option[String] = None,
val chiselCircuit: Option[Circuit] = None) {

Expand All @@ -17,6 +18,7 @@ class ChiselOptions private[stage] (
printFullStackTrace: Boolean = printFullStackTrace,
throwOnFirstError: Boolean = throwOnFirstError,
warnReflectiveNaming: Boolean = warnReflectiveNaming,
warningsAsErrors: Boolean = warningsAsErrors,
outputFile: Option[String] = outputFile,
chiselCircuit: Option[Circuit] = chiselCircuit
): ChiselOptions = {
Expand All @@ -26,6 +28,7 @@ class ChiselOptions private[stage] (
printFullStackTrace = printFullStackTrace,
throwOnFirstError = throwOnFirstError,
warnReflectiveNaming = warnReflectiveNaming,
warningsAsErrors = warningsAsErrors,
outputFile = outputFile,
chiselCircuit = chiselCircuit
)
Expand Down
1 change: 1 addition & 0 deletions src/main/scala/chisel3/stage/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package object stage {
case PrintFullStackTraceAnnotation => c.copy(printFullStackTrace = true)
case ThrowOnFirstErrorAnnotation => c.copy(throwOnFirstError = true)
case WarnReflectiveNamingAnnotation => c.copy(warnReflectiveNaming = true)
case WarningsAsErrorsAnnotation => c.copy(warningsAsErrors = true)
case ChiselOutputFileAnnotation(f) => c.copy(outputFile = Some(f))
case ChiselCircuitAnnotation(a) => c.copy(chiselCircuit = Some(a))
}
Expand Down
7 changes: 6 additions & 1 deletion src/main/scala/chisel3/stage/phases/Elaborate.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,12 @@ class Elaborate extends Phase {
val chiselOptions = view[ChiselOptions](annotations)
try {
val context =
new DynamicContext(annotations, chiselOptions.throwOnFirstError, chiselOptions.warnReflectiveNaming)
new DynamicContext(
annotations,
chiselOptions.throwOnFirstError,
chiselOptions.warnReflectiveNaming,
chiselOptions.warningsAsErrors
)
val (circuit, dut) =
Builder.build(Module(gen()), context)
Seq(ChiselCircuitAnnotation(circuit), DesignAnnotation(dut))
Expand Down
37 changes: 22 additions & 15 deletions src/test/scala/chiselTests/WarningSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,32 +4,39 @@ package chiselTests

import chisel3._
import chisel3.util._
import chisel3.stage.{ChiselGeneratorAnnotation, ChiselStage}
import chisel3.stage.ChiselStage
import chisel3.experimental.ChiselEnum
import chisel3.experimental.EnumType
import chiselTests.ChiselFlatSpec

class WarningSpec extends ChiselFlatSpec with Utils {
behavior.of("Warnings")

"Warnings" should "be de-duplicated" in {
object MyEnum extends ChiselEnum {
val e0, e1, e2 = Value
}
object MyEnum extends ChiselEnum {
val e0, e1, e2 = Value
}

class MyModule extends Module {
val in = IO(Input(UInt(2.W)))
val out1 = IO(Output(MyEnum()))
val out2 = IO(Output(MyEnum()))
def func(out: EnumType): Unit = {
out := MyEnum(in)
}
func(out1)
func(out2)
class MyModule extends Module {
val in = IO(Input(UInt(2.W)))
val out1 = IO(Output(MyEnum()))
val out2 = IO(Output(MyEnum()))
def func(out: EnumType): Unit = {
out := MyEnum(in)
}
func(out1)
func(out2)
}

"Warnings" should "be de-duplicated" in {
val (log, _) = grabLog(ChiselStage.elaborate(new MyModule))
def countSubstring(s: String, sub: String) =
s.sliding(sub.length).count(_ == sub)
countSubstring(log, "Casting non-literal UInt") should be(1)
}

"Warnings" should "be treated as errors with warningsAsErrors" in {
a[ChiselException] should be thrownBy extractCause[ChiselException] {
val args = Array("--warnings-as-errors")
(new ChiselStage).emitChirrtl(new MyModule, args)
}
}
}

0 comments on commit c4dec94

Please sign in to comment.