Fix missing parameters with comma to be errors (#4979) (#5012).

Changes

@@ -15,6 +15,7 @@ Verilator 5.025 devel
 
 * Support __en/__out signals on top level inout ports (#4812) (#4856). [Paul Wright]
 * Add CITATION.cff (#5057) (#5058). [Gijs Burghoorn]
+* Fix missing parameters with comma to be errors (#4979) (#5012). [Paul Swirhun]
 * Fix consecutive zero-delays (#5038). [Krzysztof Bieganski, Antmicro Ltd.]
 * Fix `$system` with string argument (#5042).
 * Fix width extension on delays (#5043).

src/verilog.y

@@ -3264,8 +3264,14 @@ cellpinListE<pinp>:
         ;
 
 cellparamItListE<pinp>:         // IEEE: list_of_parameter_value_assignments/list_of_parameter_assignments
-                cellparamItemE                          { $$ = $1; }
+        //                      // Empty gets a node, to track class reference of #()
-        |       cellparamItListE ',' cellparamItemE     { $$ = addNextNull($1, $3); }
+                /*empty*/                               { $$ = new AstPin{CRELINE(), PINNUMINC(), "", nullptr}; }
+        |       cellparamItList                         { $$ = $1; }
+        ;
+
+cellparamItList<pinp>:          // IEEE: list_of_parameter_value_assignments/list_of_parameter_assignments
+                cellparamItem                           { $$ = $1; }
+        |       cellparamItList ',' cellparamItem       { $$ = addNextNull($1, $3); }
         ;
 
 cellpinItListE<pinp>:           // IEEE: list_of_port_connections
@@ -3273,10 +3279,9 @@ cellpinItListE<pinp>:           // IEEE: list_of_port_connections
         |       cellpinItListE ',' cellpinItemE         { $$ = addNextNull($1, $3); }
         ;
 
-cellparamItemE<pinp>:           // IEEE: named_parameter_assignment + empty
+cellparamItem<pinp>:            // IEEE: named_parameter_assignment + empty
-        //                      // Note empty can match either () or (,); V3LinkCells cleans up ()
+        //                      // Note empty is not allowed in parameter lists
-                /* empty: ',,' is legal */              { $$ = new AstPin{CRELINE(), PINNUMINC(), "", nullptr}; }
+                yP_DOTSTAR                              { $$ = new AstPin{$1, PINNUMINC(), ".*", nullptr}; }
-        |       yP_DOTSTAR                              { $$ = new AstPin{$1, PINNUMINC(), ".*", nullptr}; }
         |       '.' idAny '(' ')'
                         { $$ = new AstPin{$<fl>2, PINNUMINC(), *$2, nullptr};
                           $$->svDotName(true); }

test_regress/t/t_class_param_comma_bad.out

@@ -0,0 +1,16 @@
+%Error: t/t_class_param_comma_bad.v:16:22: syntax error, unexpected ')', expecting TYPE-IDENTIFIER
+   16 |    Cls #(.PARAMB(14),) ce;   
+      |                      ^
+%Error: t/t_class_param_comma_bad.v:17:13: syntax error, unexpected ')', expecting TYPE-IDENTIFIER
+   17 |    Cls #(14,) cf;   
+      |             ^
+%Error: t/t_class_param_comma_bad.v:18:14: syntax error, unexpected ')', expecting TYPE-IDENTIFIER
+   18 |    Cls2 #(15,) cg;   
+      |              ^
+%Error: t/t_class_param_comma_bad.v:19:23: syntax error, unexpected ')', expecting TYPE-IDENTIFIER
+   19 |    Cls2 #(.PARAMB(16),) ch;   
+      |                       ^
+%Error: t/t_class_param_comma_bad.v:20:23: syntax error, unexpected ')', expecting TYPE-IDENTIFIER
+   20 |    Cls2 #(.PARAMC(17),) ci;   
+      |                       ^
+%Error: Exiting due to

test_regress/t/t_class_param_comma_bad.pl

@@ -0,0 +1,19 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2020 by Wilson Snyder. This program is free software; you
+# can redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+scenarios(linter => 1);
+
+lint(
+    fails => 1,
+    expect_filename => $Self->{golden_filename},
+    );
+
+ok(1);
+1;

test_regress/t/t_class_param_comma_bad.v

@@ -0,0 +1,22 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2024 by Wilson Snyder.
+// SPDX-License-Identifier: CC0-1.0
+
+class Cls #(parameter PARAMB = 12);
+endclass
+class Cls2 #(parameter PARAMB = 13, parameter PARAMC = 14);
+endclass
+
+module t (/*AUTOARG*/);
+
+   Cls #(.PARAMBAD(1)) c;  // Bad param name
+   Cls #(13, 1) cd;  // Bad param number
+   Cls #(.PARAMB(14),) ce;  // Bad superfluous comma
+   Cls #(14,) cf;  // Bad superfluous comma
+   Cls2 #(15,) cg;  // Bad superfluous comma
+   Cls2 #(.PARAMB(16),) ch;  // Bad superfluous comma
+   Cls2 #(.PARAMC(17),) ci;  // Bad superfluous comma
+
+endmodule

test_regress/t/t_inst_param_comma_bad.out

@@ -0,0 +1,19 @@
+%Error: t/t_inst_param_comma_bad.v:35:15: syntax error, unexpected ')', expecting TYPE-IDENTIFIER
+   35 |    M #(.P(13),) m1(
+      |               ^
+%Error: t/t_inst_param_comma_bad.v:40:11: syntax error, unexpected ')', expecting TYPE-IDENTIFIER
+   40 |    M #(14,) m2 (
+      |           ^
+%Error: t/t_inst_param_comma_bad.v:45:11: syntax error, unexpected ')', expecting TYPE-IDENTIFIER
+   45 |    M #(14,) m3 (
+      |           ^
+%Error: t/t_inst_param_comma_bad.v:50:15: syntax error, unexpected ')', expecting TYPE-IDENTIFIER
+   50 |    N #(.P(13),) n1(
+      |               ^
+%Error: t/t_inst_param_comma_bad.v:55:11: syntax error, unexpected ')', expecting TYPE-IDENTIFIER
+   55 |    N #(14,) n2 (
+      |           ^
+%Error: t/t_inst_param_comma_bad.v:60:11: syntax error, unexpected ')', expecting TYPE-IDENTIFIER
+   60 |    N #(14,) n3 (
+      |           ^
+%Error: Exiting due to

test_regress/t/t_inst_param_comma_bad.pl

@@ -0,0 +1,19 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2023 by Wilson Snyder. This program is free software; you
+# can redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+scenarios(linter => 1);
+
+lint(
+    fails => 1,
+    expect_filename => $Self->{golden_filename},
+    );
+
+ok(1);
+1;

test_regress/t/t_inst_param_comma_bad.v

@@ -0,0 +1,65 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2024 by Wilson Snyder.
+// SPDX-License-Identifier: CC0-1.0
+
+module M #(
+    parameter int P = 12,
+    parameter int Q = 13
+    ) (
+    input wire i,
+    output wire o
+    );
+    assign o = i;
+endmodule
+
+module N #(
+    parameter int P = 12
+    ) (
+    input wire i,
+    output wire o
+    );
+    assign o = i;
+endmodule
+
+module t (/*AUTOARG*/);
+
+  wire i1, o1, i2, o2, i3, o3, i4, o4, i5, o5, i6, o6;
+
+   // All of these have superfluous commas after the first parameter.
+   // All of the N instances produced a PINNOTFOUND error, however as reported in issue #4979,
+   // none of the M instances do when they should. The copmma after the first parameter is not
+   // allowed in verilog.
+
+   M #(.P(13),) m1(
+    .i(i1),
+    .o(o1)
+   );
+
+   M #(14,) m2 (
+    .i(i2),
+    .o(o2)
+    );
+
+   M #(14,) m3 (
+    .i(i3),
+    .o(o3)
+    );
+
+   N #(.P(13),) n1(
+    .i(i4),
+    .o(o4)
+   );
+
+   N #(14,) n2 (
+    .i(i5),
+    .o(o5)
+    );
+
+   N #(14,) n3 (
+    .i(i6),
+    .o(o6)
+    );
+
+endmodule

test_regress/t/t_lint_pindup_bad.out

@@ -1,32 +1,26 @@
-%Warning-PINMISSING: t/t_lint_pindup_bad.v:19:4: Cell has missing pin: 'exists'
+%Warning-PINMISSING: t/t_lint_pindup_bad.v:18:4: Cell has missing pin: 'exists'
-   19 |    sub (.o(o),
+   18 |    sub (.o(o),
       |    ^~~
                      ... For warning description see https://verilator.org/warn/PINMISSING?v=latest
                      ... Use "/* verilator lint_off PINMISSING */" and lint_on around source to disable this message.
-%Error: t/t_lint_pindup_bad.v:21:10: Duplicate pin connection: 'i'
+%Error: t/t_lint_pindup_bad.v:20:10: Duplicate pin connection: 'i'
-   21 |         .i(i2),   
+   20 |         .i(i2),   
       |          ^
-        t/t_lint_pindup_bad.v:20:10: ... Location of original pin connection
+        t/t_lint_pindup_bad.v:19:10: ... Location of original pin connection
-   20 |         .i(i),
+   19 |         .i(i),
       |          ^
-%Error-PINNOTFOUND: t/t_lint_pindup_bad.v:22:10: Pin not found: 'nexist'
+%Error-PINNOTFOUND: t/t_lint_pindup_bad.v:21:10: Pin not found: 'nexist'
                                                : ... Suggested alternative: 'exists'
-   22 |         .nexist(i2)   
+   21 |         .nexist(i2)   
       |          ^~~~~~
-%Error-PINNOTFOUND: t/t_lint_pindup_bad.v:16:9: Parameter not found: 'NEXIST'
+%Error-PINNOTFOUND: t/t_lint_pindup_bad.v:15:9: Parameter not found: 'NEXIST'
                                               : ... Suggested alternative: 'EXIST'
-   16 |        .NEXIST(1),   
+   15 |      #(.NEXIST(1),   
       |         ^~~~~~
 %Error: t/t_lint_pindup_bad.v:17:9: Duplicate parameter connection: 'P'
-   17 |        .P(2),
+   17 |        .P(3))   
       |         ^
-        t/t_lint_pindup_bad.v:15:8: ... Location of original parameter connection
+        t/t_lint_pindup_bad.v:16:9: ... Location of original parameter connection
-   15 |      #(,   
+   16 |        .P(2),
-      |        ^
-%Error: t/t_lint_pindup_bad.v:18:9: Duplicate parameter connection: 'P'
-   18 |        .P(3))   
       |         ^
-        t/t_lint_pindup_bad.v:15:8: ... Location of original parameter connection
-   15 |      #(,   
-      |        ^
 %Error: Exiting due to

test_regress/t/t_lint_pindup_bad.v

@@ -12,8 +12,7 @@ module t
    );
 
    sub
-     #(,  // Not found
+     #(.NEXIST(1),  // Not found
-       .NEXIST(1),  // Not found
        .P(2),
        .P(3))  // Dup
    sub (.o(o),