Add error on mixing .name and by-port instantiations.

Changes

@@ -18,6 +18,7 @@ Verilator 5.007 devel
 * Add WIDTHEXPAND and WIDTHTRUNC warnings to replace WIDTH (#3900). [Andrew Nolte]
 * Add SOURCE_DATE_EPOCH for docs/guide/conf.py (#3918). [Larry Doolittle]
 * Add /*verilator public[flat|flat_rd|flat_rw| ]*/ metacomments (#3894). [Joseph Nwabueze]
+* Add error on mixing .name and by-port instantiations.
 * Support unpacked unions.
 * Support interface classes and class implements.
 * Support global clocking and $global_clock.

src/V3AstNodeOther.h

@@ -1233,7 +1233,8 @@ private:
     AstVar* m_modVarp = nullptr;  // Input/output this pin connects to on submodule.
     AstParamTypeDType* m_modPTypep = nullptr;  // Param type this pin connects to on submodule.
     bool m_param = false;  // Pin connects to parameter
-    bool m_svImplicit = false;  // Pin is SystemVerilog .name'ed
+    bool m_svDotName = false;  // Pin is SystemVerilog .name'ed
+    bool m_svImplicit = false;  // Pin is SystemVerilog .name'ed, allow implicit
 public:
     AstPin(FileLine* fl, int pinNum, const string& name, AstNode* exprp)
         : ASTGEN_SUPER_Pin(fl)
@@ -1257,6 +1258,8 @@ public:
     void modPTypep(AstParamTypeDType* nodep) { m_modPTypep = nodep; }
     bool param() const { return m_param; }
     void param(bool flag) { m_param = flag; }
+    bool svDotName() const { return m_svDotName; }
+    void svDotName(bool flag) { m_svDotName = flag; }
     bool svImplicit() const { return m_svImplicit; }
     void svImplicit(bool flag) { m_svImplicit = flag; }
 };

src/V3AstNodes.cpp

@@ -1724,6 +1724,7 @@ void AstPin::dump(std::ostream& str) const {
     } else {
         str << " ->UNLINKED";
     }
+    if (svDotName()) str << " [.n]";
     if (svImplicit()) str << " [.SV]";
 }
 const char* AstPin::broken() const {

src/V3LinkCells.cpp

@@ -352,8 +352,10 @@ private:
         }
         // Convert .* to list of pins
         bool pinStar = false;
+        bool pinDotName = false;
         for (AstPin *nextp, *pinp = nodep->pinsp(); pinp; pinp = nextp) {
             nextp = VN_AS(pinp->nextp(), Pin);
+            if (pinp->svDotName()) pinDotName = true;
             if (pinp->dotStar()) {
                 if (pinStar) pinp->v3error("Duplicate .* in an instance (IEEE 1800-2017 23.3.2)");
                 pinStar = true;
@@ -374,8 +376,8 @@ private:
             // Note what pins exist
             std::unordered_set<string> ports;  // Symbol table of all connected port names
             for (AstPin* pinp = nodep->pinsp(); pinp; pinp = VN_AS(pinp->nextp(), Pin)) {
-                if (pinStar && pinp->name().substr(0, 11) == "__pinNumber") {
+                if ((pinStar || pinDotName) && pinp->name().substr(0, 11) == "__pinNumber") {
-                    pinp->v3error("Connect by position is illegal in .* connected instances"
+                    pinp->v3error("Mixing positional and .*/named instantiation connection"
                                   " (IEEE 1800-2017 23.3.2)");
                 }
                 if (!pinp->exprp()) {
@@ -404,6 +406,7 @@ private:
                                 nodep->fileline(), 0, portp->name(),
                                 new AstParseRef{nodep->fileline(), VParseRefExp::PX_TEXT,
                                                 portp->name(), nullptr, nullptr}};
+                            newp->svDotName(true);
                             newp->svImplicit(true);
                             nodep->addPinsp(newp);
                         } else {  // warn on the CELL that needs it, not the port

src/verilog.y

@@ -3194,18 +3194,22 @@ cellparamItemE<pinp>:           // IEEE: named_parameter_assignment + empty
         //                      // Note empty can match either () or (,); V3LinkCells cleans up ()
                 /* empty: ',,' is legal */              { $$ = new AstPin{CRELINE(), PINNUMINC(), "", nullptr}; }
         |       yP_DOTSTAR                              { $$ = new AstPin{$1, PINNUMINC(), ".*", nullptr}; }
-        |       '.' idAny '(' ')'                       { $$ = new AstPin{$<fl>2, PINNUMINC(), *$2, nullptr}; }
+        |       '.' idAny '(' ')'
+                        { $$ = new AstPin{$<fl>2, PINNUMINC(), *$2, nullptr};
+                          $$->svDotName(true); }
         |       '.' idSVKwd
                         { $$ = new AstPin{$<fl>2, PINNUMINC(), *$2,
                                           new AstParseRef{$<fl>2, VParseRefExp::PX_TEXT, *$2, nullptr, nullptr}};
-                          $$->svImplicit(true); }
+                          $$->svDotName(true); $$->svImplicit(true); }
         |       '.' idAny
                         { $$ = new AstPin{$<fl>2, PINNUMINC(), *$2,
                                           new AstParseRef{$<fl>2, VParseRefExp::PX_TEXT, *$2, nullptr, nullptr}};
-                          $$->svImplicit(true); }
+                          $$->svDotName(true); $$->svImplicit(true); }
         //                      // mintypmax is expanded here, as it might be a UDP or gate primitive
         //                      // data_type for 'parameter type' hookups
-        |       '.' idAny '(' exprOrDataType ')'        { $$ = new AstPin{$<fl>2, PINNUMINC(), *$2, $4}; }
+        |       '.' idAny '(' exprOrDataType ')'
+                        { $$ = new AstPin{$<fl>2, PINNUMINC(), *$2, $4};
+                          $$->svDotName(true); }
         //UNSUP '.' idAny '(' exprOrDataType/*expr*/ ':' expr ')'               { }
         //UNSUP '.' idAny '(' exprOrDataType/*expr*/ ':' expr ':' expr ')'      { }
         //                      // data_type for 'parameter type' hookups
@@ -3218,18 +3222,22 @@ cellpinItemE<pinp>:             // IEEE: named_port_connection + empty
         //                      // Note empty can match either () or (,); V3LinkCells cleans up ()
                 /* empty: ',,' is legal */              { $$ = new AstPin{CRELINE(), PINNUMINC(), "", nullptr}; }
         |       yP_DOTSTAR                              { $$ = new AstPin{$1, PINNUMINC(), ".*", nullptr}; }
-        |       '.' idAny '(' ')'                       { $$ = new AstPin{$<fl>2, PINNUMINC(), *$2, nullptr}; }
+        |       '.' idAny '(' ')'
+                        { $$ = new AstPin{$<fl>2, PINNUMINC(), *$2, nullptr};
+                          $$->svDotName(true); }
         |       '.' idSVKwd
                         { $$ = new AstPin{$<fl>2, PINNUMINC(), *$2,
                                           new AstParseRef{$<fl>2, VParseRefExp::PX_TEXT, *$2, nullptr, nullptr}};
-                          $$->svImplicit(true);}
+                          $$->svDotName(true); $$->svImplicit(true); }
         |       '.' idAny
                         { $$ = new AstPin{$<fl>2, PINNUMINC(), *$2,
                                           new AstParseRef{$<fl>2, VParseRefExp::PX_TEXT, *$2, nullptr, nullptr}};
-                          $$->svImplicit(true);}
+                          $$->svDotName(true); $$->svImplicit(true); }
         //                      // mintypmax is expanded here, as it might be a UDP or gate primitive
         //UNSUP               pev_expr below
-        |       '.' idAny '(' expr ')'                  { $$ = new AstPin{$<fl>2, PINNUMINC(), *$2, $4}; }
+        |       '.' idAny '(' expr ')'
+                        { $$ = new AstPin{$<fl>2, PINNUMINC(), *$2, $4};
+                          $$->svDotName(true); }
         //UNSUP '.' idAny '(' pev_expr ':' expr ')'     { }
         //UNSUP '.' idAny '(' pev_expr ':' expr ':' expr ')' { }
         //

test_regress/t/t_fuzz_genintf_bad.out

@@ -1,10 +1,4 @@
-%Error-UNSUPPORTED: t/t_fuzz_genintf_bad.v:24:12: Unsupported: Member call on object 'VARREF 'j'' which is a 'BASICDTYPE 'integer''
+%Error: t/t_fuzz_genintf_bad.v:24:11: Mixing positional and .*/named instantiation connection (IEEE 1800-2017 23.3.2)
-                                                : ... In instance t
-   24 |          j.e(0),
-      |            ^
-                    ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest
-%Error: Internal Error: t/t_fuzz_genintf_bad.v:24:11: ../V3Width.cpp:#: Unlinked pin data type
-                                                    : ... In instance t
    24 |          j.e(0),
       |           ^
-                        ... See the manual at https://verilator.org/verilator_doc.html for more assistance.
+%Error: Exiting due to

test_regress/t/t_inst_2star_bad.out

@@ -1,7 +1,10 @@
-%Error: t/t_inst_2star_bad.v:11:17: Duplicate .* in an instance (IEEE 1800-2017 23.3.2)
+%Error: t/t_inst_2star_bad.v:12:17: Duplicate .* in an instance (IEEE 1800-2017 23.3.2)
-   11 |    sub sub (.*, .*);
+   12 |    sub sub (.*, .*);
       |                 ^~
-%Error: t/t_inst_2star_bad.v:13:13: Connect by position is illegal in .* connected instances (IEEE 1800-2017 23.3.2)
+%Error: t/t_inst_2star_bad.v:14:13: Mixing positional and .*/named instantiation connection (IEEE 1800-2017 23.3.2)
-   13 |    sub sub (foo, .*);
+   14 |    sub sub (foo, .*);
+      |             ^~~
+%Error: t/t_inst_2star_bad.v:16:13: Mixing positional and .*/named instantiation connection (IEEE 1800-2017 23.3.2)
+   16 |    sub sub (foo, .bar);
       |             ^~~
 %Error: Exiting due to

test_regress/t/t_inst_2star_bad.v

@@ -7,12 +7,15 @@
 module t (/*AUTOARG*/);
 
    wire foo;
+   wire bar;
 
    sub sub (.*, .*);
 
    sub sub (foo, .*);
 
+   sub sub (foo, .bar);
+
 endmodule
 
-module sub (input foo);
+module sub (input foo, input bar);
 endmodule

test_regress/t/t_inst_missing.v

@@ -6,9 +6,13 @@
 
 module t (/*AUTOARG*/);
    wire ok = 1'b0;
+   // verilator lint_off UNDRIVEN
+   wire nc;
+   // verilator lint_on UNDRIVEN
+
    // verilator lint_off PINNOCONNECT
    // verilator lint_off PINCONNECTEMPTY
-   sub sub (.ok(ok), , .nc());
+   sub sub (ok, , nc);
    // verilator lint_on PINCONNECTEMPTY
    // verilator lint_on PINNOCONNECT
 endmodule

test_regress/t/t_inst_missing_bad.out

@@ -1,12 +1,9 @@
-%Warning-PINNOCONNECT: t/t_inst_missing_bad.v:9:22: Cell pin is not connected: '__pinNumber2'
+%Warning-PINNOCONNECT: t/t_inst_missing_bad.v:13:17: Cell pin is not connected: '__pinNumber2'
-    9 |    sub sub (.ok(ok), , .nc());
+   13 |    sub sub (ok, , nc);
-      |                      ^
+      |                 ^
                        ... For warning description see https://verilator.org/warn/PINNOCONNECT?v=latest
                        ... Use "/* verilator lint_off PINNOCONNECT */" and lint_on around source to disable this message.
-%Warning-PINCONNECTEMPTY: t/t_inst_missing_bad.v:9:25: Cell pin connected by name with empty reference: 'nc'
+%Warning-PINMISSING: t/t_inst_missing_bad.v:13:8: Cell has missing pin: 'missing'
-    9 |    sub sub (.ok(ok), , .nc());
+   13 |    sub sub (ok, , nc);
-      |                         ^~
-%Warning-PINMISSING: t/t_inst_missing_bad.v:9:8: Cell has missing pin: 'missing'
-    9 |    sub sub (.ok(ok), , .nc());
       |        ^~~
 %Error: Exiting due to

test_regress/t/t_inst_missing_bad.v

@@ -6,7 +6,11 @@
 
 module t (/*AUTOARG*/);
    wire ok = 1'b0;
-   sub sub (.ok(ok), , .nc());
+   // verilator lint_off UNDRIVEN
+   wire nc;
+   // verilator lint_on UNDRIVEN
+
+   sub sub (ok, , nc);
 endmodule
 
 module sub (input ok, input none, input nc, input missing);

test_regress/t/t_interface_modportlist.v

@@ -8,7 +8,7 @@
 
 module t(input clk);
    my_interface iface();
-   my_module m(.clk(clk), iface);
+   my_module m(.clk(clk), .iface);
 endmodule
 
 module my_module(input clk, my_interface.my_port iface);