Internals: Do data type elaboration always as child node, before movement to symbol table.

src/V3Ast.h

@@ -1809,6 +1809,8 @@ public:
     virtual bool hasDType() const { return false; }
     // Iff has a non-null childDTypep(), as generic node function
     virtual AstNodeDType* getChildDTypep() const { return NULL; }
+    // Iff has a non-null child2DTypep(), as generic node function
+    virtual AstNodeDType* getChild2DTypep() const { return NULL; }
     // Another AstNode* may have a pointer into this node, other then normal front/back/etc.
     virtual bool maybePointedTo() const { return false; }
     virtual const char* broken() const { return NULL; }

src/V3AstNodes.h

@@ -541,8 +541,9 @@ public:
     virtual string prettyDTypeName() const;
     virtual void dumpSmall(std::ostream& str) const;
     virtual V3Hash sameHash() const { return V3Hash(m_refDTypep); }
+    virtual AstNodeDType* getChildDTypep() const { return childDTypep(); }
+    virtual AstNodeDType* getChild2DTypep() const { return keyChildDTypep(); }
     virtual bool isHeavy() const { return true; }
-    AstNodeDType* getChildDTypep() const { return childDTypep(); }
     // op1 = Range of variable
     AstNodeDType* childDTypep() const { return VN_CAST(op1p(), NodeDType); }
     void childDTypep(AstNodeDType* nodep) { setOp1p(nodep); }
@@ -5619,6 +5620,7 @@ public:
     AstNode* lhsp() const { return op1p(); }
     AstNodeDType* getChildDTypep() const { return childDTypep(); }
     AstNodeDType* childDTypep() const { return VN_CAST(op2p(), NodeDType); }
+    virtual AstNodeDType* subDTypep() const { return dtypep() ? dtypep() : childDTypep(); }
 };
 
 class AstCastDynamic : public AstNodeBiop {

src/V3Width.cpp

@@ -1332,9 +1332,8 @@ private:
     // DTYPES
     virtual void visit(AstNodeArrayDType* nodep) VL_OVERRIDE {
         if (nodep->didWidthAndSet()) return;  // This node is a dtype & not both PRELIMed+FINALed
-        if (nodep->childDTypep()) nodep->refDTypep(moveChildDTypeEdit(nodep));
         // Iterate into subDTypep() to resolve that type and update pointer.
-        nodep->refDTypep(iterateEditDTypep(nodep, nodep->subDTypep()));
+        nodep->refDTypep(iterateEditMoveDTypep(nodep, nodep->subDTypep()));
         // Cleanup array size
         userIterateAndNext(nodep->rangep(), WidthVP(SELF, BOTH).p());
         nodep->dtypep(nodep);  // The array itself, not subDtype
@@ -1349,29 +1348,23 @@ private:
     }
     virtual void visit(AstAssocArrayDType* nodep) VL_OVERRIDE {
         if (nodep->didWidthAndSet()) return;  // This node is a dtype & not both PRELIMed+FINALed
-        if (nodep->childDTypep()) nodep->refDTypep(moveChildDTypeEdit(nodep));
-        if (nodep->keyChildDTypep()) {
-            nodep->keyDTypep(moveDTypeEdit(nodep, nodep->keyChildDTypep()));
-        }
         // Iterate into subDTypep() to resolve that type and update pointer.
-        nodep->refDTypep(iterateEditDTypep(nodep, nodep->subDTypep()));
-        nodep->keyDTypep(iterateEditDTypep(nodep, nodep->keyDTypep()));
+        nodep->refDTypep(iterateEditMoveDTypep(nodep, nodep->subDTypep()));
+        nodep->keyDTypep(iterateEditMoveDTypep(nodep, nodep->keyDTypep()));
         nodep->dtypep(nodep);  // The array itself, not subDtype
         UINFO(4, "dtWidthed " << nodep << endl);
     }
     virtual void visit(AstDynArrayDType* nodep) VL_OVERRIDE {
         if (nodep->didWidthAndSet()) return;  // This node is a dtype & not both PRELIMed+FINALed
-        if (nodep->childDTypep()) nodep->refDTypep(moveChildDTypeEdit(nodep));
         // Iterate into subDTypep() to resolve that type and update pointer.
-        nodep->refDTypep(iterateEditDTypep(nodep, nodep->subDTypep()));
+        nodep->refDTypep(iterateEditMoveDTypep(nodep, nodep->subDTypep()));
         nodep->dtypep(nodep);  // The array itself, not subDtype
         UINFO(4, "dtWidthed " << nodep << endl);
     }
     virtual void visit(AstQueueDType* nodep) VL_OVERRIDE {
         if (nodep->didWidthAndSet()) return;  // This node is a dtype & not both PRELIMed+FINALed
-        if (nodep->childDTypep()) nodep->refDTypep(moveChildDTypeEdit(nodep));
         // Iterate into subDTypep() to resolve that type and update pointer.
-        nodep->refDTypep(iterateEditDTypep(nodep, nodep->subDTypep()));
+        nodep->refDTypep(iterateEditMoveDTypep(nodep, nodep->subDTypep()));
         nodep->dtypep(nodep);  // The array itself, not subDtype
         if (VN_IS(nodep->boundp(), Unbounded)) {
             nodep->boundp()->unlinkFrBack()->deleteTree();  // NULL will represent unbounded
@@ -1380,9 +1373,8 @@ private:
     }
     virtual void visit(AstUnsizedArrayDType* nodep) VL_OVERRIDE {
         if (nodep->didWidthAndSet()) return;  // This node is a dtype & not both PRELIMed+FINALed
-        if (nodep->childDTypep()) nodep->refDTypep(moveChildDTypeEdit(nodep));
         // Iterate into subDTypep() to resolve that type and update pointer.
-        nodep->refDTypep(iterateEditDTypep(nodep, nodep->subDTypep()));
+        nodep->refDTypep(iterateEditMoveDTypep(nodep, nodep->subDTypep()));
         // Cleanup array size
         nodep->dtypep(nodep);  // The array itself, not subDtype
         UINFO(4, "dtWidthed " << nodep << endl);
@@ -1412,12 +1404,8 @@ private:
     }
     virtual void visit(AstConstDType* nodep) VL_OVERRIDE {
         if (nodep->didWidthAndSet()) return;  // This node is a dtype & not both PRELIMed+FINALed
-        // Move any childDTypep to instead be in global AstTypeTable.
-        // This way if this node gets deleted and some other dtype points to it
-        // it won't become a dead pointer.  This doesn't change the object pointed to.
-        if (nodep->childDTypep()) nodep->refDTypep(moveChildDTypeEdit(nodep));
         // Iterate into subDTypep() to resolve that type and update pointer.
-        nodep->refDTypep(iterateEditDTypep(nodep, nodep->subDTypep()));
+        nodep->refDTypep(iterateEditMoveDTypep(nodep, nodep->subDTypep()));
         userIterateChildren(nodep, NULL);
         nodep->dtypep(nodep);  // Should already be set, but be clear it's not the subDType
         nodep->widthFromSub(nodep->subDTypep());
@@ -1444,8 +1432,10 @@ private:
         }
         userIterateChildren(nodep, NULL);
         if (nodep->subDTypep()) {
-            nodep->refDTypep(iterateEditDTypep(nodep, nodep->subDTypep()));
+            nodep->refDTypep(iterateEditMoveDTypep(nodep, nodep->subDTypep()));
             nodep->typedefp(NULL);  // Note until line above subDTypep() may have followed this
+            // Widths are resolved, but special iterate to check for recurstion
+            userIterate(nodep->subDTypep(), NULL);
         }
         // Effectively nodep->dtypeFrom(nodep->dtypeSkipRefp());
         // But might be recursive, so instead manually recurse into the referenced type
@@ -1457,15 +1447,13 @@ private:
     }
     virtual void visit(AstTypedef* nodep) VL_OVERRIDE {
         if (nodep->didWidthAndSet()) return;  // This node is a dtype & not both PRELIMed+FINALed
-        if (nodep->childDTypep()) nodep->dtypep(moveChildDTypeEdit(nodep));
+        nodep->dtypep(iterateEditMoveDTypep(nodep, nodep->subDTypep()));
         userIterateChildren(nodep, NULL);
-        nodep->dtypep(iterateEditDTypep(nodep, nodep->subDTypep()));
     }
     virtual void visit(AstParamTypeDType* nodep) VL_OVERRIDE {
         if (nodep->didWidthAndSet()) return;  // This node is a dtype & not both PRELIMed+FINALed
-        if (nodep->childDTypep()) nodep->dtypep(moveChildDTypeEdit(nodep));
+        nodep->dtypep(iterateEditMoveDTypep(nodep, nodep->subDTypep()));
         userIterateChildren(nodep, NULL);
-        nodep->dtypep(iterateEditDTypep(nodep, nodep->subDTypep()));
         nodep->widthFromSub(nodep->subDTypep());
     }
     virtual void visit(AstCastDynamic* nodep) VL_OVERRIDE {
@@ -1494,8 +1482,7 @@ private:
         }
     }
     virtual void visit(AstCast* nodep) VL_OVERRIDE {
-        if (nodep->childDTypep()) nodep->dtypep(moveChildDTypeEdit(nodep));
-        nodep->dtypep(iterateEditDTypep(nodep, nodep->dtypep()));
+        nodep->dtypep(iterateEditMoveDTypep(nodep, nodep->subDTypep()));
         // if (debug()) nodep->dumpTree(cout, "  CastPre: ");
         userIterateAndNext(nodep->lhsp(), WidthVP(SELF, BOTH).p());
         // When more general casts are supported, the cast elimination will be done later.
@@ -1602,8 +1589,7 @@ private:
         }
         nodep->doingWidth(true);
         // Make sure dtype is sized
-        if (nodep->childDTypep()) nodep->dtypep(moveChildDTypeEdit(nodep));
-        nodep->dtypep(iterateEditDTypep(nodep, nodep->dtypep()));
+        nodep->dtypep(iterateEditMoveDTypep(nodep, nodep->subDTypep()));
         UASSERT_OBJ(nodep->dtypep(), nodep, "No dtype determined for var");
         if (AstUnsizedArrayDType* unsizedp = VN_CAST(nodep->dtypeSkipRefp(), UnsizedArrayDType)) {
             if (!(m_ftaskp && m_ftaskp->dpiImport())) {
@@ -1733,8 +1719,7 @@ private:
     virtual void visit(AstEnumDType* nodep) VL_OVERRIDE {
         if (nodep->didWidthAndSet()) return;  // This node is a dtype & not both PRELIMed+FINALed
         UINFO(5, "  ENUMDTYPE " << nodep << endl);
-        if (nodep->childDTypep()) nodep->refDTypep(moveChildDTypeEdit(nodep));
-        nodep->refDTypep(iterateEditDTypep(nodep, nodep->subDTypep()));
+        nodep->refDTypep(iterateEditMoveDTypep(nodep, nodep->subDTypep()));
         nodep->dtypep(nodep);
         nodep->widthFromSub(nodep->subDTypep());
         // Assign widths
@@ -1937,20 +1922,23 @@ private:
         userIterateChildren(nodep, NULL);  // First size all members
         nodep->repairCache();
     }
+    virtual void visit(AstClassRefDType* nodep) VL_OVERRIDE {
+        if (nodep->didWidthAndSet()) return;
+        // TODO this maybe eventually required to properly resolve members,
+        // though causes problems with t_class_forward.v, so for now avoided
+        // userIterateChildren(nodep->classp(), NULL);
+    }
     virtual void visit(AstClassExtends* nodep) VL_OVERRIDE {
         if (nodep->didWidthAndSet()) return;
         nodep->v3error("Unsupported: class extends");  // Member/meth access breaks
         VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep);
-        // if (nodep->childDTypep()) {
-        //     nodep->dtypep(moveChildDTypeEdit(nodep));  // data_type '{ pattern }
-        // }
+        // nodep->dtypep(iterateEditMoveDTypep(nodep));  // data_type '{ pattern }
         // userIterateChildren(nodep, NULL);
     }
     virtual void visit(AstMemberDType* nodep) VL_OVERRIDE {
         if (nodep->didWidthAndSet()) return;  // This node is a dtype & not both PRELIMed+FINALed
-        if (nodep->childDTypep()) nodep->refDTypep(moveChildDTypeEdit(nodep));
         // Iterate into subDTypep() to resolve that type and update pointer.
-        nodep->refDTypep(iterateEditDTypep(nodep, nodep->subDTypep()));
+        nodep->refDTypep(iterateEditMoveDTypep(nodep, nodep->subDTypep()));
         nodep->dtypep(nodep);  // The member itself, not subDtype
         nodep->widthFromSub(nodep->subDTypep());
     }
@@ -2680,8 +2668,9 @@ private:
     virtual void visit(AstPattern* nodep) VL_OVERRIDE {
         if (nodep->didWidthAndSet()) return;
         UINFO(9, "PATTERN " << nodep << endl);
-        if (nodep->childDTypep())
-            nodep->dtypep(moveChildDTypeEdit(nodep));  // data_type '{ pattern }
+        if (nodep->childDTypep()) {  // data_type '{ pattern }
+            nodep->dtypep(iterateEditMoveDTypep(nodep, nodep->subDTypep()));
+        }
         if (!nodep->dtypep() && m_vup->dtypeNullp()) {  // Get it from parent assignment/pin/etc
             nodep->dtypep(m_vup->dtypep());
         }
@@ -4957,32 +4946,45 @@ private:
     //----------------------------------------------------------------------
     // METHODS - data types
 
-    AstNodeDType* moveDTypeEdit(AstNode* nodep, AstNodeDType* dtnodep) {
-        // DTypes at parse time get added as a e.g. childDType to some node types such as AstVars.
-        // Move type to global scope, so removing/changing a variable won't lose the dtype.
-        UASSERT_OBJ(dtnodep, nodep, "Caller should check for NULL before calling moveDTypeEdit");
-        UINFO(9, "moveDTypeEdit  " << dtnodep << endl);
-        dtnodep->unlinkFrBack();  // Make non-child
-        v3Global.rootp()->typeTablep()->addTypesp(dtnodep);
+    AstNodeDType* iterateEditMoveDTypep(AstNode* parentp, AstNodeDType* dtnodep) {
+        UASSERT_OBJ(dtnodep, parentp, "Caller should check for NULL before computing dtype");
+        // Iterate into a data type to resolve that type.
+        // The data type may either:
+        // 1. Be a child (typically getChildDTypep() returns it)
+        //    DTypes at parse time get added as these to some node types
+        //    such as AstVars.
+        //    This function will move it to global scope (that is #2
+        //    will now apply).
+        // 2. Be under the Netlist and pointed to by an Ast member variable
+        //    (typically refDTypep() or dtypep() returns it)
+        //    so removing/changing a variable won't lose the dtype
+
+        // Case #1 above applies?
+        bool child1 = (parentp->getChildDTypep() == dtnodep);
+        bool child2 = (parentp->getChild2DTypep() == dtnodep);
+        if (child1 || child2) {
+            UINFO(9, "iterateEditMoveDTypep child iterating " << dtnodep << endl);
+            // Iterate, this might edit the dtypes which means dtnodep now lost
+            VL_DO_DANGLING(userIterate(dtnodep, NULL), dtnodep);
+            // Figure out the new dtnodep, remained a child of parent so find it there
+            dtnodep = child1 ? parentp->getChildDTypep() : parentp->getChild2DTypep();
+            UASSERT_OBJ(dtnodep, parentp, "iterateEditMoveDTypep lost pointer to child");
+            UASSERT_OBJ(dtnodep->didWidth(), parentp,
+                        "iterateEditMoveDTypep didn't get width resolution of "
+                            << dtnodep->prettyTypeName());
+            // Move to under netlist
+            UINFO(9, "iterateEditMoveDTypep child moving " << dtnodep << endl);
+            dtnodep->unlinkFrBack();
+            v3Global.rootp()->typeTablep()->addTypesp(dtnodep);
+        }
+        if (!dtnodep->didWidth()) {
+            UINFO(9, "iterateEditMoveDTypep pointer iterating " << dtnodep << endl);
+            userIterate(dtnodep, NULL);
+            UASSERT_OBJ(dtnodep->didWidth(), parentp,
+                        "iterateEditMoveDTypep didn't get width resolution");
+        }
         return dtnodep;
     }
-    AstNodeDType* moveChildDTypeEdit(AstNode* nodep) {
-        return moveDTypeEdit(nodep, nodep->getChildDTypep());
-    }
-    AstNodeDType* iterateEditDTypep(AstNode* parentp, AstNodeDType* nodep) {
-        // Iterate into a data type to resolve that type. This process
-        // may eventually create a new data type, but not today
-        // it may make a new datatype, need subChildDType() to point to it;
-        // maybe we have user5p indicate a "replace me with" pointer.
-        // Need to be careful with "implicit" types though.
-        //
-        // Alternative is to have WidthVP return information.
-        // or have a call outside of normal visitor land.
-        // or have a m_return type (but need to return if width called multiple times)
-        UASSERT_OBJ(nodep, parentp, "Null dtype when widthing dtype");
-        userIterate(nodep, NULL);
-        return nodep;
-    }
 
     AstConst* dimensionValue(FileLine* fileline, AstNodeDType* nodep, AstAttrType attrType,
                              int dim) {

test_regress/t/t_metacmt_onoff.out

@@ -1,8 +1,10 @@
 %Warning-LITENDIAN: t/t_metacmt_onoff.v:8:8: Little bit endian vector: MSB < LSB of bit range: 0:1
+                                           : ... In instance t
     8 |    reg [0:1] show1; /*verilator lint_off LITENDIAN*/  reg [0:2] ign2; /*verilator lint_on LITENDIAN*/   reg [0:3] show3;
       |        ^
                     ... Use "/* verilator lint_off LITENDIAN */" and lint_on around source to disable this message.
 %Warning-LITENDIAN: t/t_metacmt_onoff.v:8:109: Little bit endian vector: MSB < LSB of bit range: 0:3
+                                             : ... In instance t
     8 |    reg [0:1] show1; /*verilator lint_off LITENDIAN*/  reg [0:2] ign2; /*verilator lint_on LITENDIAN*/   reg [0:3] show3;
       |                                                                                                             ^
 %Error: Exiting due to

test_regress/t/t_select_bad_msb.out

@@ -1,4 +1,5 @@
 %Warning-LITENDIAN: t/t_select_bad_msb.v:12:8: Little bit endian vector: MSB < LSB of bit range: 0:22
+                                             : ... In instance t
    12 |    reg [0:22] backwd;
       |        ^
                     ... Use "/* verilator lint_off LITENDIAN */" and lint_on around source to disable this message.

test_regress/t/t_struct_unpacked2.out

@@ -3,6 +3,7 @@
       |             ^
                    ... Use "/* verilator lint_off UNPACKED */" and lint_on around source to disable this message.
 %Warning-UNPACKED: t/t_struct_unpacked2.v:9:12: Unsupported: Unpacked struct/union
+                                              : ... In instance x
     9 |    typedef struct {
       |            ^~~~~~
 %Error: Exiting due to

test_regress/t/t_struct_unpacked_bad.out

@@ -1,4 +1,5 @@
 %Warning-UNPACKED: t/t_struct_unpacked_bad.v:9:12: Unsupported: Unpacked struct/union
+                                                 : ... In instance x
     9 |    typedef struct {
       |            ^~~~~~
                    ... Use "/* verilator lint_off UNPACKED */" and lint_on around source to disable this message.

test_regress/t/t_xml_tag.out

@@ -58,24 +58,24 @@
       <basicdtype fl="d31" loc="d,31,26,31,27" id="5" name="logic" left="31" right="0"/>
       <basicdtype fl="d8" loc="d,8,4,8,11" id="6" name="integer" left="31" right="0"/>
       <basicdtype fl="d14" loc="d,14,11,14,17" id="1" name="logic"/>
+      <basicdtype fl="d21" loc="d,21,7,21,12" id="8" name="logic"/>
+      <basicdtype fl="d22" loc="d,22,7,22,12" id="9" name="logic"/>
+      <basicdtype fl="d23" loc="d,23,7,23,12" id="10" name="logic"/>
+      <basicdtype fl="d24" loc="d,24,7,24,12" id="11" name="logic"/>
       <structdtype fl="d20" loc="d,20,12,20,18" id="2" name="m.my_struct">
-        <memberdtype fl="d21" loc="d,21,16,21,19" id="8" name="clk" tag="this is clk" sub_dtype_id="9"/>
-        <memberdtype fl="d22" loc="d,22,16,22,17" id="10" name="k" sub_dtype_id="11"/>
-        <memberdtype fl="d23" loc="d,23,16,23,22" id="12" name="enable" tag="enable" sub_dtype_id="13"/>
-        <memberdtype fl="d24" loc="d,24,16,24,20" id="14" name="data" tag="data" sub_dtype_id="15"/>
+        <memberdtype fl="d21" loc="d,21,16,21,19" id="12" name="clk" tag="this is clk" sub_dtype_id="8"/>
+        <memberdtype fl="d22" loc="d,22,16,22,17" id="13" name="k" sub_dtype_id="9"/>
+        <memberdtype fl="d23" loc="d,23,16,23,22" id="14" name="enable" tag="enable" sub_dtype_id="10"/>
+        <memberdtype fl="d24" loc="d,24,16,24,20" id="15" name="data" tag="data" sub_dtype_id="11"/>
       </structdtype>
-      <basicdtype fl="d21" loc="d,21,7,21,12" id="9" name="logic"/>
-      <basicdtype fl="d22" loc="d,22,7,22,12" id="11" name="logic"/>
-      <basicdtype fl="d23" loc="d,23,7,23,12" id="13" name="logic"/>
-      <basicdtype fl="d24" loc="d,24,7,24,12" id="15" name="logic"/>
       <ifacerefdtype fl="d29" loc="d,29,8,29,12" id="3" modportname=""/>
+      <refdtype fl="d31" loc="d,31,4,31,13" id="16" name="my_struct" sub_dtype_id="2"/>
       <unpackarraydtype fl="d31" loc="d,31,26,31,27" id="4" sub_dtype_id="2">
         <range fl="d31" loc="d,31,26,31,27">
           <const fl="d31" loc="d,31,26,31,27" name="32&apos;h1" dtype_id="5"/>
           <const fl="d31" loc="d,31,26,31,27" name="32&apos;h0" dtype_id="5"/>
         </range>
       </unpackarraydtype>
-      <refdtype fl="d31" loc="d,31,4,31,13" id="16" name="my_struct" sub_dtype_id="2"/>
       <basicdtype fl="d35" loc="d,35,21,35,27" id="7" name="string"/>
     </typetable>
   </netlist>