From 1f0357ba93b8b81191468dcf853c722943487fad Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Wed, 16 Jul 2025 08:18:57 -0400 Subject: [PATCH] Add NOEFFECT warning, replacing previous `foreach` error. --- Changes | 1 + docs/guide/warnings.rst | 15 ++++++++ src/V3Begin.cpp | 6 ++- src/V3Error.h | 49 +++++++++++-------------- src/verilog.y | 4 +- test_regress/t/t_foreach_bad.out | 7 +--- test_regress/t/t_foreach_bad.v | 2 - test_regress/t/t_foreach_noivar.py | 18 +++++++++ test_regress/t/t_foreach_noivar.v | 31 ++++++++++++++++ test_regress/t/t_foreach_noivar_bad.out | 11 ++++++ test_regress/t/t_foreach_noivar_bad.py | 17 +++++++++ 11 files changed, 124 insertions(+), 37 deletions(-) create mode 100755 test_regress/t/t_foreach_noivar.py create mode 100644 test_regress/t/t_foreach_noivar.v create mode 100644 test_regress/t/t_foreach_noivar_bad.out create mode 100755 test_regress/t/t_foreach_noivar_bad.py diff --git a/Changes b/Changes index 7e20b5412..c5323559b 100644 --- a/Changes +++ b/Changes @@ -14,6 +14,7 @@ Verilator 5.039 devel **Other:** * Add ENUMITEMWIDTH error, and apply to X-extended and ranged values. +* Add NOEFFECT warning, replacing previous `foreach` error. * Support member-level triggers for virtual interfaces (#5166) (#6148). [Yilou Wang] * Support disable dotted references (#6154). [Ryszard Rozak, Antmicro Ltd.] * Support randomize() on class member selects (#6161). [Igor Zaworski, Antmicro Ltd.] diff --git a/docs/guide/warnings.rst b/docs/guide/warnings.rst index b14612600..a7143fe3b 100644 --- a/docs/guide/warnings.rst +++ b/docs/guide/warnings.rst @@ -1308,6 +1308,21 @@ List Of Warnings simulate correctly. +.. option:: NOEFFECT + + Warns that the statement will have no effect and is roughly equivalent + to not being present. This is only issued when it is "non-obvious", + e.g. a :code:`if (0)` will not result in this warning. + + Faulty example: + + .. code-block:: sv + + foreach (array[]) begin ... end //<--- Warning + + For a fix, remove the statement. + + .. option:: NOLATCH .. TODO better example diff --git a/src/V3Begin.cpp b/src/V3Begin.cpp index 0484b9038..b1f5ce772 100644 --- a/src/V3Begin.cpp +++ b/src/V3Begin.cpp @@ -530,7 +530,11 @@ AstNode* V3Begin::convertToWhile(AstForeach* nodep) { } // The parser validates we don't have "foreach (array[,,,])" AstNode* const bodyp = nodep->stmtsp(); - UASSERT_OBJ(newp, nodep, "foreach has no non-empty loop variable"); + if (!newp) { + nodep->v3warn(NOEFFECT, "foreach with no loop variable has no effect"); + VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep); + return nullptr; + } if (bodyp) { bodyPointp->replaceWith(bodyp->unlinkFrBackWithNext()); } else { diff --git a/src/V3Error.h b/src/V3Error.h index 307de72e8..b8ca71866 100644 --- a/src/V3Error.h +++ b/src/V3Error.h @@ -125,6 +125,7 @@ public: MULTIDRIVEN, // Driven from multiple blocks MULTITOP, // Multiple top level modules NEWERSTD, // Newer language standard required + NOEFFECT, // Statement has no effect NOLATCH, // No latch detected in always_latch block NONSTD, // Non-standard feature present in other sims NULLPORT, // Null port detected in module definition @@ -190,41 +191,33 @@ public: : m_e(static_cast(_e)) {} // Need () or GCC 4.8 false warning constexpr operator en() const VL_MT_SAFE { return m_e; } const char* ascii() const VL_MT_SAFE { - // clang-format off static const char* const names[] = { // Leading spaces indicate it can't be disabled. " MIN", " INFO", " FATAL", " FATALMANY", " FATALSRC", " ERROR", " FIRST_NAMED", // Boolean - " I_CELLDEFINE", " I_COVERAGE", " I_DEF_NETTYPE_WIRE", " I_LINT", " I_TIMING", " I_TRACING", " I_UNUSED", + " I_CELLDEFINE", " I_COVERAGE", " I_DEF_NETTYPE_WIRE", " I_LINT", " I_TIMING", + " I_TRACING", " I_UNUSED", // Errors "LIFETIME", "NEEDTIMINGOPT", "NOTIMING", "PORTSHORT", "TASKNSVAR", "UNSUPPORTED", // Warnings - " EC_FIRST_WARN", - "ALWCOMBORDER", "ASCRANGE", "ASSIGNDLY", "ASSIGNIN", "BADSTDPRAGMA", "BADVLTPRAGMA", - "BLKANDNBLK", "BLKLOOPINIT", "BLKSEQ", "BSSPACE", - "CASEINCOMPLETE", "CASEOVERLAP", "CASEWITHX", "CASEX", "CASTCONST", "CDCRSTLOGIC", "CLKDATA", - "CMPCONST", "COLONPLUS", "COMBDLY", "CONSTRAINTIGN", "CONTASSREG", "COVERIGN", - "DECLFILENAME", "DEFOVERRIDE", "DEFPARAM", "DEPRECATED", - "ENCAPSULATED", "ENDLABEL", "ENUMITEMWIDTH", "ENUMVALUE", "EOFNEWLINE", - "GENCLK", "GENUNNAMED", "HIERBLOCK", - "IFDEPTH", "IGNOREDRETURN", - "IMPERFECTSCH", "IMPLICIT", "IMPLICITSTATIC", "IMPORTSTAR", "IMPURE", - "INCABSPATH", "INFINITELOOP", "INITIALDLY", "INSECURE", - "LATCH", "LITENDIAN", "MINTYPMAXDLY", "MISINDENT", "MODDUP", "MODMISSING", - "MULTIDRIVEN", "MULTITOP", "NEWERSTD", "NOLATCH", "NONSTD", "NULLPORT", "PINCONNECTEMPTY", - "PINMISSING", "PINNOCONNECT", "PINNOTFOUND", "PKGNODECL", "PREPROCZERO", "PROCASSINIT", "PROCASSWIRE", - "PROFOUTOFDATE", "PROTECTED", "RANDC", "REALCVT", "REDEFMACRO", "RISEFALLDLY", - "SELRANGE", "SHORTREAL", "SIDEEFFECT", "SPLITVAR", - "STATICVAR", "STMTDLY", "SYMRSVDWORD", "SYNCASYNCNET", - "TICKCOUNT", "TIMESCALEMOD", - "UNDRIVEN", "UNOPT", "UNOPTFLAT", "UNOPTTHREADS", - "UNPACKED", "UNSIGNED", "UNUSEDGENVAR", "UNUSEDLOOP" ,"UNUSEDPARAM", "UNUSEDSIGNAL", - "USERERROR", "USERFATAL", "USERINFO", "USERWARN", - "VARHIDDEN", "WAITCONST", "WIDTH", "WIDTHCONCAT", "WIDTHEXPAND", "WIDTHTRUNC", "WIDTHXZEXPAND", - "ZERODLY", "ZEROREPL", - " MAX" - }; - // clang-format on + " EC_FIRST_WARN", "ALWCOMBORDER", "ASCRANGE", "ASSIGNDLY", "ASSIGNIN", "BADSTDPRAGMA", + "BADVLTPRAGMA", "BLKANDNBLK", "BLKLOOPINIT", "BLKSEQ", "BSSPACE", "CASEINCOMPLETE", + "CASEOVERLAP", "CASEWITHX", "CASEX", "CASTCONST", "CDCRSTLOGIC", "CLKDATA", "CMPCONST", + "COLONPLUS", "COMBDLY", "CONSTRAINTIGN", "CONTASSREG", "COVERIGN", "DECLFILENAME", + "DEFOVERRIDE", "DEFPARAM", "DEPRECATED", "ENCAPSULATED", "ENDLABEL", "ENUMITEMWIDTH", + "ENUMVALUE", "EOFNEWLINE", "GENCLK", "GENUNNAMED", "HIERBLOCK", "IFDEPTH", + "IGNOREDRETURN", "IMPERFECTSCH", "IMPLICIT", "IMPLICITSTATIC", "IMPORTSTAR", "IMPURE", + "INCABSPATH", "INFINITELOOP", "INITIALDLY", "INSECURE", "LATCH", "LITENDIAN", + "MINTYPMAXDLY", "MISINDENT", "MODDUP", "MODMISSING", "MULTIDRIVEN", "MULTITOP", + "NEWERSTD", "NOEFFECT", "NOLATCH", "NONSTD", "NULLPORT", "PINCONNECTEMPTY", + "PINMISSING", "PINNOCONNECT", "PINNOTFOUND", "PKGNODECL", "PREPROCZERO", "PROCASSINIT", + "PROCASSWIRE", "PROFOUTOFDATE", "PROTECTED", "RANDC", "REALCVT", "REDEFMACRO", + "RISEFALLDLY", "SELRANGE", "SHORTREAL", "SIDEEFFECT", "SPLITVAR", "STATICVAR", + "STMTDLY", "SYMRSVDWORD", "SYNCASYNCNET", "TICKCOUNT", "TIMESCALEMOD", "UNDRIVEN", + "UNOPT", "UNOPTFLAT", "UNOPTTHREADS", "UNPACKED", "UNSIGNED", "UNUSEDGENVAR", + "UNUSEDLOOP", "UNUSEDPARAM", "UNUSEDSIGNAL", "USERERROR", "USERFATAL", "USERINFO", + "USERWARN", "VARHIDDEN", "WAITCONST", "WIDTH", "WIDTHCONCAT", "WIDTHEXPAND", + "WIDTHTRUNC", "WIDTHXZEXPAND", "ZERODLY", "ZEROREPL", " MAX"}; return names[m_e]; } // Warnings that default to off diff --git a/src/verilog.y b/src/verilog.y index 82ae0137f..b25b207f7 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -4181,6 +4181,7 @@ loop_variables: // IEEE: loop_variables parseRefBase { $$ = $1; } | loop_variables ',' parseRefBase { $$ = $1->addNext($3); } | ',' parseRefBase { $$ = new AstEmpty{$1}; $$->addNext($2); } + | ',' { $$ = new AstEmpty{$1}; } ; //************************************************ @@ -6112,7 +6113,6 @@ idArrayedForeach: // IEEE: id + select (under foreach expression) id { $$ = new AstParseRef{$1, VParseRefExp::PX_TEXT, *$1, nullptr, nullptr}; } // // IEEE: id + part_select_range/constant_part_select_range - | idArrayed '[' ']' { $$ = $1; } // Or AstArraySel, don't know yet. | idArrayed '[' expr ']' { $$ = new AstSelBit{$2, $1, $3}; } // Or AstArraySel, don't know yet. | idArrayed '[' constExpr ':' constExpr ']' { $$ = new AstSelExtract{$2, $1, $3, $5}; } // // IEEE: id + indexed_range/constant_indexed_range @@ -6120,6 +6120,8 @@ idArrayedForeach: // IEEE: id + select (under foreach expression) | idArrayed '[' expr yP_MINUSCOLON constExpr ']' { $$ = new AstSelMinus{$2, $1, $3, $5}; } // // IEEE: loop_variables (under foreach expression) // // To avoid conflicts we allow expr as first element, must post-check + | idArrayed '[' ']' + { $$ = new AstSelLoopVars{$2, $1, new AstEmpty{$3}}; } | idArrayed '[' expr ',' loop_variables ']' { $$ = new AstSelLoopVars{$2, $1, addNextNull(static_cast($3), $5)}; } | idArrayed '[' ',' loop_variables ']' diff --git a/test_regress/t/t_foreach_bad.out b/test_regress/t/t_foreach_bad.out index 4987c12d9..9c519fec6 100644 --- a/test_regress/t/t_foreach_bad.out +++ b/test_regress/t/t_foreach_bad.out @@ -2,10 +2,7 @@ 14 | foreach (array); | ^~~~~~~ ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. -%Error: t/t_foreach_bad.v:16:7: Foreach missing bracketed loop variable is no-operation (IEEE 1800-2023 12.7.3) - 16 | foreach (array[]); - | ^~~~~~~ -%Error: t/t_foreach_bad.v:20:23: 'foreach' loop variable expects simple variable name - 20 | foreach (array[a.b]); +%Error: t/t_foreach_bad.v:18:23: 'foreach' loop variable expects simple variable name + 18 | foreach (array[a.b]); | ^ %Error: Exiting due to diff --git a/test_regress/t/t_foreach_bad.v b/test_regress/t/t_foreach_bad.v index 1226338c9..ac2d7bd68 100644 --- a/test_regress/t/t_foreach_bad.v +++ b/test_regress/t/t_foreach_bad.v @@ -13,8 +13,6 @@ module t (/*AUTOARG*/); initial begin foreach (array); // no index - foreach (array[]); // no index - foreach (array.array[a]); // not supported foreach (array[a.b]); // no index diff --git a/test_regress/t/t_foreach_noivar.py b/test_regress/t/t_foreach_noivar.py new file mode 100755 index 000000000..53f0d4385 --- /dev/null +++ b/test_regress/t/t_foreach_noivar.py @@ -0,0 +1,18 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2024 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 + +import vltest_bootstrap + +test.scenarios('simulator') + +test.compile(verilator_flags2=['-Wno-NOEFFECT']) + +test.execute() + +test.passes() diff --git a/test_regress/t/t_foreach_noivar.v b/test_regress/t/t_foreach_noivar.v new file mode 100644 index 000000000..7646328d6 --- /dev/null +++ b/test_regress/t/t_foreach_noivar.v @@ -0,0 +1,31 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2025 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +`define stop $stop +`define checkh(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0); + +module t (/*AUTOARG*/); + + reg [63:0] sum; // Checked not in objects + reg [2:1] [4:3] array [5:6] [7:8]; + + initial begin + sum = 0; + foreach (array[]) begin // NOP + ++sum; + end + `checkh(sum, 0); + + sum = 0; + foreach (array[,,]) begin // NOP + ++sum; + end + `checkh(sum, 0); + + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_foreach_noivar_bad.out b/test_regress/t/t_foreach_noivar_bad.out new file mode 100644 index 000000000..2494ada33 --- /dev/null +++ b/test_regress/t/t_foreach_noivar_bad.out @@ -0,0 +1,11 @@ +%Warning-NOEFFECT: t/t_foreach_noivar.v:17:5: foreach with no loop variable has no effect + : ... note: In instance 't' + 17 | foreach (array[]) begin + | ^~~~~~~ + ... For warning description see https://verilator.org/warn/NOEFFECT?v=latest + ... Use "/* verilator lint_off NOEFFECT */" and lint_on around source to disable this message. +%Warning-NOEFFECT: t/t_foreach_noivar.v:23:5: foreach with no loop variable has no effect + : ... note: In instance 't' + 23 | foreach (array[,,]) begin + | ^~~~~~~ +%Error: Exiting due to diff --git a/test_regress/t/t_foreach_noivar_bad.py b/test_regress/t/t_foreach_noivar_bad.py new file mode 100755 index 000000000..2101e3902 --- /dev/null +++ b/test_regress/t/t_foreach_noivar_bad.py @@ -0,0 +1,17 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2024 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 + +import vltest_bootstrap + +test.scenarios('vlt') +test.top_filename = 't/t_foreach_noivar.v' + +test.lint(fails=True, expect_filename=test.golden_filename) + +test.passes()