Merge pull request #312 from ucb-bar/improveunboundioerrormessage305

Generate a better error message for missing IO() wrapper - fix #305

2 files changed, 48 insertions, 4 deletions

diff --git a/chiselFrontend/src/main/scala/chisel3/core/Binding.scala b/chiselFrontend/src/main/scala/chisel3/core/Binding.scala
index 5378f3ae..467cb4eb 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Binding.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Binding.scala
@@ -53,6 +53,7 @@ object Binding {
   case class BindingException(message: String) extends Exception(message)
   def AlreadyBoundException(binding: String) = BindingException(s": Already bound to $binding")
   def NotSynthesizableException = BindingException(s": Not bound to synthesizable node, currently only Type description")
+  def MissingIOWrapperException = BindingException(": Missing IO() wrapper")
 
   // This recursively walks down the Data tree to look at all the leaf 'Element's
   // Will build up an error string in case something goes wrong
@@ -138,6 +139,29 @@ object Binding {
         }
       }
     }
+
+    /** Diagnose a binding error caused by a missing IO() wrapper.
+      * @param element the element triggering the binding error.
+      * @return true if the element is a member of the module's io but ioDefined is false.
+      */
+    def isMissingIOWrapper(element: Element): Boolean = {
+      element._parent match {
+        case None => false
+        case Some(x: Module) => {
+          // If the IO() wrapper has been executed, it isn't missing.
+          if (x.ioDefined) {
+            false
+          } else {
+            // TODO: We should issue the message only once, and if we get here,
+            //  we know the wrapper is missing, whether or not the element is a member of io.
+            //  But if it's not an io element, we want to issue the complementary "unbound" error.
+            //  Revisit this when we collect error messages instead of throwing exceptions.
+            x.io.flatten.contains(element)
+          }
+        }
+      }
+    }
+
     try walkToBinding(
       target,
       element => element.binding match {
@@ -145,10 +169,16 @@ object Binding {
         case binding =>
           // The following kludge is an attempt to provide backward compatibility
           // It should be done at at higher level.
-          if ((forcedModule.compileOptions.requireIOWrap || !elementOfIO(element)))
-            throw NotSynthesizableException
-          else
+          if ((forcedModule.compileOptions.requireIOWrap || !elementOfIO(element))) {
+            // Generate a better error message if this is a result of a missing IO() wrapper.
+            if (isMissingIOWrapper(element)) {
+              throw MissingIOWrapperException
+            } else {
+              throw NotSynthesizableException
+            }
+          } else {
             Binding.bind(element, PortBinder(element._parent.get), "Error: IO")
+          }
       }
     )
     catch {
diff --git a/src/test/scala/chiselTests/IOCompatibility.scala b/src/test/scala/chiselTests/IOCompatibility.scala
index 7bf3dded..552fe776 100644
--- a/src/test/scala/chiselTests/IOCompatibility.scala
+++ b/src/test/scala/chiselTests/IOCompatibility.scala
@@ -3,6 +3,8 @@
 package chiselTests
 
 import chisel3._
+import chisel3.core.Binding.BindingException
+import org.scalatest._
 
 class IOCSimpleIO extends Bundle {
   val in  = Input(UInt(width=32))
@@ -33,7 +35,7 @@ class IOCModuleWire extends Module {
   io.out := inc.out
 }
 
-class IOCompatibilitySpec extends ChiselPropSpec {
+class IOCompatibilitySpec extends ChiselPropSpec with Matchers {
 
   property("IOCModuleVec should elaborate") {
     elaborate { new IOCModuleVec(2) }
@@ -42,4 +44,16 @@ class IOCompatibilitySpec extends ChiselPropSpec {
   property("IOCModuleWire should elaborate") {
     elaborate { new IOCModuleWire }
   }
+
+
+  class IOUnwrapped extends Module {
+    val io = new IOCSimpleIO
+    io.out := io.in
+  }
+
+  property("Unwrapped IO should generate an exception") {
+    a [BindingException] should be thrownBy {
+      elaborate(new IOUnwrapped)
+    }
+  }
 }