refactor parameter binding resolution

- fix type/expr ambiguity for interface and class parameters
- check for parameter kind mismatch up front
- centralize key logic in ResolveBindings

src/Convert/Package.hs

@@ -30,6 +30,7 @@ import Data.Maybe (mapMaybe)
 import qualified Data.Map.Strict as Map
 import qualified Data.Set as Set
 
+import Convert.ResolveBindings (exprToType, resolveBindings)
 import Convert.Scoper
 import Convert.Traverse
 import Language.SystemVerilog.AST
@@ -504,10 +505,12 @@ resolveCSIdent className paramBindings scopeKeys itemName = do
             Decl $ ParamType Parameter x $
                 case lookup x' bindings of
                     Just (Left t') -> t'
-                    Just (Right (Ident y)) -> Alias y []
                     Just (Right e') ->
-                        error $ "cannot override type parameter " ++ show x'
-                            ++ " in class " ++ show className
+                        case exprToType e' of
+                            Just t' -> t'
+                            Nothing ->
+                                error $ "cannot override type parameter "
+                                    ++ show x' ++ " in class " ++ show className
                                     ++ " with expression " ++ show e'
                     Nothing ->
                         if t == UnknownType

src/Convert/ParamType.hs

@@ -13,7 +13,6 @@ import Data.Maybe (isJust, isNothing, fromJust)
 import qualified Data.Map.Strict as Map
 import qualified Data.Set as Set
 
-import Convert.ExprUtils
 import Convert.Traverse
 import Language.SystemVerilog.AST
 
@@ -241,25 +240,6 @@ isDefaultName m =
 defaultTag :: Identifier
 defaultTag = "_sv2v_default"
 
--- attempt to convert an expression to syntactically equivalent type
-exprToType :: Expr -> Maybe Type
-exprToType (Ident       x) = Just $ Alias       x []
-exprToType (PSIdent y   x) = Just $ PSAlias y   x []
-exprToType (CSIdent y p x) = Just $ CSAlias y p x []
-exprToType (Range e NonIndexed r) =
-    case exprToType e of
-        Nothing -> Nothing
-        Just t -> Just $ tf (rs ++ [r])
-            where (tf, rs) = typeRanges t
-exprToType (Bit e i) =
-    case exprToType e of
-        Nothing -> Nothing
-        Just t -> Just $ tf (rs ++ [r])
-            where
-                (tf, rs) = typeRanges t
-                r = (simplify $ BinOp Sub i (RawNum 1), RawNum 0)
-exprToType _ = Nothing
 -- checks where a type is sufficiently resolved to be substituted
 isSimpleType :: Type -> Bool
 isSimpleType typ =
@@ -352,21 +332,11 @@ convertModuleItemM info (orig @ (Instance m bindings x r p)) =
             Map.toList resolvedTypesWithDecls
         resolveType :: Identifier -> Maybe Type -> (Type, (IdentSet, [Decl]))
         resolveType paramName defaultType =
-            case (Map.lookup paramName bindingsMap, defaultType) of
-                (Nothing, Just t) -> (t, (Set.empty, []))
-                (Nothing, Nothing) ->
-                    error $ "instantiation " ++ show orig ++
-                        " is missing a type parameter: " ++ paramName
-                (Just (Left t), _) -> prepareTypeExprs x paramName t
-                (Just (Right e), _) ->
-                    -- Some types are parsed as expressions because of the
-                    -- ambiguities of defined type names.
-                    case exprToType e of
-                        Just t -> prepareTypeExprs x paramName t
-                        Nothing ->
-                            error $ "instantiation " ++ show orig
-                                ++ " has expr " ++ show e
-                                ++ " for type param: " ++ paramName
+            case Map.lookup paramName bindingsMap of
+                Nothing -> (t, (Set.empty, []))
+                    where Just t = defaultType
+                Just b -> prepareTypeExprs x paramName t
+                    where Left t = b
 
         -- leave only the normal expression params behind
         isParamType = flip Map.member maybeTypeMap

src/Convert/ResolveBindings.hs

+{-# LANGUAGE TupleSections #-}
 {- sv2v
  - Author: Zachary Snow <zach@zachjs.com>
  -
  - Conversion for `.*` and unnamed bindings
  -
  - While positional bindings need not be converted, resolving them here
- - simplifies downstream conversions.
+ - simplifies downstream conversions. This conversion is also responsible for
+ - performing some basic validation and resolving the ambiguity between types
+ - and expressions in parameter binding contexts.
  -}
 
-module Convert.ResolveBindings (convert) where
+module Convert.ResolveBindings
+    ( convert
+    , exprToType
+    , resolveBindings
+    ) where
 
 import Control.Monad.Writer.Strict
 import qualified Data.Map.Strict as Map
 
+import Convert.ExprUtils (simplify)
 import Convert.Traverse
 import Language.SystemVerilog.AST
 
-type Ports = Map.Map Identifier ([Identifier], [Identifier])
+type Parts = Map.Map Identifier ([(Identifier, Bool)], [Identifier])
 
 convert :: [AST] -> [AST]
 convert =
     traverseFiles
-        (collectDescriptionsM collectPortsM)
+        (collectDescriptionsM collectPartsM)
         (traverseDescriptions . traverseModuleItems . mapInstance)
 
-collectPortsM :: Description -> Writer Ports ()
-collectPortsM (Part _ _ _ _ name ports items) =
+collectPartsM :: Description -> Writer Parts ()
+collectPartsM (Part _ _ _ _ name ports items) =
     tell $ Map.singleton name (params, ports)
-    where params = parameterNames items
-collectPortsM _ = return ()
+    where params = parameterInfos items
+collectPartsM _ = return ()
 
--- given a list of module items, produces the parameter names in order
-parameterNames :: [ModuleItem] -> [Identifier]
-parameterNames =
+-- given a list of module items, produces the parameters in order
+parameterInfos :: [ModuleItem] -> [(Identifier, Bool)]
+parameterInfos =
     execWriter . mapM (collectNestedModuleItemsM $ collectDeclsM collectDeclM)
     where
-        collectDeclM :: Decl -> Writer [Identifier] ()
-        collectDeclM (Param Parameter   _ x _) = tell [x]
-        collectDeclM (ParamType Parameter x _) = tell [x]
+        collectDeclM :: Decl -> Writer [(Identifier, Bool)] ()
+        collectDeclM (Param Parameter   _ x _) = tell [(x, False)]
+        collectDeclM (ParamType Parameter x _) = tell [(x, True)]
         collectDeclM _ = return ()
 
-mapInstance :: Ports -> ModuleItem -> ModuleItem
-mapInstance modulePorts (Instance m paramBindings x rs portBindings) =
+mapInstance :: Parts -> ModuleItem -> ModuleItem
+mapInstance parts (Instance m paramBindings x rs portBindings) =
     -- if we can't find it, just skip :(
-    if maybeModuleInfo == Nothing
+    if maybePartInfo == Nothing
         then Instance m paramBindings x rs portBindings
         else Instance m paramBindings' x rs portBindings'
     where
-        maybeModuleInfo = Map.lookup m modulePorts
-        Just (paramNames, portNames) = maybeModuleInfo
+        maybePartInfo = Map.lookup m parts
+        Just (paramInfos, portNames) = maybePartInfo
+        paramNames = map fst paramInfos
 
         msg :: String -> String
         msg = flip (++) $ " in instance " ++ show x ++ " of " ++ show m
 
-        paramBindings' = resolveBindings (msg "parameter overrides") paramNames
-            paramBindings
-        portBindings' = resolveBindings (msg "port connections") portNames
-            $ concatMap expandStar portBindings
+        paramBindings' = map checkParam $
+            resolveBindings (msg "parameter overrides") paramNames paramBindings
+        portBindings' = resolveBindings (msg "port connections") portNames $
+            concatMap expandStar portBindings
 
         expandStar :: PortBinding -> [PortBinding]
         expandStar ("*", Nil) =
@@ -64,4 +73,53 @@ mapInstance modulePorts (Instance m paramBindings x rs portBindings) =
             where alreadyBound = map fst portBindings
         expandStar other = [other]
 
+        -- ensures parameter and binding kinds (type vs. expr) match
+        checkParam :: ParamBinding -> ParamBinding
+        checkParam (paramName, Right e) =
+            (paramName, ) $
+            if isType
+                then case exprToType e of
+                    Nothing -> kindMismatch paramName "a type" "expression" e
+                    Just t -> Left t
+                else Right e
+            where Just isType = lookup paramName paramInfos
+        checkParam (paramName, Left t) =
+            if isType
+                then (paramName, Left t)
+                else kindMismatch paramName "an expression" "type" t
+            where Just isType = lookup paramName paramInfos
+
+        kindMismatch :: Show k => Identifier -> String -> String -> k -> a
+        kindMismatch paramName expected actual value =
+            error $ msg ("parameter " ++ show paramName)
+                ++ " expects " ++ expected
+                ++ ", but was given " ++ actual
+                ++ ' ' : show value
+
 mapInstance _ other = other
+
+-- attempt to convert an expression to syntactically equivalent type
+exprToType :: Expr -> Maybe Type
+exprToType (Ident       x) = Just $ Alias       x []
+exprToType (PSIdent y   x) = Just $ PSAlias y   x []
+exprToType (CSIdent y p x) = Just $ CSAlias y p x []
+exprToType (Range e NonIndexed r) = do
+    (tf, rs) <- fmap typeRanges $ exprToType e
+    Just $ tf (rs ++ [r])
+exprToType (Bit e i) = do
+    (tf, rs) <- fmap typeRanges $ exprToType e
+    let r = (simplify $ BinOp Sub i (RawNum 1), RawNum 0)
+    Just $ tf (rs ++ [r])
+exprToType _ = Nothing
+
+type Binding t = (Identifier, t)
+-- give a set of bindings explicit names
+resolveBindings :: String -> [Identifier] -> [Binding t] -> [Binding t]
+resolveBindings _ _ [] = []
+resolveBindings location available bindings =
+    if length available < length bindings then
+        error $ "too many bindings specified for " ++ location
+    else if null $ fst $ head bindings then
+        zip available $ map snd bindings
+    else
+        bindings

src/Language/SystemVerilog/AST.hs

@@ -29,7 +29,6 @@ module Language.SystemVerilog.AST
     , exprToLHS
     , lhsToExpr
     , shortHash
-    , resolveBindings
     ) where
 
 import Text.Printf (printf)
@@ -85,15 +84,3 @@ shortHash :: (Show a) => a -> String
 shortHash x =
     take 5 $ printf "%05X" val
     where val = hash $ show x
-
-type Binding t = (Identifier, t)
--- give a set of bindings explicit names
-resolveBindings :: String -> [Identifier] -> [Binding t] -> [Binding t]
-resolveBindings _ _ [] = []
-resolveBindings location available bindings =
-    if length available < length bindings then
-        error $ "too many bindings specified for " ++ location
-    else if null $ fst $ head bindings then
-        zip available $ map snd bindings
-    else
-        bindings

test/basic/ambiguous_tore.sv

+class C #(
+    parameter PARAM_E = 1,
+    parameter type PARAM_T = logic
+);
+    localparam E = PARAM_E * $bits(PARAM_T);
+    localparam type T = PARAM_T [PARAM_E - 1:0];
+endclass
+
+package P;
+    localparam E = 2;
+    localparam type T = logic [E - 1:0];
+endpackage
+
+module M #(
+    parameter DELAY,
+    parameter PREFIX,
+    parameter SUFFIX,
+    parameter E,
+    parameter type T
+);
+    initial #DELAY $display("M: %sE%s=%0d $bits(%sT%s)=%0d",
+        PREFIX, SUFFIX, E, PREFIX, SUFFIX, $bits(T));
+endmodule
+
+interface I #(
+    parameter DELAY,
+    parameter PREFIX,
+    parameter SUFFIX,
+    parameter E,
+    parameter type T
+);
+    initial #DELAY $display("I: %sE%s=%0d $bits(%sT%s)=%0d",
+        PREFIX, SUFFIX, E, PREFIX, SUFFIX, $bits(T));
+endinterface
+
+module top;
+    parameter FLAG = 1;
+
+    localparam E = 3;
+    localparam type T = logic [E * 2 - 1:0];
+
+    `define TEST(D, prefix) \
+        if (FLAG) begin \
+            localparam the_expr = prefix``E; \
+            localparam type the_type = prefix``T; \
+            initial begin \
+                #(D * 10); \
+                $display(`"prefix``E = %0d %0d`", the_expr, prefix``E); \
+                $display(`"$bits(prefix``E) = %0d %0d`", $bits(the_expr), $bits(prefix``E)); \
+                $display(`"$bits(prefix``T) = %0d %0d`", $bits(the_type), $bits(prefix``T)); \
+                $display(`"$left(prefix``E) = %0d`", $left(prefix``E)); \
+                $display(`"$left(prefix``T) = %0d`", $left(prefix``T)); \
+                $display(`"prefix``E'('z) = %b`", prefix``E'('z)); \
+                $display(`"prefix``T'('z) = %b`", prefix``T'('z)); \
+            end \
+            I #(D*10+1, `"prefix`", "", prefix``E, prefix``T) i1(); \
+            M #(D*10+2, `"prefix`", "", prefix``E, prefix``T) m1(); \
+            I #(D*10+3, `"prefix`", "[6]", prefix``E[6], prefix``T[6]) i2(); \
+            M #(D*10+4, `"prefix`", "[6]", prefix``E[6], prefix``T[6]) m2(); \
+            I #(D*10+5, `"prefix`", "[6:0]", prefix``E[6:0], prefix``T[6:0]) i3(); \
+            M #(D*10+6, `"prefix`", "[6:0]", prefix``E[6:0], prefix``T[6:0]) m3(); \
+        end
+
+    `TEST(0, )
+    `TEST(1, P::)
+
+    `TEST(2, C#()::)
+    `TEST(3, C#(E, T)::)
+    `TEST(4, C#(P::E, P::T)::)
+    `TEST(5, C#(C#(E, T)::E, C#(E, T)::T)::)
+    `TEST(6, C#(E[1], T[2])::)
+    `TEST(7, C#(E[2:0], T[2:0])::)
+
+endmodule

test/basic/ambiguous_tore.v

+module MI;
+    parameter KIND = "";
+    parameter DELAY = 0;
+    parameter PREFIX = "";
+    parameter SUFFIX = "";
+    parameter E = 1;
+    parameter T = 1;
+    initial #DELAY $display("%s: %sE%s=%0d $bits(%sT%s)=%0d",
+        KIND, PREFIX, SUFFIX, E, PREFIX, SUFFIX, T);
+endmodule
+
+module top;
+    parameter FLAG = 1;
+
+    `define TEST(D, prefix, eval, tval, leftval) \
+        if (FLAG) begin \
+            localparam [eval:1] ez = 1'sbz; \
+            localparam [tval:1] tz = 1'sbz; \
+            initial begin \
+                #(D * 10); \
+                $display(`"prefix``E = %0d %0d`", eval, eval); \
+                $display(`"$bits(prefix``E) = %0d %0d`", 32, 32); \
+                $display(`"$bits(prefix``T) = %0d %0d`", tval, tval); \
+                $display(`"$left(prefix``E) = %0d`", 31); \
+                $display(`"$left(prefix``T) = %0d`", leftval); \
+                $display(`"prefix``E'('z) = %b`", ez); \
+                $display(`"prefix``T'('z) = %b`", tz); \
+            end \
+            MI #("I", D*10+1, `"prefix`", "", eval, tval) i1(); \
+            MI #("M", D*10+2, `"prefix`", "", eval, tval) m1(); \
+            MI #("I", D*10+3, `"prefix`", "[6]", (eval >> 6) & 1'b1, tval*6) i2(); \
+            MI #("M", D*10+4, `"prefix`", "[6]", (eval >> 6) & 1'b1, tval*6) m2(); \
+            MI #("I", D*10+5, `"prefix`", "[6:0]", eval & 7'h7f, tval * 7) i3(); \
+            MI #("M", D*10+6, `"prefix`", "[6:0]", eval & 7'h7f, tval * 7) m3(); \
+        end
+
+    `define TEST_C(D, prefix, efac, tfac) \
+        `TEST(D, prefix, (efac)*(tfac), (tfac)*(efac), (efac)-1)
+
+    `TEST(0, , 3, 6, 5)
+    `TEST(1, P::, 2, 2, 1)
+
+    `TEST_C(2, C#()::, 1, 1)
+    `TEST_C(3, C#(E, T)::, 3, 6)
+    `TEST_C(4, C#(P::E, P::T)::, 2, 2)
+    `TEST_C(5, C#(C#(E, T)::E, C#(E, T)::T)::, 18, 18)
+    `TEST_C(6, C#(E[1], T[2])::, 1, 12)
+    `TEST_C(7, C#(E[2:0], T[2:0])::, 3, 18)
+
+endmodule

test/error/interface_param_mismatch_expr.sv

+// pattern: parameter "P" in instance "intf" of "Interface" expects an expression, but was given type logic
 interface Interface;
-    parameter T = 0;
-    logic [T-1:0] x;
+    parameter P = 0;
+    logic [P-1:0] x;
 endinterface
-
 module top;
     Interface #(logic) intf();
 endmodule

test/error/interface_param_mismatch_type.sv

+// pattern: parameter "P" in instance "intf" of "Interface" expects a type, but was given expression 1
 interface Interface;
-    parameter type T;
-    T x;
+    parameter type P;
+    P x;
 endinterface
-
 module top;
     Interface #(1) intf();
 endmodule

test/error/module_param_mismatch_expr.sv

+// pattern: parameter "P" in instance "mod" of "Module" expects an expression, but was given type logic
+module Module;
+    parameter P = 0;
+    logic [P-1:0] x;
+endmodule
+module top;
+    Module #(logic) mod();
+endmodule

test/error/module_param_mismatch_type.sv

+// pattern: parameter "P" in instance "mod" of "Module" expects a type, but was given expression 1
+module Module;
+    parameter type P;
+    P x;
+endmodule
+module top;
+    Module #(1) mod();
+endmodule