From 5d888d2ac04407b17c176c8939ad1bb8f7557017 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Mon, 10 Aug 2026 22:57:58 +1200 Subject: [PATCH] Add SIMILARNAME warning when variables have names that only differ in lexical case (#7992) (#8020) --- docs/CONTRIBUTORS | 1 + docs/gen/ex_SIMILARNAME_faulty.rst | 6 +++ docs/gen/ex_SIMILARNAME_msg.rst | 9 +++++ docs/guide/warnings.rst | 19 ++++++++++ src/V3Error.h | 20 +++++----- src/V3LinkDot.cpp | 12 ++++++ src/V3SymTable.h | 45 ++++++++++++++++++++++- test_regress/t/t_lint_similarname.py | 18 +++++++++ test_regress/t/t_lint_similarname.v | 20 ++++++++++ test_regress/t/t_lint_similarname_bad.out | 9 +++++ test_regress/t/t_lint_similarname_bad.py | 27 ++++++++++++++ 11 files changed, 175 insertions(+), 11 deletions(-) create mode 100644 docs/gen/ex_SIMILARNAME_faulty.rst create mode 100644 docs/gen/ex_SIMILARNAME_msg.rst create mode 100755 test_regress/t/t_lint_similarname.py create mode 100644 test_regress/t/t_lint_similarname.v create mode 100644 test_regress/t/t_lint_similarname_bad.out create mode 100755 test_regress/t/t_lint_similarname_bad.py diff --git a/docs/CONTRIBUTORS b/docs/CONTRIBUTORS index e124d4e26..27f8e06f8 100644 --- a/docs/CONTRIBUTORS +++ b/docs/CONTRIBUTORS @@ -241,6 +241,7 @@ Oleh Maksymenko Patrick Creighton Patrick Stewart Paul Bowen-Huggett +Paul Campbell Paul Swirhun Paul Wright Pawel Jewstafjew diff --git a/docs/gen/ex_SIMILARNAME_faulty.rst b/docs/gen/ex_SIMILARNAME_faulty.rst new file mode 100644 index 000000000..f8664a738 --- /dev/null +++ b/docs/gen/ex_SIMILARNAME_faulty.rst @@ -0,0 +1,6 @@ +.. comment: generated by t_lint_similarname_bad +.. code-block:: sv + :linenos: + + reg i; + wire I; diff --git a/docs/gen/ex_SIMILARNAME_msg.rst b/docs/gen/ex_SIMILARNAME_msg.rst new file mode 100644 index 000000000..0711791be --- /dev/null +++ b/docs/gen/ex_SIMILARNAME_msg.rst @@ -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; + | ^ diff --git a/docs/guide/warnings.rst b/docs/guide/warnings.rst index 0fcc0475b..1f918ed6d 100644 --- a/docs/guide/warnings.rst +++ b/docs/guide/warnings.rst @@ -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 diff --git a/src/V3Error.h b/src/V3Error.h index a6a516c8d..dcd419c31 100644 --- a/src/V3Error.h +++ b/src/V3Error.h @@ -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; } diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index 210f157f3..b9d786d69 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -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 diff --git a/src/V3SymTable.h b/src/V3SymTable.h index f3cb1bfb1..f0f9c1643 100644 --- a/src/V3SymTable.h +++ b/src/V3SymTable.h @@ -45,6 +45,7 @@ class VSymEnt final { // MEMBERS using IdNameMap = std::multimap; IdNameMap m_idNameMap; // Hash of variables by name + std::unordered_set 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 haveExterns; for (IdNameMap::const_iterator it = srcp->m_idNameMap.begin(); it != srcp->m_idNameMap.end(); ++it) { diff --git a/test_regress/t/t_lint_similarname.py b/test_regress/t/t_lint_similarname.py new file mode 100755 index 000000000..7415174e7 --- /dev/null +++ b/test_regress/t/t_lint_similarname.py @@ -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() diff --git a/test_regress/t/t_lint_similarname.v b/test_regress/t/t_lint_similarname.v new file mode 100644 index 000000000..ecbda5c95 --- /dev/null +++ b/test_regress/t/t_lint_similarname.v @@ -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 diff --git a/test_regress/t/t_lint_similarname_bad.out b/test_regress/t/t_lint_similarname_bad.out new file mode 100644 index 000000000..f4541eddb --- /dev/null +++ b/test_regress/t/t_lint_similarname_bad.out @@ -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 diff --git a/test_regress/t/t_lint_similarname_bad.py b/test_regress/t/t_lint_similarname_bad.py new file mode 100755 index 000000000..db4d64cba --- /dev/null +++ b/test_regress/t/t_lint_similarname_bad.py @@ -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()