From d67c4100bd9e24cb9337ad932ba342fd5c49dee4 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Tue, 10 Mar 2020 21:18:36 -0400 Subject: Wrap elaboration in ChiselException Signed-off-by: Schuyler Eldridge squash! Wrap elaboration in ChiselException --- src/main/scala/chisel3/stage/ChiselAnnotations.scala | 2 +- src/test/scala/chiselTests/ChiselSpec.scala | 5 ++--- src/test/scala/chiselTests/OneHotMuxSpec.scala | 16 ++++++++++++---- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/main/scala/chisel3/stage/ChiselAnnotations.scala b/src/main/scala/chisel3/stage/ChiselAnnotations.scala index bfce0e8d..11697d7d 100644 --- a/src/main/scala/chisel3/stage/ChiselAnnotations.scala +++ b/src/main/scala/chisel3/stage/ChiselAnnotations.scala @@ -52,7 +52,7 @@ case class ChiselGeneratorAnnotation(gen: () => RawModule) extends NoTargetAnnot } catch { case e @ (_: OptionsException | _: ChiselException) => throw e case e: Throwable => - throw new OptionsException(s"Exception thrown when elaborating ChiselGeneratorAnnotation", e) + throw new ChiselException(s"Exception thrown when elaborating ChiselGeneratorAnnotation", e) } } diff --git a/src/test/scala/chiselTests/ChiselSpec.scala b/src/test/scala/chiselTests/ChiselSpec.scala index 9af7e88f..be1b2d60 100644 --- a/src/test/scala/chiselTests/ChiselSpec.scala +++ b/src/test/scala/chiselTests/ChiselSpec.scala @@ -7,7 +7,6 @@ import org.scalatest.prop._ import org.scalacheck._ import chisel3._ import chisel3.testers._ -import firrtl.options.OptionsException import firrtl.{AnnotationSeq, CommonOptions, ExecutionOptionsManager, FirrtlExecutionFailure, FirrtlExecutionSuccess, HasFirrtlOptions} import firrtl.util.BackendCompilationUtilities import java.io.ByteArrayOutputStream @@ -100,7 +99,7 @@ class ChiselTestUtilitiesSpec extends ChiselFlatSpec { import org.scalatest.exceptions.TestFailedException // Who tests the testers? "assertKnownWidth" should "error when the expected width is wrong" in { - val caught = intercept[OptionsException] { + val caught = intercept[ChiselException] { assertKnownWidth(7) { Wire(UInt(8.W)) } @@ -123,7 +122,7 @@ class ChiselTestUtilitiesSpec extends ChiselFlatSpec { } "assertInferredWidth" should "error if the width is known" in { - val caught = intercept[OptionsException] { + val caught = intercept[ChiselException] { assertInferredWidth(8) { Wire(UInt(8.W)) } diff --git a/src/test/scala/chiselTests/OneHotMuxSpec.scala b/src/test/scala/chiselTests/OneHotMuxSpec.scala index 7476ebe2..e6c9add4 100644 --- a/src/test/scala/chiselTests/OneHotMuxSpec.scala +++ b/src/test/scala/chiselTests/OneHotMuxSpec.scala @@ -4,6 +4,7 @@ package chiselTests import chisel3._ import chisel3.experimental.FixedPoint +import chisel3.internal.ChiselException import chisel3.testers.BasicTester import chisel3.util.{Mux1H, UIntToOH} import org.scalatest._ @@ -31,23 +32,31 @@ class OneHotMuxSpec extends FreeSpec with Matchers with ChiselRunners { assertTesterPasses(new ParameterizedAggregateOneHotTester) } "simple one hot mux with all aggregates containing inferred width fixed values should NOT work" in { - intercept[ChiselException] { + intercept [ChiselException] { assertTesterPasses(new InferredWidthAggregateOneHotTester) } } "simple one hot mux with all fixed width bundles but with different bundles should Not work" in { - intercept[IllegalArgumentException] { + try { assertTesterPasses(new DifferentBundleOneHotTester) + } catch { + case a: ChiselException => a.getCause match { + case _: IllegalArgumentException => + } } } "UIntToOH with output width greater than 2^(input width)" in { assertTesterPasses(new UIntToOHTester) } "UIntToOH should not accept width of zero (until zero-width wires are fixed" in { - intercept[java.lang.IllegalArgumentException] { + try { assertTesterPasses(new BasicTester { val out = UIntToOH(0.U, 0) }) + } catch { + case a: ChiselException => a.getCause match { + case _: IllegalArgumentException => + } } } @@ -305,4 +314,3 @@ class UIntToOHTester extends BasicTester { stop() } - -- cgit v1.2.3 From 73974725f09338e265733c93866fd15e76a2bdd0 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Wed, 11 Mar 2020 14:42:22 -0400 Subject: Report trimmed stack trace of Builder cause Changes the behavior of ChiselException stack trace trimming to use either the first exception that includes a method from the Builder or the outer exception. Signed-off-by: Schuyler Eldridge --- .../src/main/scala/chisel3/internal/Error.scala | 53 +++++++++++++++++++--- 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/chiselFrontend/src/main/scala/chisel3/internal/Error.scala b/chiselFrontend/src/main/scala/chisel3/internal/Error.scala index 91d9d7de..0108237a 100644 --- a/chiselFrontend/src/main/scala/chisel3/internal/Error.scala +++ b/chiselFrontend/src/main/scala/chisel3/internal/Error.scala @@ -2,6 +2,7 @@ package chisel3.internal +import scala.annotation.tailrec import scala.collection.mutable.{ArrayBuffer, LinkedHashMap} class ChiselException(message: String, cause: Throwable = null) extends Exception(message, cause) { @@ -9,27 +10,65 @@ class ChiselException(message: String, cause: Throwable = null) extends Exceptio val blacklistPackages = Set("chisel3", "scala", "java", "sun", "sbt") val builderName = "chisel3.internal.Builder" - /** trims the top of the stack of elements belonging to [[blacklistPackages]] - * then trims the bottom elements until it reaches [[builderName]] - * then continues trimming elements belonging to [[blacklistPackages]] + /** Examine a [[Throwable]], recursively searching it's causes, for the first [[Throwable]] that contains a stack + * trace including a specific class name. + * @param throwable the root exception + * @param className a class name string to search for + * @return [[Some]] exception if the class name was found, [[None]] otherwise */ - def trimmedStackTrace: Array[StackTraceElement] = { + @tailrec + private def findCause(throwable: Throwable, className: String): Option[Throwable] = + throwable.getStackTrace().collectFirst { + case ste if ste.getClassName().startsWith(className) => throwable + } match { + case a: Some[_] => a + case None => throwable.getCause() match { + case null => None + case cause: Throwable => findCause(cause, className) + } + } + + private lazy val likelyCause: Throwable = findCause(this, builderName) match { + case Some(a) => a + case None => this + } + + /** For an exception, return a stack trace trimmed to user code only + * + * This does the following actions: + * + * 1. Trims the top of the stack trace while elements match [[blacklistPackages]] + * 2. Trims the bottom of the stack trace until an element matches [[builderName]] + * 3. Trims from the [[builderName]] all [[blacklistPackages]] + * + * @param throwable the exception whose stack trace should be trimmed + * @return an array of stack trace elements + */ + private def trimmedStackTrace(throwable: Throwable): Array[StackTraceElement] = { def isBlacklisted(ste: StackTraceElement) = { val packageName = ste.getClassName().takeWhile(_ != '.') blacklistPackages.contains(packageName) } - val trimmedLeft = getStackTrace().view.dropWhile(isBlacklisted) + val trimmedLeft = throwable.getStackTrace().view.dropWhile(isBlacklisted) val trimmedReverse = trimmedLeft.reverse .dropWhile(ste => !ste.getClassName.startsWith(builderName)) .dropWhile(isBlacklisted) trimmedReverse.reverse.toArray } + /** trims the top of the stack of elements belonging to [[blacklistPackages]] + * then trims the bottom elements until it reaches [[builderName]] + * then continues trimming elements belonging to [[blacklistPackages]] + */ + @deprecated("This method will be removed in 3.4", "3.3") + def trimmedStackTrace: Array[StackTraceElement] = trimmedStackTrace(this) + def chiselStackTrace: String = { - val trimmed = trimmedStackTrace + val trimmed = trimmedStackTrace(likelyCause) + val sw = new java.io.StringWriter - sw.write(toString + "\n") + sw.write(likelyCause.toString + "\n") sw.write("\t...\n") trimmed.foreach(ste => sw.write(s"\tat $ste\n")) sw.write("\t... (Stack trace trimmed to user code only, rerun with --full-stacktrace if you wish to see the full stack trace)\n") // scalastyle:ignore line.size.limit -- cgit v1.2.3 From 18520bf16c38b4e8c5985c692baf9807c30459b7 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Wed, 11 Mar 2020 14:45:18 -0400 Subject: Test nested ChiselException in ChiselMain Adds two tests: 1. Test that an internal requirement failure (a bare exception) inside a Builder is properly reported/trimmed by ChsielStage/ChiselMain 2. Test that the full stack trace includes the ChiselException Signed-off-by: Schuyler Eldridge --- .../scala/chiselTests/stage/ChiselMainSpec.scala | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/test/scala/chiselTests/stage/ChiselMainSpec.scala b/src/test/scala/chiselTests/stage/ChiselMainSpec.scala index 2b3b9c2c..f5b47982 100644 --- a/src/test/scala/chiselTests/stage/ChiselMainSpec.scala +++ b/src/test/scala/chiselTests/stage/ChiselMainSpec.scala @@ -25,6 +25,11 @@ object ChiselMainSpec { out := in } + /** A module that fails a requirement */ + class FailingRequirementModule extends RawModule { + require(false) + } + } case class TestClassAspect() extends InspectorAspect[RawModule] ({ @@ -128,7 +133,22 @@ class ChiselMainSpec extends FeatureSpec with GivenWhenThen with Matchers with c result = 1) ).foreach(runStageExpectFiles) } - feature("Aspect library") { + feature("Report properly trimmed stack traces") { + Seq( + ChiselMainTest(args = Array("-X", "low"), + generator = Some(classOf[FailingRequirementModule]), + stdout = Some("requirement failed"), + result = 1), + ChiselMainTest(args = Array("-X", "low", "--full-stacktrace"), + generator = Some(classOf[FailingRequirementModule]), + stdout = Some("chisel3.internal.ChiselException"), + result = 1) + ).foreach(runStageExpectFiles) + } + + info("As an aspect writer") + info("I write an aspect") + feature("Running aspects via the command line") { Seq( ChiselMainTest(args = Array( "-X", "high", "--with-aspect", "chiselTests.stage.TestClassAspect" ), generator = Some(classOf[SameTypesModule]), -- cgit v1.2.3 From 30a0a5f95eafe77614787f9a403835c7992290ed Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Thu, 19 Mar 2020 12:30:21 -0400 Subject: Code style improvement Co-authored-by: Jack Koenig Signed-off-by: Schuyler Eldridge --- chiselFrontend/src/main/scala/chisel3/internal/Error.scala | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/chiselFrontend/src/main/scala/chisel3/internal/Error.scala b/chiselFrontend/src/main/scala/chisel3/internal/Error.scala index 0108237a..cc9cf0b1 100644 --- a/chiselFrontend/src/main/scala/chisel3/internal/Error.scala +++ b/chiselFrontend/src/main/scala/chisel3/internal/Error.scala @@ -28,10 +28,7 @@ class ChiselException(message: String, cause: Throwable = null) extends Exceptio } } - private lazy val likelyCause: Throwable = findCause(this, builderName) match { - case Some(a) => a - case None => this - } + private lazy val likelyCause: Throwable = findCause(this, builderName).getOrElse(this) /** For an exception, return a stack trace trimmed to user code only * -- cgit v1.2.3 From 969a56d6232449417b944a95fc942395e26c8b1a Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Thu, 19 Mar 2020 13:14:47 -0400 Subject: Safer generation of ChiselException.builderName Change ChiselException.builderName to compute the name of Chisel's internal Builder as opposed to hard-coding this with a string. Signed-off-by: Schuyler Eldridge --- chiselFrontend/src/main/scala/chisel3/internal/Error.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/chiselFrontend/src/main/scala/chisel3/internal/Error.scala b/chiselFrontend/src/main/scala/chisel3/internal/Error.scala index cc9cf0b1..2622a648 100644 --- a/chiselFrontend/src/main/scala/chisel3/internal/Error.scala +++ b/chiselFrontend/src/main/scala/chisel3/internal/Error.scala @@ -7,8 +7,8 @@ import scala.collection.mutable.{ArrayBuffer, LinkedHashMap} class ChiselException(message: String, cause: Throwable = null) extends Exception(message, cause) { - val blacklistPackages = Set("chisel3", "scala", "java", "sun", "sbt") - val builderName = "chisel3.internal.Builder" + val blacklistPackages: Set[String] = Set("chisel3", "scala", "java", "sun", "sbt") + val builderName: String = chisel3.internal.Builder.getClass.getName /** Examine a [[Throwable]], recursively searching it's causes, for the first [[Throwable]] that contains a stack * trace including a specific class name. -- cgit v1.2.3 From 3171cdf8472d50a4bcc6011c9b7786199a780514 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Thu, 19 Mar 2020 13:16:11 -0400 Subject: Add Scaladoc to ChiselException Signed-off-by: Schuyler Eldridge --- chiselFrontend/src/main/scala/chisel3/internal/Error.scala | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/chiselFrontend/src/main/scala/chisel3/internal/Error.scala b/chiselFrontend/src/main/scala/chisel3/internal/Error.scala index 2622a648..f50a6579 100644 --- a/chiselFrontend/src/main/scala/chisel3/internal/Error.scala +++ b/chiselFrontend/src/main/scala/chisel3/internal/Error.scala @@ -7,7 +7,10 @@ import scala.collection.mutable.{ArrayBuffer, LinkedHashMap} class ChiselException(message: String, cause: Throwable = null) extends Exception(message, cause) { + /** Package names whose stack trace elements should be trimmed when generating a trimmed stack trace */ val blacklistPackages: Set[String] = Set("chisel3", "scala", "java", "sun", "sbt") + + /** The object name of Chisel's internal `Builder`. Everything stack trace element after this will be trimmed. */ val builderName: String = chisel3.internal.Builder.getClass.getName /** Examine a [[Throwable]], recursively searching it's causes, for the first [[Throwable]] that contains a stack @@ -28,6 +31,10 @@ class ChiselException(message: String, cause: Throwable = null) extends Exceptio } } + /** Examine this [[ChiselException]] and it's causes for the first [[Throwable]] that contains a stack trace including + * a stack trace element whose declaring class is the [[builderName]]. If no such element exists, return this + * [[ChiselException]]. + */ private lazy val likelyCause: Throwable = findCause(this, builderName).getOrElse(this) /** For an exception, return a stack trace trimmed to user code only -- cgit v1.2.3