Refactor exceptions to remove stack trace from user errors (#1157)

20 files changed, 81 insertions, 65 deletions

diff --git a/src/main/scala/firrtl/Compiler.scala b/src/main/scala/firrtl/Compiler.scala
index 367defb5..600b991e 100644
--- a/src/main/scala/firrtl/Compiler.scala
+++ b/src/main/scala/firrtl/Compiler.scala
@@ -43,7 +43,7 @@ case class CircuitState(
   def getEmittedCircuit: EmittedCircuit = emittedCircuitOption match {
     case Some(emittedCircuit) => emittedCircuit
     case None =>
-      throw new FIRRTLException(s"No EmittedCircuit found! Did you delete any annotations?\n$deletedAnnotations")
+      throw new FirrtlInternalException(s"No EmittedCircuit found! Did you delete any annotations?\n$deletedAnnotations")
   }
 
   /** Helper function for extracting emitted components from annotations */
@@ -315,9 +315,6 @@ trait Emitter extends Transform {
   def outputSuffix: String
 }
 
-/** Wraps exceptions from CustomTransforms so they can be reported appropriately */
-case class CustomTransformException(cause: Throwable) extends Exception("", cause)
-
 object CompilerUtils extends LazyLogging {
   /** Generates a sequence of [[Transform]]s to lower a Firrtl circuit
     *
diff --git a/src/main/scala/firrtl/FirrtlException.scala b/src/main/scala/firrtl/FirrtlException.scala
new file mode 100644
index 00000000..20d984a1
--- /dev/null
+++ b/src/main/scala/firrtl/FirrtlException.scala
@@ -0,0 +1,43 @@
+// See LICENSE for license details.
+
+package firrtl
+
+import scala.util.control.NoStackTrace
+
+@deprecated("External users should use either FirrtlUserException or their own hierarchy", "1.2")
+object FIRRTLException {
+  def defaultMessage(message: String, cause: Throwable) = {
+    if (message != null) {
+      message
+    } else if (cause != null) {
+      cause.toString
+    } else {
+      null
+    }
+  }
+}
+@deprecated("External users should use either FirrtlUserException or their own hierarchy", "1.2")
+class FIRRTLException(val str: String, cause: Throwable = null)
+  extends RuntimeException(FIRRTLException.defaultMessage(str, cause), cause)
+
+/** Exception indicating user error
+  *
+  * These exceptions indicate a problem due to bad input and thus do not include a stack trace.
+  * This can be extended by custom transform writers.
+  */
+class FirrtlUserException(message: String, cause: Throwable = null)
+  extends RuntimeException(message, cause) with NoStackTrace
+
+/** Wraps exceptions from CustomTransforms so they can be reported appropriately */
+case class CustomTransformException(cause: Throwable) extends Exception("", cause)
+
+/** Exception indicating something went wrong *within* Firrtl itself
+  *
+  * These exceptions indicate a problem inside the compiler and include a stack trace to help
+  * developers debug the issue.
+  *
+  * This class is private because these are issues within Firrtl itself. Exceptions thrown in custom
+  * transforms are treated differently and should thus have their own structure
+  */
+private[firrtl] class FirrtlInternalException(message: String, cause: Throwable = null)
+  extends Exception(message, cause)
diff --git a/src/main/scala/firrtl/Parser.scala b/src/main/scala/firrtl/Parser.scala
index 2e3ddde0..4e26d642 100644
--- a/src/main/scala/firrtl/Parser.scala
+++ b/src/main/scala/firrtl/Parser.scala
@@ -9,7 +9,7 @@ import firrtl.ir._
 import firrtl.Utils.time
 import firrtl.antlr.{FIRRTLParser, _}
 
-class ParserException(message: String) extends FIRRTLException(message)
+class ParserException(message: String) extends FirrtlUserException(message)
 
 case class ParameterNotSpecifiedException(message: String) extends ParserException(message)
 case class ParameterRedefinedException(message: String) extends ParserException(message)
diff --git a/src/main/scala/firrtl/Utils.scala b/src/main/scala/firrtl/Utils.scala
index 206afc09..91da600a 100644
--- a/src/main/scala/firrtl/Utils.scala
+++ b/src/main/scala/firrtl/Utils.scala
@@ -13,20 +13,6 @@ import scala.util.matching.Regex
 import firrtl.annotations.{ReferenceTarget, TargetToken}
 import _root_.logger.LazyLogging
 
-object FIRRTLException {
-  def defaultMessage(message: String, cause: Throwable) = {
-    if (message != null) {
-      message
-    } else if (cause != null) {
-      cause.toString
-    } else {
-      null
-    }
-  }
-}
-class FIRRTLException(val str: String, cause: Throwable = null)
-  extends RuntimeException(FIRRTLException.defaultMessage(str, cause), cause)
-
 object seqCat {
   def apply(args: Seq[Expression]): Expression = args.length match {
     case 0 => Utils.error("Empty Seq passed to seqcat")
@@ -434,7 +420,7 @@ object Utils extends LazyLogging {
   }
    
 // =================================
-  def error(str: String, cause: Throwable = null) = throw new FIRRTLException(str, cause)
+  def error(str: String, cause: Throwable = null) = throw new FirrtlInternalException(str, cause)
 
 //// =============== EXPANSION FUNCTIONS ================
   def get_size(t: Type): Int = t match {
@@ -632,7 +618,7 @@ object Utils extends LazyLogging {
     case EmptyExpression => root
   }
 
-  case class DeclarationNotFoundException(msg: String) extends FIRRTLException(msg)
+  case class DeclarationNotFoundException(msg: String) extends FirrtlUserException(msg)
 
   /** Gets the root declaration of an expression
     *
diff --git a/src/main/scala/firrtl/annotations/AnnotationUtils.scala b/src/main/scala/firrtl/annotations/AnnotationUtils.scala
index 241c7a08..9a7a8fd2 100644
--- a/src/main/scala/firrtl/annotations/AnnotationUtils.scala
+++ b/src/main/scala/firrtl/annotations/AnnotationUtils.scala
@@ -11,13 +11,13 @@ import firrtl.annotations.AnnotationYamlProtocol._
 
 import firrtl.ir._
 
-case class InvalidAnnotationFileException(file: File, cause: Throwable = null)
-  extends FIRRTLException(s"$file, see cause below", cause)
-case class InvalidAnnotationJSONException(msg: String) extends FIRRTLException(msg)
-case class AnnotationFileNotFoundException(file: File) extends FIRRTLException(
+case class InvalidAnnotationFileException(file: File, cause: FirrtlUserException = null)
+  extends FirrtlUserException(s"$file", cause)
+case class InvalidAnnotationJSONException(msg: String) extends FirrtlUserException(msg)
+case class AnnotationFileNotFoundException(file: File) extends FirrtlUserException(
   s"Annotation file $file not found!"
 )
-case class AnnotationClassNotFoundException(className: String) extends FIRRTLException(
+case class AnnotationClassNotFoundException(className: String) extends FirrtlUserException(
   s"Annotation class $className not found! Please check spelling and classpath"
 )
 
diff --git a/src/main/scala/firrtl/annotations/AnnotationYamlProtocol.scala b/src/main/scala/firrtl/annotations/AnnotationYamlProtocol.scala
index e0337d6e..4bb643b3 100644
--- a/src/main/scala/firrtl/annotations/AnnotationYamlProtocol.scala
+++ b/src/main/scala/firrtl/annotations/AnnotationYamlProtocol.scala
@@ -31,9 +31,6 @@ object AnnotationYamlProtocol extends DefaultYamlProtocol {
         case annotationException: AnnotationException =>
           Utils.error(
             s"Error: ${annotationException.getMessage} while parsing annotation from yaml\n${yamlValue.prettyPrint}")
-        case annotationException: FIRRTLException =>
-          Utils.error(
-            s"Error: ${annotationException.getMessage} while parsing annotation from yaml\n${yamlValue.prettyPrint}")
       }
     }
     def toTarget(string: String): Named = string.split("""\.""", -1).toSeq match {
diff --git a/src/main/scala/firrtl/annotations/JsonProtocol.scala b/src/main/scala/firrtl/annotations/JsonProtocol.scala
index 5270c26f..b09155d8 100644
--- a/src/main/scala/firrtl/annotations/JsonProtocol.scala
+++ b/src/main/scala/firrtl/annotations/JsonProtocol.scala
@@ -37,7 +37,7 @@ object JsonProtocol {
       try {
         Class.forName(s).asInstanceOf[Class[_ <: Transform]].newInstance()
       } catch {
-        case e: java.lang.InstantiationException => throw new FIRRTLException(
+        case e: java.lang.InstantiationException => throw new FirrtlInternalException(
           "NoSuchMethodException during construction of serialized Transform. Is your Transform an inner class?", e)
         case t: Throwable => throw t
       }},
@@ -113,10 +113,11 @@ object JsonProtocol {
     // Translate some generic errors to specific ones
     case e: java.lang.ClassNotFoundException =>
       Failure(new AnnotationClassNotFoundException(e.getMessage))
-    case e: org.json4s.ParserUtil.ParseException =>
+    // Eat the stack traces of json4s exceptions
+    case e @ (_: org.json4s.ParserUtil.ParseException | _: org.json4s.MappingException) =>
       Failure(new InvalidAnnotationJSONException(e.getMessage))
   }.recoverWith { // If the input is a file, wrap in InvalidAnnotationFileException
-    case e => in match {
+    case e: FirrtlUserException => in match {
       case FileInput(file) =>
         Failure(new InvalidAnnotationFileException(file, e))
       case _ => Failure(e)
diff --git a/src/main/scala/firrtl/annotations/LoadMemoryAnnotation.scala b/src/main/scala/firrtl/annotations/LoadMemoryAnnotation.scala
index c52bf5f6..64c30bdb 100644
--- a/src/main/scala/firrtl/annotations/LoadMemoryAnnotation.scala
+++ b/src/main/scala/firrtl/annotations/LoadMemoryAnnotation.scala
@@ -4,7 +4,7 @@ package firrtl.annotations
 
 import java.io.File
 
-import firrtl.FIRRTLException
+import firrtl.FirrtlUserException
 
 /** Representation of the two types of `readmem` statements available in Verilog.
   */
@@ -21,7 +21,7 @@ object MemoryLoadFileType {
   def deserialize(s: String): MemoryLoadFileType = s match {
     case "h" => MemoryLoadFileType.Hex
     case "b" => MemoryLoadFileType.Binary
-    case _ => throw new FIRRTLException(s"Unrecognized MemoryLoadFileType: $s")
+    case _ => throw new FirrtlUserException(s"Unrecognized MemoryLoadFileType: $s")
   }
 }
 
diff --git a/src/main/scala/firrtl/annotations/transforms/EliminateTargetPaths.scala b/src/main/scala/firrtl/annotations/transforms/EliminateTargetPaths.scala
index 8f604c9f..f6bab6ea 100644
--- a/src/main/scala/firrtl/annotations/transforms/EliminateTargetPaths.scala
+++ b/src/main/scala/firrtl/annotations/transforms/EliminateTargetPaths.scala
@@ -8,7 +8,7 @@ import firrtl.annotations.TargetToken.{Instance, OfModule}
 import firrtl.annotations.analysis.DuplicationHelper
 import firrtl.annotations._
 import firrtl.ir._
-import firrtl.{CircuitForm, CircuitState, FIRRTLException, HighForm, RenameMap, Transform, WDefInstance}
+import firrtl.{CircuitForm, CircuitState, FirrtlInternalException, HighForm, RenameMap, Transform, WDefInstance}
 
 import scala.collection.mutable
 
@@ -23,7 +23,7 @@ case class ResolvePaths(targets: Seq[CompleteTarget]) extends Annotation {
   }
 }
 
-case class NoSuchTargetException(message: String) extends FIRRTLException(message)
+case class NoSuchTargetException(message: String) extends FirrtlInternalException(message)
 
 /** For a set of non-local targets, modify the instance/module hierarchy of the circuit such that
   * the paths in each non-local target can be removed
diff --git a/src/main/scala/firrtl/passes/LowerTypes.scala b/src/main/scala/firrtl/passes/LowerTypes.scala
index ee03afb5..8035c1ce 100644
--- a/src/main/scala/firrtl/passes/LowerTypes.scala
+++ b/src/main/scala/firrtl/passes/LowerTypes.scala
@@ -81,7 +81,7 @@ object LowerTypes extends Transform {
       names ++ subNames
     }
   }
-  private case class LowerTypesException(msg: String) extends FIRRTLException(msg)
+  private case class LowerTypesException(msg: String) extends FirrtlInternalException(msg)
   private def error(msg: String)(info: Info, mname: String) =
     throw LowerTypesException(s"$info: [module $mname] $msg")
 
diff --git a/src/main/scala/firrtl/passes/Passes.scala b/src/main/scala/firrtl/passes/Passes.scala
index 67daaf10..1428ea5f 100644
--- a/src/main/scala/firrtl/passes/Passes.scala
+++ b/src/main/scala/firrtl/passes/Passes.scala
@@ -31,8 +31,8 @@ trait Pass extends Transform {
 }
 
 // Error handling
-class PassException(message: String) extends Exception(message)
-class PassExceptions(val exceptions: Seq[PassException]) extends Exception("\n" + exceptions.mkString("\n"))
+class PassException(message: String) extends FirrtlUserException(message)
+class PassExceptions(val exceptions: Seq[PassException]) extends FirrtlUserException("\n" + exceptions.mkString("\n"))
 class Errors {
   val errors = collection.mutable.ArrayBuffer[PassException]()
   def append(pe: PassException) = errors.append(pe)
diff --git a/src/main/scala/firrtl/passes/Uniquify.scala b/src/main/scala/firrtl/passes/Uniquify.scala
index a36f5ce8..52a2f95e 100644
--- a/src/main/scala/firrtl/passes/Uniquify.scala
+++ b/src/main/scala/firrtl/passes/Uniquify.scala
@@ -34,7 +34,7 @@ import MemPortUtils.memType
 object Uniquify extends Transform {
   def inputForm = UnknownForm
   def outputForm = UnknownForm
-  private case class UniquifyException(msg: String) extends FIRRTLException(msg)
+  private case class UniquifyException(msg: String) extends FirrtlInternalException(msg)
   private def error(msg: String)(implicit sinfo: Info, mname: String) =
     throw new UniquifyException(s"$sinfo: [moduleOpt $mname] $msg")
 
diff --git a/src/main/scala/firrtl/stage/FirrtlStage.scala b/src/main/scala/firrtl/stage/FirrtlStage.scala
index 79a22961..92b461a1 100644
--- a/src/main/scala/firrtl/stage/FirrtlStage.scala
+++ b/src/main/scala/firrtl/stage/FirrtlStage.scala
@@ -2,8 +2,9 @@
 
 package firrtl.stage
 
-import firrtl.{AnnotationSeq, CustomTransformException, FIRRTLException, Utils}
-import firrtl.options.{Stage, Phase, PhaseException, Shell, OptionsException, StageMain}
+import firrtl.{AnnotationSeq, CustomTransformException, FirrtlInternalException,
+               FirrtlUserException, FIRRTLException, Utils}
+import firrtl.options.{Stage, Phase, PhaseException, Shell, OptionsException, StageMain, StageUtils}
 import firrtl.options.phases.DeletedWrapper
 import firrtl.passes.{PassException, PassExceptions}
 
@@ -28,8 +29,8 @@ class FirrtlStage extends Stage {
   } catch {
     /* Rethrow the exceptions which are expected or due to the runtime environment (out of memory, stack overflow, etc.).
      * Any UNEXPECTED exceptions should be treated as internal errors. */
-    case p @ (_: ControlThrowable | _: PassException | _: PassExceptions | _: FIRRTLException | _: OptionsException
-                | _: PhaseException) => throw p
+    case p @ (_: ControlThrowable | _: FIRRTLException | _: OptionsException | _: FirrtlUserException
+                | _: FirrtlInternalException | _: PhaseException) => throw p
     case CustomTransformException(cause) => throw cause
     case e: Exception => Utils.throwInternalError(exception = Some(e))
   }
diff --git a/src/main/scala/firrtl/transforms/BlackBoxSourceHelper.scala b/src/main/scala/firrtl/transforms/BlackBoxSourceHelper.scala
index 49d484ee..8192b416 100644
--- a/src/main/scala/firrtl/transforms/BlackBoxSourceHelper.scala
+++ b/src/main/scala/firrtl/transforms/BlackBoxSourceHelper.scala
@@ -43,8 +43,8 @@ case class BlackBoxResourceFileNameAnno(resourceFileName: String) extends BlackB
   * @param fileName the name of the BlackBox file (only used for error message generation)
   * @param e an underlying exception that generated this
   */
-class BlackBoxNotFoundException(fileName: String, e: Throwable = null) extends FIRRTLException(
-  s"BlackBox '$fileName' not found. Did you misspell it? Is it in src/{main,test}/resources?", e)
+class BlackBoxNotFoundException(fileName: String, message: String) extends FirrtlUserException(
+  s"BlackBox '$fileName' not found. Did you misspell it? Is it in src/{main,test}/resources?\n$message")
 
 /** Handle source for Verilog ExtModules (BlackBoxes)
   *
@@ -136,8 +136,8 @@ object BlackBoxSourceHelper {
     * @param code some code to run
     */
   private def safeFile[A](fileName: String)(code: => A) = try { code } catch {
-    case e@ (_: FileNotFoundException | _: NullPointerException) => throw new BlackBoxNotFoundException(fileName, e)
-    case t: Throwable                                            => throw t
+    case e @ (_: FileNotFoundException | _: NullPointerException) =>
+      throw new BlackBoxNotFoundException(fileName, e.getMessage)
   }
 
   /**
diff --git a/src/test/scala/firrtlTests/AnnotationTests.scala b/src/test/scala/firrtlTests/AnnotationTests.scala
index 7b944470..03e3198e 100644
--- a/src/test/scala/firrtlTests/AnnotationTests.scala
+++ b/src/test/scala/firrtlTests/AnnotationTests.scala
@@ -2,7 +2,7 @@
 
 package firrtlTests
 
-import java.io.{File, FileWriter, Writer}
+import java.io.{File, FileWriter}
 
 import firrtl.annotations.AnnotationYamlProtocol._
 import firrtl.annotations._
@@ -12,10 +12,8 @@ import firrtl.transforms.OptimizableExtModuleAnnotation
 import firrtl.passes.InlineAnnotation
 import firrtl.passes.memlib.PinAnnotation
 import firrtl.util.BackendCompilationUtilities
-import firrtl.transforms.DontTouchAnnotation
 import net.jcazevedo.moultingyaml._
 import org.scalatest.Matchers
-import logger._
 
 /**
  * An example methodology for testing Firrtl annotations.
@@ -72,11 +70,11 @@ abstract class AnnotationTests extends AnnotationSpec with Matchers {
     result.annotations.head should matchPattern {
       case DeletedAnnotation(`tname`, `inlineAnn`) =>
     }
-    val exception = (intercept[FIRRTLException] {
+    val exception = (intercept[Exception] {
       result.getEmittedCircuit
     })
     val deleted = result.deletedAnnotations
-    exception.str should be (s"No EmittedCircuit found! Did you delete any annotations?\n$deleted")
+    exception.getMessage should be (s"No EmittedCircuit found! Did you delete any annotations?\n$deleted")
   }
 
   "Renaming" should "propagate in Lowering of memories" in {
diff --git a/src/test/scala/firrtlTests/ParserSpec.scala b/src/test/scala/firrtlTests/ParserSpec.scala
index 384b75d2..711df5ed 100644
--- a/src/test/scala/firrtlTests/ParserSpec.scala
+++ b/src/test/scala/firrtlTests/ParserSpec.scala
@@ -2,10 +2,8 @@
 
 package firrtlTests
 
-import org.scalatest._
 import firrtl._
 import org.scalacheck.Gen
-import org.scalacheck.Prop.forAll
 
 class ParserSpec extends FirrtlFlatSpec {
 
diff --git a/src/test/scala/firrtlTests/RenameMapSpec.scala b/src/test/scala/firrtlTests/RenameMapSpec.scala
index 3e569dcd..3241db16 100644
--- a/src/test/scala/firrtlTests/RenameMapSpec.scala
+++ b/src/test/scala/firrtlTests/RenameMapSpec.scala
@@ -3,11 +3,8 @@
 package firrtlTests
 
 import firrtl.RenameMap
-import firrtl.FIRRTLException
 import firrtl.RenameMap.IllegalRenameException
 import firrtl.annotations._
-import firrtl.annotations.Target
-import firrtl.annotations.TargetToken.{Instance, OfModule}
 
 class RenameMapSpec extends FirrtlFlatSpec {
   val cir   = CircuitTarget("Top")
diff --git a/src/test/scala/firrtlTests/options/OptionParserSpec.scala b/src/test/scala/firrtlTests/options/OptionParserSpec.scala
index 0a9ac2d5..3059ba1a 100644
--- a/src/test/scala/firrtlTests/options/OptionParserSpec.scala
+++ b/src/test/scala/firrtlTests/options/OptionParserSpec.scala
@@ -2,7 +2,7 @@
 
 package firrtlTests.options
 
-import firrtl.{AnnotationSeq, FIRRTLException}
+import firrtl.AnnotationSeq
 import firrtl.annotations.{Annotation, NoTargetAnnotation}
 import firrtl.options.{DoNotTerminateOnExit, DuplicateHandling, ExceptOnError, OptionsException}
 
diff --git a/src/test/scala/firrtlTests/options/phases/GetIncludesSpec.scala b/src/test/scala/firrtlTests/options/phases/GetIncludesSpec.scala
index 1007ca8c..3bdf65a8 100644
--- a/src/test/scala/firrtlTests/options/phases/GetIncludesSpec.scala
+++ b/src/test/scala/firrtlTests/options/phases/GetIncludesSpec.scala
@@ -7,7 +7,7 @@ import org.scalatest.{FlatSpec, Matchers}
 import java.io.{File, PrintWriter}
 
 import firrtl.AnnotationSeq
-import firrtl.annotations.{Annotation, AnnotationFileNotFoundException, JsonProtocol,
+import firrtl.annotations.{AnnotationFileNotFoundException, JsonProtocol,
   NoTargetAnnotation}
 import firrtl.options.phases.GetIncludes
 import firrtl.options.{InputAnnotationFileAnnotation, Phase}
diff --git a/src/test/scala/firrtlTests/transforms/BlackBoxSourceHelperSpec.scala b/src/test/scala/firrtlTests/transforms/BlackBoxSourceHelperSpec.scala
index 6e63317b..213ead77 100644
--- a/src/test/scala/firrtlTests/transforms/BlackBoxSourceHelperSpec.scala
+++ b/src/test/scala/firrtlTests/transforms/BlackBoxSourceHelperSpec.scala
@@ -2,13 +2,11 @@
 
 package firrtlTests.transforms
 
-import firrtl.annotations.{Annotation, CircuitName, ModuleName}
+import firrtl.annotations.{CircuitName, ModuleName}
 import firrtl.transforms._
-import firrtl.{FIRRTLException, Transform, VerilogCompiler, VerilogEmitter}
-import firrtlTests.{HighTransformSpec, LowTransformSpec}
+import firrtl.{Transform, VerilogEmitter}
+import firrtlTests.LowTransformSpec
 import firrtl.FileUtils
-import org.scalacheck.Test.Failed
-import org.scalatest.{FreeSpec, Matchers, Succeeded}
 
 
 class BlacklBoxSourceHelperTransformSpec extends LowTransformSpec {