Add checks on register clock and reset types (#33) (#553)

Remove infix notation on calls with side effects.

3 files changed, 126 insertions, 50 deletions

diff --git a/src/main/scala/firrtl/passes/CheckWidths.scala b/src/main/scala/firrtl/passes/CheckWidths.scala
index 24735009..8f6ab18f 100644
--- a/src/main/scala/firrtl/passes/CheckWidths.scala
+++ b/src/main/scala/firrtl/passes/CheckWidths.scala
@@ -92,6 +92,12 @@ object CheckWidths extends Pass {
               errors.append(new AttachWidthsNotEqual(infox, mname, e.serialize, exprs.head.serialize))
           )
           s
+        case sx: DefRegister =>
+          sx.reset.tpe match {
+            case UIntType(IntWidth(w)) if w == 1 =>
+            case _ => errors.append(new CheckTypes.IllegalResetType(info, mname, sx.name))
+          }
+          s
         case _ => s
       }
     }
diff --git a/src/main/scala/firrtl/passes/Checks.scala b/src/main/scala/firrtl/passes/Checks.scala
index 0bebcd18..3389224e 100644
--- a/src/main/scala/firrtl/passes/Checks.scala
+++ b/src/main/scala/firrtl/passes/Checks.scala
@@ -61,11 +61,11 @@ object CheckHighForm extends Pass {
       def correctNum(ne: Option[Int], nc: Int) {
         ne match {
           case Some(i) if e.args.length != i =>
-            errors append new IncorrectNumArgsException(info, mname, e.op.toString, i)
+            errors.append(new IncorrectNumArgsException(info, mname, e.op.toString, i))
           case _ => // Do Nothing
         }
         if (e.consts.length != nc)
-          errors append new IncorrectNumConstsException(info, mname, e.op.toString, nc)
+          errors.append(new IncorrectNumConstsException(info, mname, e.op.toString, nc))
       }
 
       e.op match {
@@ -89,25 +89,25 @@ object CheckHighForm extends Pass {
         case ((percentx, n), b) if percentx && (validFormats contains b) =>
           (false, n + 1)
         case ((percentx, n), b) if percentx && b != '%' =>
-          errors append new BadPrintfException(info, mname, b.toChar)
+          errors.append(new BadPrintfException(info, mname, b.toChar))
           (false, n)
         case ((percentx, n), b) =>
           (if (b == '%') !percentx else false /* %% -> percentx = false */, n)
       }
-      if (percent) errors append new BadPrintfTrailingException(info, mname)
-      if (npercents != i) errors append new BadPrintfIncorrectNumException(info, mname)
+      if (percent) errors.append(new BadPrintfTrailingException(info, mname))
+      if (npercents != i) errors.append(new BadPrintfIncorrectNumException(info, mname))
     }
 
     def checkValidLoc(info: Info, mname: String, e: Expression) = e match {
       case _: UIntLiteral | _: SIntLiteral | _: DoPrim =>
-        errors append new InvalidLOCException(info, mname)
+        errors.append(new InvalidLOCException(info, mname))
       case _ => // Do Nothing
     }
 
     def checkHighFormW(info: Info, mname: String)(w: Width): Width = {
       w match {
         case wx: IntWidth if wx.width < 0 =>
-          errors append new NegWidthException(info, mname)
+          errors.append(new NegWidthException(info, mname))
         case wx => // Do nothing
       }
       w
@@ -116,7 +116,7 @@ object CheckHighForm extends Pass {
     def checkHighFormT(info: Info, mname: String)(t: Type): Type =
       t map checkHighFormT(info, mname) match {
         case tx: VectorType if tx.size < 0 => 
-          errors append new NegVecSizeException(info, mname)
+          errors.append(new NegVecSizeException(info, mname))
           t
         case _ => t map checkHighFormW(info, mname)
       }
@@ -124,7 +124,7 @@ object CheckHighForm extends Pass {
     def validSubexp(info: Info, mname: String)(e: Expression): Expression = {
       e match {
         case _: WRef | _: WSubField | _: WSubIndex | _: WSubAccess | _: Mux | _: ValidIf => // No error
-        case _ => errors append new InvalidAccessException(info, mname)
+        case _ => errors.append(new InvalidAccessException(info, mname))
       }
       e
     }
@@ -132,9 +132,9 @@ object CheckHighForm extends Pass {
     def checkHighFormE(info: Info, mname: String, names: NameSet)(e: Expression): Expression = {
       e match {
         case ex: WRef if !names(ex.name) =>
-          errors append new UndeclaredReferenceException(info, mname, ex.name)
+          errors.append(new UndeclaredReferenceException(info, mname, ex.name))
         case ex: UIntLiteral if ex.value < 0 =>
-          errors append new NegUIntException(info, mname)
+          errors.append(new NegUIntException(info, mname))
         case ex: DoPrim => checkHighFormPrimop(info, mname, ex)
         case _: WRef | _: UIntLiteral | _: Mux | _: ValidIf =>
         case ex: WSubAccess => validSubexp(info, mname)(ex.exp)
@@ -147,7 +147,7 @@ object CheckHighForm extends Pass {
 
     def checkName(info: Info, mname: String, names: NameSet)(name: String): String = {
       if (names(name))
-        errors append new NotUniqueException(info, mname, name)
+        errors.append(new NotUniqueException(info, mname, name))
       names += name
       name
     }
@@ -157,16 +157,16 @@ object CheckHighForm extends Pass {
       s map checkName(info, mname, names) match {
         case sx: DefMemory =>
           if (hasFlip(sx.dataType))
-            errors append new MemWithFlipException(info, mname, sx.name)
+            errors.append(new MemWithFlipException(info, mname, sx.name))
           if (sx.depth <= 0)
-            errors append new NegMemSizeException(info, mname)
+            errors.append(new NegMemSizeException(info, mname))
         case sx: WDefInstance =>
           if (!moduleNames(sx.module))
-            errors append new ModuleNotDefinedException(info, mname, sx.module)
+            errors.append(new ModuleNotDefinedException(info, mname, sx.module))
           // Check to see if a recursive module instantiation has occured
           val childToParent = moduleGraph add (mname, sx.module)
           if (childToParent.nonEmpty)
-            errors append new InstanceLoop(info, mname, childToParent mkString "->")
+            errors.append(new InstanceLoop(info, mname, childToParent mkString "->"))
         case sx: Connect => checkValidLoc(info, mname, sx.loc)
         case sx: PartialConnect => checkValidLoc(info, mname, sx.loc)
         case sx: Print => checkFstring(info, mname, sx.string, sx.args.length)
@@ -193,7 +193,7 @@ object CheckHighForm extends Pass {
     c.modules foreach checkHighFormM
     c.modules count (_.name == c.main) match {
       case 1 =>
-      case _ => errors append new NoTopModuleException(c.info, c.main)
+      case _ => errors.append(new NoTopModuleException(c.info, c.main))
     }
     errors.trigger()
     c
@@ -225,6 +225,8 @@ object CheckTypes extends Pass {
     s"$info: [module $mname]  Printf arguments must be either UIntType or SIntType.")
   class ReqClk(info: Info, mname: String) extends PassException(
     s"$info: [module $mname]  Requires a clock typed signal.")
+  class RegReqClk(info: Info, mname: String, name: String) extends PassException(
+    s"$info: [module $mname]  Register $name requires a clock typed signal.")
   class EnNotUInt(info: Info, mname: String) extends PassException(
     s"$info: [module $mname]  Enable must be a UIntType typed signal.")
   class PredNotUInt(info: Info, mname: String) extends PassException(
@@ -258,6 +260,8 @@ object CheckTypes extends Pass {
     s"$info: [module $mname]  Cannot declare a reg, node, or memory with an Analog type: $decName.")
   class IllegalAttachExp(info: Info, mname: String, expName: String) extends PassException(
     s"$info: [module $mname]  Attach expression must be an port, wire, or port of instance: $expName.")
+  class IllegalResetType(info: Info, mname: String, exp: String) extends PassException(
+    s"$info: [module $mname]  Register resets must have type UInt<1>: $exp.")
 
   //;---------------- Helper Functions --------------
   def ut: UIntType = UIntType(UnknownWidth)
@@ -275,7 +279,7 @@ object CheckTypes extends Pass {
     def check_types_primop(info: Info, mname: String, e: DoPrim) {
       def all_same_type (ls:Seq[Expression]) {
         if (ls exists (x => wt(ls.head.tpe) != wt(e.tpe)))
-          errors append new OpNotAllSameType(info, mname, e.op.serialize)
+          errors.append(new OpNotAllSameType(info, mname, e.op.serialize))
       }
       def allUSC(ls: Seq[Expression]) {
         val error = ls.foldLeft(false)((error, x) => x.tpe match {
@@ -295,7 +299,7 @@ object CheckTypes extends Pass {
         if (ls exists (x => x.tpe match {
           case _: UIntType | _: SIntType => false
           case _ => true
-        })) errors append new OpNotGround(info, mname, e.op.serialize)
+        })) errors.append(new OpNotGround(info, mname, e.op.serialize))
       }
       def allF(ls: Seq[Expression]) {
         val error = ls.foldLeft(false)((error, x) => x.tpe match {
@@ -314,13 +318,13 @@ object CheckTypes extends Pass {
         if (ls exists (x => x.tpe match {
           case _: UIntType => false
           case _ => true
-        })) errors append new OpNotAllUInt(info, mname, e.op.serialize)
+        })) errors.append(new OpNotAllUInt(info, mname, e.op.serialize))
       }
       def is_uint (x:Expression) {
         if (x.tpe match {
           case _: UIntType => false
           case _ => true
-        }) errors append new OpNotUInt(info, mname, e.op.serialize, x.serialize)
+        }) errors.append(new OpNotUInt(info, mname, e.op.serialize, x.serialize))
       }
       e.op match {
         case AsUInt | AsSInt | AsFixedPoint =>
@@ -339,44 +343,44 @@ object CheckTypes extends Pass {
         case (e: WSubField) => e.exp.tpe match {
           case (t: BundleType) => t.fields find (_.name == e.name) match {
             case Some(_) =>
-            case None => errors append new SubfieldNotInBundle(info, mname, e.name)
+            case None => errors.append(new SubfieldNotInBundle(info, mname, e.name))
           }
-          case _ => errors append new SubfieldOnNonBundle(info, mname, e.name)
+          case _ => errors.append(new SubfieldOnNonBundle(info, mname, e.name))
         }
         case (e: WSubIndex) => e.exp.tpe match {
           case (t: VectorType) if e.value < t.size =>
           case (t: VectorType) =>
-            errors append new IndexTooLarge(info, mname, e.value)
+            errors.append(new IndexTooLarge(info, mname, e.value))
           case _ =>
-            errors append new IndexOnNonVector(info, mname)
+            errors.append(new IndexOnNonVector(info, mname))
         }
         case (e: WSubAccess) =>
           e.exp.tpe match {
             case _: VectorType =>
-            case _ => errors append new IndexOnNonVector(info, mname)
+            case _ => errors.append(new IndexOnNonVector(info, mname))
           }
           e.index.tpe match {
             case _: UIntType =>
-            case _ => errors append new AccessIndexNotUInt(info, mname)
+            case _ => errors.append(new AccessIndexNotUInt(info, mname))
           }
         case (e: DoPrim) => check_types_primop(info, mname, e)
         case (e: Mux) =>
           if (wt(e.tval.tpe) != wt(e.fval.tpe))
-            errors append new MuxSameType(info, mname)
+            errors.append(new MuxSameType(info, mname))
           if (!passive(e.tpe))
-            errors append new MuxPassiveTypes(info, mname)
+            errors.append(new MuxPassiveTypes(info, mname))
           e.cond.tpe match {
             case _: UIntType =>
-            case _ => errors append new MuxCondUInt(info, mname)
+            case _ => errors.append(new MuxCondUInt(info, mname))
           }
           if ((e.tval.tpe == ClockType) || (e.fval.tpe == ClockType))
             errors.append(new MuxClock(info, mname))
         case (e: ValidIf) =>
           if (!passive(e.tpe))
-            errors append new ValidIfPassiveTypes(info, mname)
+            errors.append(new ValidIfPassiveTypes(info, mname))
           e.cond.tpe match {
             case _: UIntType =>
-            case _ => errors append new ValidIfCondUInt(info, mname)
+            case _ => errors.append(new ValidIfCondUInt(info, mname))
           }
         case _ =>
       }
@@ -411,19 +415,28 @@ object CheckTypes extends Pass {
       val info = get_info(s) match { case NoInfo => minfo case x => x }
       s match {
         case sx: Connect if wt(sx.loc.tpe) != wt(sx.expr.tpe) =>
-          errors append new InvalidConnect(info, mname, sx.loc.serialize, sx.expr.serialize)
+          errors.append(new InvalidConnect(info, mname, sx.loc.serialize, sx.expr.serialize))
         case sx: PartialConnect if !bulk_equals(sx.loc.tpe, sx.expr.tpe, Default, Default) =>
-          errors append new InvalidConnect(info, mname, sx.loc.serialize, sx.expr.serialize)
-        case sx: DefRegister => sx.tpe match {
-          case AnalogType(w) => errors append new IllegalAnalogDeclaration(info, mname, sx.name)
-          case t if wt(sx.tpe) != wt(sx.init.tpe) => errors append new InvalidRegInit(info, mname)
-          case t =>
-        }
+          errors.append(new InvalidConnect(info, mname, sx.loc.serialize, sx.expr.serialize))
+        case sx: DefRegister =>
+          sx.tpe match {
+            case AnalogType(_) => errors.append(new IllegalAnalogDeclaration(info, mname, sx.name))
+            case t if wt(sx.tpe) != wt(sx.init.tpe) => errors.append(new InvalidRegInit(info, mname))
+            case t =>
+          }
+          sx.reset.tpe match {
+            case UIntType(IntWidth(w)) if w == 1 =>
+            case UIntType(UnknownWidth) => // cannot catch here, though width may ultimately be wrong
+            case _ => errors.append(new IllegalResetType(info, mname, sx.name))
+          }
+          if (sx.clock.tpe != ClockType) {
+            errors.append(new RegReqClk(info, mname, sx.name))
+          }
         case sx: Conditionally if wt(sx.pred.tpe) != wt(ut) =>
-          errors append new PredNotUInt(info, mname)
+          errors.append(new PredNotUInt(info, mname))
         case sx: DefNode => sx.value.tpe match {
-          case AnalogType(w) => errors append new IllegalAnalogDeclaration(info, mname, sx.name)
-          case t if !passive(sx.value.tpe) => errors append new NodePassiveType(info, mname)
+          case AnalogType(w) => errors.append(new IllegalAnalogDeclaration(info, mname, sx.name))
+          case t if !passive(sx.value.tpe) => errors.append(new NodePassiveType(info, mname))
           case t =>
         }
         case sx: Attach =>
@@ -438,15 +451,15 @@ object CheckTypes extends Pass {
             }
           }
         case sx: Stop =>
-          if (wt(sx.clk.tpe) != wt(ClockType)) errors append new ReqClk(info, mname)
-          if (wt(sx.en.tpe) != wt(ut)) errors append new EnNotUInt(info, mname)
+          if (wt(sx.clk.tpe) != wt(ClockType)) errors.append(new ReqClk(info, mname))
+          if (wt(sx.en.tpe) != wt(ut)) errors.append(new EnNotUInt(info, mname))
         case sx: Print =>
           if (sx.args exists (x => wt(x.tpe) != wt(ut) && wt(x.tpe) != wt(st)))
-            errors append new PrintfArgNotGround(info, mname)
-          if (wt(sx.clk.tpe) != wt(ClockType)) errors append new ReqClk(info, mname)
-          if (wt(sx.en.tpe) != wt(ut)) errors append new EnNotUInt(info, mname)
+            errors.append(new PrintfArgNotGround(info, mname))
+          if (wt(sx.clk.tpe) != wt(ClockType)) errors.append(new ReqClk(info, mname))
+          if (wt(sx.en.tpe) != wt(ut)) errors.append(new EnNotUInt(info, mname))
         case sx: DefMemory => sx.dataType match {
-          case AnalogType(w) => errors append new IllegalAnalogDeclaration(info, mname, sx.name)
+          case AnalogType(w) => errors.append(new IllegalAnalogDeclaration(info, mname, sx.name))
           case t =>
         }
         case _ =>
@@ -502,11 +515,11 @@ object CheckGenders extends Pass {
       val gender = get_gender(e,genders)
       (gender, desired) match {
         case (MALE, FEMALE) =>
-          errors append new WrongGender(info, mname, e.serialize, desired, gender)
+          errors.append(new WrongGender(info, mname, e.serialize, desired, gender))
         case (FEMALE, MALE) => kind(e) match {
           case PortKind | InstanceKind if !flip_q(e.tpe) => // OK!
           case _ =>
-            errors append new WrongGender(info, mname, e.serialize, desired, gender)
+            errors.append(new WrongGender(info, mname, e.serialize, desired, gender))
         }
         case _ =>
       }
diff --git a/src/test/scala/firrtlTests/CheckSpec.scala b/src/test/scala/firrtlTests/CheckSpec.scala
index 2fe5baf5..0d0df020 100644
--- a/src/test/scala/firrtlTests/CheckSpec.scala
+++ b/src/test/scala/firrtlTests/CheckSpec.scala
@@ -185,4 +185,61 @@ class CheckSpec extends FlatSpec with Matchers {
     }
   }
 
+  "Clocks with types other than ClockType" should "throw an exception" in {
+    val passes = Seq(
+      ToWorkingIR,
+      CheckHighForm,
+      ResolveKinds,
+      InferTypes,
+      CheckTypes)
+    val input =
+      """
+          |circuit Top : 
+          |    
+          |  module Top :
+          |    input clk : UInt<1>
+          |    input i : UInt<1>
+          |    output o : UInt<1>
+          |
+          |    reg r : UInt<1>, clk
+          |    r <= i
+          |    o <= r
+          |
+          |""".stripMargin
+    intercept[CheckTypes.RegReqClk] {
+      passes.foldLeft(Parser.parse(input.split("\n").toIterator)) {
+        (c: Circuit, p: Pass) => p.run(c)
+      }
+    }
+  }
+
+  "Illegal reset type" should "throw an exception" in {
+    val passes = Seq(
+      ToWorkingIR,
+      CheckHighForm,
+      ResolveKinds,
+      InferTypes,
+      CheckTypes)
+    val input =
+      """
+          |circuit Top : 
+          |    
+          |  module Top :
+          |    input clk : Clock
+          |    input reset : UInt<2>
+          |    input i : UInt<1>
+          |    output o : UInt<1>
+          |
+          |    reg r : UInt<1>, clk with : (reset => (reset, UInt<1>("h00")))
+          |    r <= i
+          |    o <= r
+          |
+          |""".stripMargin
+    intercept[CheckTypes.IllegalResetType] {
+      passes.foldLeft(Parser.parse(input.split("\n").toIterator)) {
+        (c: Circuit, p: Pass) => p.run(c)
+      }
+    }
+  }
+
 }