From 5891a0eb7df097b779c26a6b385727ebce79dbbf Mon Sep 17 00:00:00 2001
From: Zachary Snow <zach@zachjs.com>
Date: Wed, 30 Dec 2020 17:17:27 -0700
Subject: [PATCH] improve edge case messaging

- error for unbound module instance interface ports
- warning for converting an interface alone
- warning for converting a package alone
- update terminology for modules removed by interface conversion
---
 src/Convert/Interface.hs                | 29 +++++++++++++++----------
 src/sv2v.hs                             | 24 ++++++++++++++++++++
 test/error/interface_unbound_modport.sv | 12 ++++++++++
 3 files changed, 54 insertions(+), 11 deletions(-)
 create mode 100644 test/error/interface_unbound_modport.sv

diff --git a/src/Convert/Interface.hs b/src/Convert/Interface.hs
index cab813d..268a23e 100644
--- a/src/Convert/Interface.hs
+++ b/src/Convert/Interface.hs
@@ -62,7 +62,7 @@ convertDescription parts (Part attrs extern Module lifetime name ports items) =
         Part attrs extern Module lifetime name ports items'
     else
         PackageItem $ Decl $ CommentDecl $
-            "removed interface-using module: " ++ name
+            "removed module with interface ports: " ++ name
     where
         items' = evalScoper return traverseModuleItemM return return name items
 
@@ -80,15 +80,21 @@ convertDescription parts (Part attrs extern Module lifetime name ports items) =
                 convertNested $ Generate $ map GenModuleItem $
                     inlineInstance rs []
                     partItems instanceName paramBindings portBindings
-            else if not $ null (extractModportInstances partInfo) then do
-                modports <- embedScopes (\l () -> l) ()
-                -- inline instantiation of a module
-                convertNested $ Generate $ map GenModuleItem $
-                    inlineInstance rs
-                        (modportBindings modports)
-                        partItems instanceName paramBindings portBindings
-            else
+            else if null modportInstances then
                 return instanceItem
+            else do
+                -- inline instantiation of a module
+                modportBindings <-
+                    embedScopes (\l () -> getModportBindings l) ()
+                if length modportInstances /= length modportBindings
+                    then
+                        error $ "instance " ++ instanceName ++ " of " ++ part
+                            ++ " has interface ports "
+                            ++ showKeys modportInstances ++ ", but only "
+                            ++ showKeys modportBindings ++ " are connected"
+                    else convertNested $ Generate $ map GenModuleItem $
+                            inlineInstance rs modportBindings partItems
+                            instanceName paramBindings portBindings
             where
                 Instance part rawParamBindings instanceName rs rawPortBindings =
                     instanceItem
@@ -101,10 +107,11 @@ convertDescription parts (Part attrs extern Module lifetime name ports items) =
                 portBindings = resolveBindings partPorts rawPortBindings
 
                 modportInstances = extractModportInstances partInfo
-                modportBindings modports = mapMaybe
+                getModportBindings modports = mapMaybe
                     (inferModportBinding modports modportInstances) $
                     map (second $ addImpliedSlice modports) portBindings
                 second f = \(a, b) -> (a, f b)
+                showKeys = show . map fst
 
         traverseModuleItemM other = return other
 
@@ -317,7 +324,7 @@ inlineInstance ranges modportBindings items
         inlineKind =
             if null modportBindings
                 then "interface"
-                else "interface-using module"
+                else "module"
 
         comment = MIPackageItem $ Decl $ CommentDecl $
             "expanded " ++ inlineKind ++ " instance: " ++ instanceName
diff --git a/src/sv2v.hs b/src/sv2v.hs
index eafe553..89c8df8 100644
--- a/src/sv2v.hs
+++ b/src/sv2v.hs
@@ -10,6 +10,7 @@ import System.Exit
 import Data.List (elemIndex)
 import Job (readJob, files, exclude, incdir, define, siloed, skipPreprocessor)
 import Convert (convert)
+import Language.SystemVerilog.AST
 import Language.SystemVerilog.Parser (parseFiles)
 
 splitDefine :: String -> (String, String)
@@ -18,6 +19,28 @@ splitDefine str =
         Nothing -> (str, "")
         Just idx -> (take idx str, drop (idx + 1) str)
 
+isInterface :: Description -> Bool
+isInterface (Part _ _ Interface _ _ _ _ ) = True
+isInterface _ = False
+
+isPackage :: Description -> Bool
+isPackage Package{} = True
+isPackage _ = False
+
+emptyWarnings :: [AST] -> [AST] -> IO ()
+emptyWarnings before after =
+    if all null before || any (not . null) after then
+        return ()
+    else if any (any isInterface) before then
+        hPutStr stderr $ "Warning: Source includes an interface but output is "
+            ++ "empty because there is no top-level module which has no ports "
+            ++ "which are interfaces."
+    else if any (any isPackage) before then
+        hPutStr stderr $ "Warning: Source includes packages but no modules. "
+            ++ "Please convert packages alongside the modules that use them."
+    else
+        return ()
+
 main :: IO ()
 main = do
     job <- readJob
@@ -32,6 +55,7 @@ main = do
         Right asts -> do
             -- convert the files
             let asts' = convert (exclude job) asts
+            emptyWarnings asts asts'
             -- print the converted files out
             hPrint stdout $ concat asts'
             exitSuccess
diff --git a/test/error/interface_unbound_modport.sv b/test/error/interface_unbound_modport.sv
new file mode 100644
index 0000000..7b507a5
--- /dev/null
+++ b/test/error/interface_unbound_modport.sv
@@ -0,0 +1,12 @@
+interface Interface;
+    logic x;
+endinterface
+
+module Module(i);
+    Interface i;
+endmodule
+
+module top;
+    Interface i();
+    Module m();
+endmodule