diff --git a/Changes b/Changes index 492f904aa..f8d63157c 100644 --- a/Changes +++ b/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). diff --git a/src/verilog.y b/src/verilog.y index d6bd09fd5..b840de673 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -3264,8 +3264,14 @@ cellpinListE: ; cellparamItListE: // IEEE: list_of_parameter_value_assignments/list_of_parameter_assignments - cellparamItemE { $$ = $1; } - | cellparamItListE ',' cellparamItemE { $$ = addNextNull($1, $3); } + // // Empty gets a node, to track class reference of #() + /*empty*/ { $$ = new AstPin{CRELINE(), PINNUMINC(), "", nullptr}; } + | cellparamItList { $$ = $1; } + ; + +cellparamItList: // IEEE: list_of_parameter_value_assignments/list_of_parameter_assignments + cellparamItem { $$ = $1; } + | cellparamItList ',' cellparamItem { $$ = addNextNull($1, $3); } ; cellpinItListE: // IEEE: list_of_port_connections @@ -3273,10 +3279,9 @@ cellpinItListE: // IEEE: list_of_port_connections | cellpinItListE ',' cellpinItemE { $$ = addNextNull($1, $3); } ; -cellparamItemE: // 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}; } +cellparamItem: // IEEE: named_parameter_assignment + empty + // // Note empty is not allowed in parameter lists + yP_DOTSTAR { $$ = new AstPin{$1, PINNUMINC(), ".*", nullptr}; } | '.' idAny '(' ')' { $$ = new AstPin{$2, PINNUMINC(), *$2, nullptr}; $$->svDotName(true); } diff --git a/test_regress/t/t_class_param_comma_bad.out b/test_regress/t/t_class_param_comma_bad.out new file mode 100644 index 000000000..b7de6da25 --- /dev/null +++ b/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 diff --git a/test_regress/t/t_class_param_comma_bad.pl b/test_regress/t/t_class_param_comma_bad.pl new file mode 100755 index 000000000..7be596e0f --- /dev/null +++ b/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; diff --git a/test_regress/t/t_class_param_comma_bad.v b/test_regress/t/t_class_param_comma_bad.v new file mode 100644 index 000000000..9f165b470 --- /dev/null +++ b/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 diff --git a/test_regress/t/t_inst_param_comma_bad.out b/test_regress/t/t_inst_param_comma_bad.out new file mode 100644 index 000000000..a272b2577 --- /dev/null +++ b/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 diff --git a/test_regress/t/t_inst_param_comma_bad.pl b/test_regress/t/t_inst_param_comma_bad.pl new file mode 100755 index 000000000..376c2d2ee --- /dev/null +++ b/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; diff --git a/test_regress/t/t_inst_param_comma_bad.v b/test_regress/t/t_inst_param_comma_bad.v new file mode 100644 index 000000000..1ef73846d --- /dev/null +++ b/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 diff --git a/test_regress/t/t_lint_pindup_bad.out b/test_regress/t/t_lint_pindup_bad.out index 7cdb90a6c..c818d6170 100644 --- a/test_regress/t/t_lint_pindup_bad.out +++ b/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' - 19 | sub (.o(o), +%Warning-PINMISSING: t/t_lint_pindup_bad.v:18:4: Cell has missing pin: 'exists' + 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' - 21 | .i(i2), +%Error: t/t_lint_pindup_bad.v:20:10: Duplicate pin connection: 'i' + 20 | .i(i2), | ^ - t/t_lint_pindup_bad.v:20:10: ... Location of original pin connection - 20 | .i(i), + t/t_lint_pindup_bad.v:19:10: ... Location of original pin connection + 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 - 15 | #(, - | ^ -%Error: t/t_lint_pindup_bad.v:18:9: Duplicate parameter connection: 'P' - 18 | .P(3)) + t/t_lint_pindup_bad.v:16:9: ... Location of original parameter connection + 16 | .P(2), | ^ - t/t_lint_pindup_bad.v:15:8: ... Location of original parameter connection - 15 | #(, - | ^ %Error: Exiting due to diff --git a/test_regress/t/t_lint_pindup_bad.v b/test_regress/t/t_lint_pindup_bad.v index 115911643..f98163b7f 100644 --- a/test_regress/t/t_lint_pindup_bad.v +++ b/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),