fix inadvertent duplicate declaration generation

Disabling the package item injection routine used in the enum conversion
when there were no items to inject exposed cases where conversions would
generate duplicate declarations. The hierarchical constant and param
type conversions were trivially affected. The package conversion could
inject class items within a generate region and then re-inject them
outside of that generate region. The package conversions now uses an
upgraded generate region flattening utility to ensure injected class
items are seen. This also includes coverage for a conflict which
occurred even without the enum conversion change.

src/Convert/Enum.hs

@@ -39,7 +39,10 @@ convertDescription :: Description -> [Description]
 convertDescription (description @ Part{}) =
     [Part attrs extern kw lifetime name ports items']
     where
-        items' = inject enumItems items -- only keep what's used
+        items' = -- only keep what's used
+            if null enumItems
+                then items
+                else inject enumItems items
         Part attrs extern kw lifetime name ports items = description'
         (description', enumItems) = convertDescription' description
 convertDescription other = [other]

src/Convert/HierConst.hs

@@ -19,6 +19,7 @@
 
 module Convert.HierConst (convert) where
 
+import Control.Monad (when)
 import Data.Either (fromLeft)
 import qualified Data.Map.Strict as Map
 
@@ -86,6 +87,8 @@ traverseExprM (expr @ (Dot _ x)) = do
         (Just ([_, _], _, Left{}), Just ([_, _], _, Left{})) ->
             return $ Ident x
         (Just (accesses @ [Access _ Nil, _], _, Left False), _) -> do
+            details <- lookupElemM $ prefix x
+            when (details == Nothing) $
                 insertElem accesses (Left True)
             return $ Ident $ prefix x
         (Just ([Access _ Nil, _], _, Left True), _) ->

src/Convert/Package.hs

@@ -610,7 +610,8 @@ convertDescription pis (orig @ Part{}) =
         else Part attrs extern kw lifetime name ports items'
     where
         Part attrs extern kw lifetime name ports items = orig
-        items' = addItems pis Set.empty (map addUsedPIs items)
+        items' = addItems pis Set.empty $ map addUsedPIs $
+            foldr breakGenerate [] items -- ensure decls are visible
 convertDescription _ other = other
 
 -- attempt to fix simple declaration order issues

src/Convert/ParamType.hs

@@ -23,6 +23,7 @@ type Instance = Map.Map Identifier (Type, IdentSet)
 type Instances = Set.Set (Identifier, Instance)
 
 type IdentSet = Set.Set Identifier
+type DeclMap = Map.Map Identifier Decl
 type UsageMap = [(Identifier, Set.Set Identifier)]
 
 convert :: [AST] -> [AST]
@@ -241,20 +242,20 @@ typeIsUnresolved =
         collectUnresolvedExprM Dot    {} = tell $ Any True
         collectUnresolvedExprM _ = return ()
 
-prepareTypeExprs :: Identifier -> Identifier -> Type -> (Type, (IdentSet, [Decl]))
+prepareTypeExprs :: Identifier -> Identifier -> Type -> (Type, (IdentSet, DeclMap))
 prepareTypeExprs instanceName paramName =
     runWriter . traverseNestedTypesM
         (traverseTypeExprsM $ traverseNestedExprsM prepareExpr)
     where
-        prepareExpr :: Expr -> Writer (IdentSet, [Decl]) Expr
+        prepareExpr :: Expr -> Writer (IdentSet, DeclMap) Expr
         prepareExpr (e @ Call{}) = do
-            tell (Set.empty, [decl])
+            tell (Set.empty, Map.singleton x decl)
             prepareExpr $ Ident x
             where
                 decl = Param Localparam (TypeOf e) x e
                 x = instanceName ++ "_sv2v_pfunc_" ++ shortHash e
         prepareExpr (Ident x) = do
-            tell (Set.singleton x, [])
+            tell (Set.singleton x, Map.empty)
             return $ Ident $ paramName ++ '_' : x
         prepareExpr other = return other
 
@@ -301,9 +302,9 @@ convertModuleItemM (orig @ (Instance m bindings x r p)) =
         bindingsMap = Map.fromList bindings
         resolvedTypesWithDecls = Map.mapMaybeWithKey resolveType bindingsMap
         resolvedTypes = Map.map (\(a, (b, _)) -> (a, b)) resolvedTypesWithDecls
-        addedDecls = concatMap (snd . snd . snd) $
-            Map.toList resolvedTypesWithDecls
-        resolveType :: Identifier -> TypeOrExpr -> Maybe (Type, (IdentSet, [Decl]))
+        addedDecls = Map.elems $ Map.unions $ map (snd . snd) $
+            Map.elems resolvedTypesWithDecls
+        resolveType :: Identifier -> TypeOrExpr -> Maybe (Type, (IdentSet, DeclMap))
         resolveType _ Right{} = Nothing
         resolveType paramName (Left t) =
             Just $ prepareTypeExprs x paramName t

src/Convert/Traverse.hs

@@ -11,6 +11,7 @@ module Convert.Traverse
 , unmonad
 , collectify
 , mapBothM
+, breakGenerate
 , traverseDescriptionsM
 , traverseDescriptions
 , collectDescriptionsM
@@ -132,21 +133,21 @@ traverseDescriptions = map
 collectDescriptionsM :: Monad m => CollectorM m Description -> CollectorM m AST
 collectDescriptionsM = mapM_
 
-breakGenerate :: ModuleItem -> [ModuleItem]
-breakGenerate (Generate genItems) =
-    if all isGenModuleItem genItems
-        then map (\(GenModuleItem item) -> item) genItems
-        else [Generate genItems]
-    where
-        isGenModuleItem :: GenItem -> Bool
-        isGenModuleItem (GenModuleItem _) = True
-        isGenModuleItem _ = False
-breakGenerate other = [other]
+breakGenerate :: ModuleItem -> [ModuleItem] -> [ModuleItem]
+breakGenerate (Generate genItems) items =
+    foldr breakGenerateStep items genItems
+breakGenerate item items = item : items
+
+breakGenerateStep :: GenItem -> [ModuleItem] -> [ModuleItem]
+breakGenerateStep (GenModuleItem item) items = item : items
+breakGenerateStep genItem (Generate genItems : items) =
+    Generate (genItem : genItems) : items
+breakGenerateStep genItem items = Generate [genItem] : items
 
 traverseModuleItemsM :: Monad m => MapperM m ModuleItem -> MapperM m Description
 traverseModuleItemsM mapper (Part attrs extern kw lifetime name ports items) = do
     items' <- mapM (traverseNestedModuleItemsM mapper) items
-    let items'' = concatMap breakGenerate items'
+    let items'' = foldr breakGenerate [] items'
     return $ Part attrs extern kw lifetime name ports items''
     where
 traverseModuleItemsM mapper (PackageItem packageItem) = do
@@ -159,18 +160,18 @@ traverseModuleItemsM mapper (Package lifetime name items) = do
     let itemsWrapped = map MIPackageItem items
     itemsWrapped' <- mapM (traverseNestedModuleItemsM mapper) itemsWrapped
     let items' = map (\(MIPackageItem item) -> item) $
-                    concatMap breakGenerate itemsWrapped'
+                    foldr breakGenerate [] itemsWrapped'
     return $ Package lifetime name items'
 traverseModuleItemsM mapper (Class lifetime name decls items) = do
     let declsWrapped = map (MIPackageItem . Decl) decls
     declsWrapped' <- mapM (traverseNestedModuleItemsM mapper) declsWrapped
     let decls' = map (\(MIPackageItem (Decl decl)) -> decl) $
-                    concatMap breakGenerate declsWrapped'
+                    foldr breakGenerate [] declsWrapped'
     items' <- fmap concat $ mapM indirect items
     return $ Class lifetime name decls' items'
     where
         indirect (qualifier, item) =
-            fmap (map (unwrap qualifier) . breakGenerate) $
+            fmap (map (unwrap qualifier) . flip breakGenerate []) $
             traverseNestedModuleItemsM mapper (MIPackageItem item)
         unwrap qualifier = \(MIPackageItem item) -> (qualifier, item)
 

test/core/paramtype.sv

@@ -126,3 +126,4 @@ module p_6; parameter W = 2; p #(logic [$signed(W):0], logic [$signed(W):0]) x()
 module p_7; parameter W = 2; p #(logic [square(W):0], logic [square(W):0]) x(); endmodule
 module p_8; p #(OtherT) x(); endmodule
 module p_9; p #(SomeT, OtherT) x(); endmodule
+module p_A; if (1) begin : blk p #(SomeT, OtherT) x(); end endmodule

test/core/paramtype.v

@@ -54,5 +54,7 @@ module top;
         $display("p 1 00000000000000000000000000000010 1");
         $display("p 0000000000 00000000000000000000000000000001 10");
         $display("p 0000000001 00000000000000000000000000000010 10");
+        $display("p 0000000000 00000000000000000000000000000001 10");
+        $display("p 0000000001 00000000000000000000000000000010 10");
     end
 endmodule