Add ASSIGNEQEXPR when use `=` inside expressions (#5567).

This commit is contained in:
Wilson Snyder 2025-09-14 08:28:47 -04:00
parent 06c3c87f4e
commit b455f9b591
13 changed files with 165 additions and 25 deletions

View File

@ -16,6 +16,7 @@ Verilator 5.041 devel
* Add error on non-packed struct randc (#5999). [Seth Pellegrino]
* Add configure `--enable-asan` to compile verilator_bin with the address sanitizer (#6404). [Geza Lore]
* Add $(LDFLAGS) and $(LIBS) to when building shared libraries (#6425) (#6426). [Ahmed El-Mahmoudy]
* Add ASSIGNEQEXPR when use `=` inside expressions (#5567). [Ethan Sifferman]
* Support pure functions in sensitivity lists (#6393). [Krzysztof Bieganski, Antmicro Ltd.]
* Improve automatic selection of logic for DFG synthesis (#6370). [Geza Lore]
* Improve `covergroup with function sample` handling (#6387). [Jakub Wasilewski]

View File

@ -0,0 +1,9 @@
.. comment: generated by t_lint_assigneqexpr_bad
.. code-block:: sv
:linenos:
:emphasize-lines: 3
assign d_o = // Note = not == below
(
c_o = 1 // <--- Warning: ASSIGNEQEXPR
) ? 1 : (

View File

@ -0,0 +1,5 @@
.. comment: generated by t_lint_assigneqexpr_bad
.. code-block::
%Warning-ASSIGNEQEXPR: example.v:1:11 Assignment '=' inside expression
: ... Was a '==' intended, or suggest use a separate statement

View File

@ -1878,7 +1878,7 @@ Summary:
.. option:: -Wno-lint
Disable all lint-related warning messages, and all style warnings. This
is equivalent to ``-Wno-ALWCOMBORDER`` ``-Wno-ASCRANGE``
is equivalent to ``-Wno-ALWCOMBORDER`` ``-Wno-ASCRANGE`` ``-Wno-ASSIGNEQEXPR``
``-Wno-BSSPACE`` ``-Wno-CASEINCOMPLETE`` ``-Wno-CASEOVERLAP``
``-Wno-CASEX`` ``-Wno-CASTCONST`` ``-Wno-CASEWITHX`` ``-Wno-CMPCONST``
``-Wno-COLONPLUS`` ``-Wno-IMPLICIT`` ``-Wno-IMPLICITSTATIC``
@ -1932,14 +1932,14 @@ Summary:
Enable all lint-related warning messages (note that by default, they are
already enabled), but do not affect style messages. This is equivalent
to ``-Wwarn-ALWCOMBORDER`` ``-Wwarn-ASCRANGE`` ``-Wwarn-BSSPACE``
``-Wwarn-CASEINCOMPLETE`` ``-Wwarn-CASEOVERLAP`` ``-Wwarn-CASEWITHX``
``-Wwarn-CASEX`` ``-Wwarn-CASTCONST`` ``-Wwarn-CMPCONST``
``-Wwarn-COLONPLUS`` ``-Wwarn-IMPLICIT`` ``-Wwarn-IMPLICITSTATIC``
``-Wwarn-LATCH`` ``-Wwarn-MISINDENT`` ``-Wwarn-NEWERSTD``
``-Wwarn-PREPROCZERO`` ``-Wwarn-PINMISSING`` ``-Wwarn-REALCVT``
``-Wwarn-STATICVAR`` ``-Wwarn-UNSIGNED`` ``-Wwarn-WIDTHTRUNC``
``-Wwarn-WIDTHEXPAND`` ``-Wwarn-WIDTHXZEXPAND``.
to ``-Wwarn-ALWCOMBORDER`` ``-Wwarn-ASCRANGE`` ``-Wno-ASSIGNEQEXPR``
``-Wwarn-BSSPACE`` ``-Wwarn-CASEINCOMPLETE`` ``-Wwarn-CASEOVERLAP``
``-Wwarn-CASEWITHX`` ``-Wwarn-CASEX`` ``-Wwarn-CASTCONST``
``-Wwarn-CMPCONST`` ``-Wwarn-COLONPLUS`` ``-Wwarn-IMPLICIT``
``-Wwarn-IMPLICITSTATIC`` ``-Wwarn-LATCH`` ``-Wwarn-MISINDENT``
``-Wwarn-NEWERSTD`` ``-Wwarn-PREPROCZERO`` ``-Wwarn-PINMISSING``
``-Wwarn-REALCVT`` ``-Wwarn-STATICVAR`` ``-Wwarn-UNSIGNED``
``-Wwarn-WIDTHTRUNC`` ``-Wwarn-WIDTHEXPAND`` ``-Wwarn-WIDTHXZEXPAND``.
.. option:: -Wwarn-style

View File

@ -169,6 +169,29 @@ List Of Warnings
This warning is issued only if Verilator is run with :vlopt:`--no-timing`.
.. option:: ASSIGNEQEXPR
Warning that an assignment with `=` appears in a complex expression.
The intent may have been to use `==`, or, if `=` is correct this may be
a readability issue.
Faulty example:
.. include:: ../../docs/gen/ex_ASSIGNEQEXPR_faulty.rst
Results in:
.. include:: ../../docs/gen/ex_ASSIGNEQEXPR_msg.rst
To repair, make the assignment into a separate statement.
Disabled by default as this is a code-style warning; it will simulate
correctly.
Ignoring this warning will only suppress the lint check; it will
simulate correctly.
.. option:: ASSIGNIN
.. TODO better example

View File

@ -75,6 +75,7 @@ public:
ALWNEVER, // always will never execute
ASCRANGE, // Ascending bit range vector
ASSIGNDLY, // Assignment delays
ASSIGNEQEXPR, // Assignment equal (=) in expression
ASSIGNIN, // Assigning to input
BADSTDPRAGMA, // Any error related to pragmas
BADVLTPRAGMA, // Unknown Verilator pragma
@ -206,15 +207,15 @@ public:
"CONSTWRITTEN", "LIFETIME", "NEEDTIMINGOPT", "NOTIMING", "PORTSHORT", "TASKNSVAR",
"UNSUPPORTED",
// Warnings
" EC_FIRST_WARN", "ALWCOMBORDER", "ALWNEVER", "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",
" EC_FIRST_WARN", "ALWCOMBORDER", "ALWNEVER", "ASCRANGE", "ASSIGNDLY", "ASSIGNEQEXPR",
"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",
"PARAMNODEFAULT", "PINCONNECTEMPTY", "PINMISSING", "PINNOCONNECT", "PINNOTFOUND",
"PKGNODECL", "PREPROCZERO", "PROCASSINIT", "PROCASSWIRE", "PROFOUTOFDATE", "PROTECTED",
@ -264,12 +265,12 @@ public:
}
// Warnings that are lint only
bool lintError() const VL_MT_SAFE {
return (m_e == ALWCOMBORDER || m_e == ASCRANGE || m_e == BSSPACE || m_e == CASEINCOMPLETE
|| m_e == CASEOVERLAP || m_e == CASEWITHX || m_e == CASEX || m_e == CASTCONST
|| m_e == CMPCONST || m_e == COLONPLUS || m_e == IMPLICIT || m_e == IMPLICITSTATIC
|| m_e == LATCH || m_e == MISINDENT || m_e == NEWERSTD || m_e == PREPROCZERO
|| m_e == PINMISSING || m_e == REALCVT || m_e == STATICVAR || m_e == UNSIGNED
|| m_e == WIDTH || m_e == WIDTHTRUNC || m_e == WIDTHEXPAND
return (m_e == ALWCOMBORDER || m_e == ASCRANGE || m_e == ASSIGNEQEXPR || m_e == BSSPACE
|| m_e == CASEINCOMPLETE || m_e == CASEOVERLAP || m_e == CASEWITHX || m_e == CASEX
|| m_e == CASTCONST || m_e == CMPCONST || m_e == COLONPLUS || m_e == IMPLICIT
|| m_e == IMPLICITSTATIC || m_e == LATCH || m_e == MISINDENT || m_e == NEWERSTD
|| m_e == PREPROCZERO || m_e == PINMISSING || m_e == REALCVT || m_e == STATICVAR
|| m_e == UNSIGNED || m_e == WIDTH || m_e == WIDTHTRUNC || m_e == WIDTHEXPAND
|| m_e == WIDTHXZEXPAND);
}
// Warnings that are style only

View File

@ -157,6 +157,13 @@ static void ERRSVKWD(FileLine* fileline, const string& tokname) {
: ""));
}
static void ASSIGNEQEXPR(FileLine* fileline) {
fileline->v3warn(ASSIGNEQEXPR,
"Assignment '=' inside expression\n"
<< fileline->warnMore()
<< "... Was a '==' intended, or suggest use a separate statement");
}
static void UNSUPREAL(FileLine* fileline) {
fileline->v3warn(SHORTREAL,
"Unsupported: shortreal being promoted to real (suggest use real instead)");
@ -4828,7 +4835,8 @@ expr<nodeExprp>: // IEEE: part of expression/constant_expression/
// // Need exprScope of variable_lvalue to prevent conflict
| '(' ~p~exprScope '=' expr ')'
{ $$ = new AstExprStmt{$1, new AstAssign{$3, $2, $4},
$2->cloneTreePure(true)}; }
$2->cloneTreePure(true)};
ASSIGNEQEXPR($<fl>3); }
| '(' ~p~exprScope yP_PLUSEQ expr ')'
{ $$ = new AstExprStmt{$1, new AstAssign{$3, $2, new AstAdd{$3, $2->cloneTreePure(true), $4}},
$2->cloneTreePure(true)}; }

View File

@ -13,6 +13,7 @@ module t;
int b;
int i;
// verilator lint_off ASSIGNEQEXPR
initial begin
a = 10;
i = (a = 2);

View File

@ -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=['--binary', '--trace', '-Wno-ASSIGNEQEXPR'])
test.execute()
test.passes()

View File

@ -0,0 +1,32 @@
// 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
module t (
input logic a2_i,
a1_i,
a0_i,
input logic b_i,
output logic d_o
);
// verilator lint_off PINMISSING
Sub sub (.a_i({a2_i, a1_i, a0_i}), .b_i, .d_o);
// verilator lint_on PINMISSING
endmodule
module Sub (
input logic [2:0] a_i,
input logic b_i,
output logic c_o,
output logic d_o
);
assign c_o = (a_i != 0) ? 1 : 0;
assign d_o = // Note = not == below
(
c_o = 1 // <--- Warning: ASSIGNEQEXPR
) ? 1 : (
c_o = 0 // <--- Warning: ASSIGNEQEXPR
) ? b_i : 0;
endmodule

View File

@ -0,0 +1,11 @@
%Warning-ASSIGNEQEXPR: t/t_lint_assigneqexpr.v:28:11: Assignment '=' inside expression
: ... Was a '==' intended, or suggest use a separate statement
28 | c_o = 1
| ^
... For warning description see https://verilator.org/warn/ASSIGNEQEXPR?v=latest
... Use "/* verilator lint_off ASSIGNEQEXPR */" and lint_on around source to disable this message.
%Warning-ASSIGNEQEXPR: t/t_lint_assigneqexpr.v:30:11: Assignment '=' inside expression
: ... Was a '==' intended, or suggest use a separate statement
30 | c_o = 0
| ^
%Error: Exiting due to

View File

@ -0,0 +1,29 @@
#!/usr/bin/env python3
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2025 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_lint_assigneqexpr.v"
root = ".."
test.lint(verilator_flags2=['-Wall -Wno-DECLFILENAME'],
fails=True,
expect_filename=test.golden_filename)
test.extract(in_filename=test.top_filename,
out_filename=root + "/docs/gen/ex_ASSIGNEQEXPR_faulty.rst",
lines="26-29")
test.extract(in_filename=test.golden_filename,
out_filename=root + "/docs/gen/ex_ASSIGNEQEXPR_msg.rst",
lines="7-8")
test.passes()

View File

@ -21,7 +21,9 @@ module t (/*AUTOARG*/
x <= 1;
end
else if (cyc == 1) begin
// verilator lint_off ASSIGNEQEXPR
y <= (x = 2);
// verilator lint_on ASSIGNEQEXPR
end else begin
if (x != 2) $stop;
if (y != 2) $stop;