Add SIMILARNAME warning when variables have names that only differ in lexical case (#7992) (#8020)

This commit is contained in:
Paul Campbell 2026-08-10 22:57:58 +12:00 committed by GitHub
parent 38259d11b6
commit 5d888d2ac0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 175 additions and 11 deletions

View File

@ -241,6 +241,7 @@ Oleh Maksymenko
Patrick Creighton
Patrick Stewart
Paul Bowen-Huggett
Paul Campbell
Paul Swirhun
Paul Wright
Pawel Jewstafjew

View File

@ -0,0 +1,6 @@
.. comment: generated by t_lint_similarname_bad
.. code-block:: sv
:linenos:
reg i;
wire I;

View File

@ -0,0 +1,9 @@
.. comment: generated by t_lint_similarname_bad
.. code-block::
%Warning-SIMILARNAME: example.v:1:8 Declaration overlaps another with different case: 'I'
13 | wire I;
| ^
example.v:1:7 ... Location of original declaration
12 | reg i;
| ^

View File

@ -2128,6 +2128,25 @@ List Of Warnings
simulators.
.. option:: SIMILARNAME
Warns that a variable name only differs from another in lexical case.
Faulty example:
.. include:: ../../docs/gen/ex_SIMILARNAME_faulty.rst
Results in:
.. include:: ../../docs/gen/ex_SIMILARNAME_msg.rst
Disabled by default as this is a code-style warning; it will simulate
correctly.
This is a warning as some downstream VLSI tools do
not distinguish net and gate names with the same case.
.. option:: SPECIFYIGN
Warns that Verilator does not support certain constructs in

View File

@ -163,6 +163,7 @@ public:
SELRANGE, // Selection index out of range
SHORTREAL, // Shortreal not supported
SIDEEFFECT, // Sideeffect ignored
SIMILARNAME, // names differ only by case
SPECIFYIGN, // Specify construct ignored
SPLITVAR, // Cannot split the variable
STATICVAR, // Static variable declared in a loop with a declaration assignment
@ -239,12 +240,13 @@ public:
"PINCONNECTEMPTY", "PINMISSING", "PINNOCONNECT", "PINNOTFOUND", "PKGNODECL",
"PREPROCZERO", "PROCASSINIT", "PROCASSWIRE", "PROFOUTOFDATE", "PROTECTED",
"PROTOTYPEMIS", "RANDC", "REALCVT", "REDEFMACRO", "RISEFALLDLY", "SELRANGE",
"SHORTREAL", "SIDEEFFECT", "SPECIFYIGN", "SPLITVAR", "STATICVAR", "STMTDLY",
"SUPERNFIRST", "SYMRSVDWORD", "SYNCASYNCNET", "TICKCOUNT", "TIMESCALEMOD", "UNDRIVEN",
"UNOPT", "UNOPTFLAT", "UNOPTTHREADS", "UNPACKED", "UNSATCONSTR", "UNSIGNED", "UNUSED",
"UNUSEDGENVAR", "UNUSEDLOOP", "UNUSEDPARAM", "UNUSEDSIGNAL", "USERERROR", "USERFATAL",
"USERINFO", "USERWARN", "VARHIDDEN", "WAITCONST", "WIDTH", "WIDTHCONCAT",
"WIDTHEXPAND", "WIDTHTRUNC", "WIDTHXZEXPAND", "ZERODLY", "ZEROREPL", " MAX"};
"SHORTREAL", "SIDEEFFECT", "SIMILARNAME", "SPECIFYIGN", "SPLITVAR", "STATICVAR",
"STMTDLY", "SUPERNFIRST", "SYMRSVDWORD", "SYNCASYNCNET", "TICKCOUNT", "TIMESCALEMOD",
"UNDRIVEN", "UNOPT", "UNOPTFLAT", "UNOPTTHREADS", "UNPACKED", "UNSATCONSTR",
"UNSIGNED", "UNUSED", "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
@ -298,9 +300,9 @@ public:
|| m_e == BLKSEQ || m_e == DECLFILENAME || m_e == DEFPARAM || m_e == EOFNEWLINE
|| m_e == GENUNNAMED || m_e == IMPORTSTAR || m_e == INCABSPATH
|| m_e == MULTIDRIVENPROC || m_e == PINCONNECTEMPTY || m_e == PINNOCONNECT
|| m_e == PROCASSINIT || m_e == SYNCASYNCNET || m_e == UNDRIVEN
|| m_e == UNUSEDGENVAR || m_e == UNUSEDLOOP || m_e == UNUSEDPARAM
|| m_e == UNUSEDSIGNAL || m_e == VARHIDDEN);
|| m_e == PROCASSINIT || m_e == SIMILARNAME || m_e == SYNCASYNCNET
|| m_e == UNDRIVEN || m_e == UNUSEDGENVAR || m_e == UNUSEDLOOP
|| m_e == UNUSEDPARAM || m_e == UNUSEDSIGNAL || m_e == VARHIDDEN);
}
bool isNamed() const { return m_e >= EC_FIRST_NAMED; }

View File

@ -356,10 +356,22 @@ public:
//
// Note we only check for conflicts at the same level; it's ok if one block hides another
// We also wouldn't want to not insert it even though it's lower down
const VSymEnt* const foundp = lookupSymp->findIdFlat(name);
AstNode* const fnodep = foundp ? foundp->nodep() : nullptr;
if (!fnodep) {
// Not found, will add in a moment.
if (!lookupSymp->ignoreForSimilarTest(nodep->type())) { // ignore typedefs etc
const VSymEnt* const alt = lookupSymp->findSimilarIdFlat(name);
if (alt) {
nodep->v3warn(SIMILARNAME, "Declaration overlaps another with different case: "
<< nodep->prettyNameQ() << '\n'
<< nodep->warnContextPrimary() << '\n'
<< alt->nodep()->warnOther()
<< "... Location of original declaration\n"
<< alt->nodep()->warnContextSecondary());
}
}
} else if (nodep == fnodep) { // Already inserted.
// Good.
} else if (foundp->imported()) { // From package

View File

@ -45,6 +45,7 @@ class VSymEnt final {
// MEMBERS
using IdNameMap = std::multimap<std::string, VSymEnt*>;
IdNameMap m_idNameMap; // Hash of variables by name
std::unordered_set<std::string> m_idNameSimilarMap; // Variables by name with same case
AstNode* m_nodep; // Node that entry belongs to
VSymEnt* m_fallbackp = nullptr; // Table "above" this in name scope, for fallback resolution
VSymEnt* m_parentp = nullptr; // Table that created this
@ -135,6 +136,14 @@ public:
} else {
m_idNameMap.emplace(name, entp);
}
if (name.find("__DOT__") == std::string::npos
&& !ignoreForSimilarTest(entp->nodep()->type())) { // ignore hierarchical equivalents
string lc = name;
for (auto& c : lc) c = (char)tolower(c);
if (m_idNameSimilarMap.find(lc) == m_idNameSimilarMap.end())
m_idNameSimilarMap.insert(lc);
}
return entp;
}
void reinsert(const string& name, VSymEnt* entp) {
@ -159,6 +168,36 @@ public:
if (it != m_idNameMap.end()) return it->second;
return nullptr;
}
bool ignoreForSimilarTest(VNType t) { // node types that don't affect final net types
switch (t) {
case VNType::TypedefFwd:
case VNType::Typedef:
case VNType::ParamTypeDType:
case VNType::EnumItem:
case VNType::EnumItemRef:
case VNType::Let:
case VNType::Class:
case VNType::Task:
case VNType::Func: return true;
default: return false;
}
}
VSymEnt* findSimilarIdFlat(const string& name) const {
// Find identifier without looking upward through symbol hierarchy
// Were looking for symbols that are the same when compared without
// caring about case, but that are not the same name
if (name.find("__DOT__") != std::string::npos) // ignore hierarchical equivalents
return nullptr;
string s = name;
for (auto& c : s) c = (char)tolower(c);
if (m_idNameSimilarMap.find(s) == m_idNameSimilarMap.end()) return nullptr;
for (auto it = m_idNameMap.begin(); it != m_idNameMap.end(); ++it) {
string t = it->first;
for (auto& c : t) c = (char)tolower(c);
if (t == s && name != it->first) { return it->second; }
}
return nullptr;
}
VSymEnt* findIdFallback(const string& name) const {
// Find identifier looking upward through symbol hierarchy
// First, scan this begin/end block or module for the name
@ -168,7 +207,8 @@ public:
return nullptr;
}
void candidateIdFlat(VSpellCheck* spellerp, const VNodeMatcher* matcherp) const {
// Suggest alternative symbol candidates without looking upward through symbol hierarchy
// Suggest alternative symbol candidates without looking upward through symbol
// hierarchy
for (IdNameMap::const_iterator it = m_idNameMap.begin(); it != m_idNameMap.end(); ++it) {
const AstNode* const itemp = it->second->nodep();
if (itemp && (!matcherp || matcherp->nodeMatch(itemp))) {
@ -209,7 +249,8 @@ public:
// Used for classes in early parsing only to handle "extends"
// If an "extern foo" exists, then we can't import "foo" from the base class.
// But ok for "extern foo" and "foo" to both come from base (so must check before insert)
// But ok for "extern foo" and "foo" to both come from base (so must check before
// insert)
std::unordered_set<std::string> haveExterns;
for (IdNameMap::const_iterator it = srcp->m_idNameMap.begin();
it != srcp->m_idNameMap.end(); ++it) {

View File

@ -0,0 +1,18 @@
#!/usr/bin/env python3
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# 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-FileCopyrightText: 2025 Wilson Snyder
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
import vltest_bootstrap
test.scenarios('simulator_st')
test.compile(verilator_flags2=['-Wall -Wno-DECLFILENAME -Wno-SIMILARNAME'])
test.execute()
test.passes()

View File

@ -0,0 +1,20 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain.
// SPDX-FileCopyrightText: 2025 Wilson Snyder
// SPDX-License-Identifier: CC0-1.0
module t;
// verilator lint_off UNDRIVEN
// verilator lint_off UNUSEDSIGNAL
reg i;
wire I;
initial begin
$finish;
end
endmodule

View File

@ -0,0 +1,9 @@
%Warning-SIMILARNAME: t/t_lint_similarname.v:13:8: Declaration overlaps another with different case: 'I'
13 | wire I;
| ^
t/t_lint_similarname.v:12:7: ... Location of original declaration
12 | reg i;
| ^
... For warning description see https://verilator.org/warn/SIMILARNAME?v=latest
... Use "/* verilator lint_off SIMILARNAME */" and lint_on around source to disable this message.
%Error: Exiting due to

View File

@ -0,0 +1,27 @@
#!/usr/bin/env python3
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# 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-FileCopyrightText: 2025 Wilson Snyder
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
import vltest_bootstrap
test.scenarios('vlt')
test.top_filename = "t/t_lint_similarname.v"
test.lint(verilator_flags2=['-Wall -Wno-DECLFILENAME'],
fails=True,
expect_filename=test.golden_filename)
test.extract(in_filename=test.top_filename,
out_filename=test.root + "/docs/gen/ex_SIMILARNAME_faulty.rst",
lines="12-13")
test.extract(in_filename=test.golden_filename,
out_filename=test.root + "/docs/gen/ex_SIMILARNAME_msg.rst",
lines="1-6")
test.passes()