Add --waiver-multiline for context-sensitive --waiver-output.

Changes

@@ -17,6 +17,7 @@ Verilator 5.031 devel
 * Support basic constrained random for multi-dimensional dynamic array and queue (#5591). [Yilou Wang]
 * Support `pure constraint`.
 * Add `--no-std-package` as subset-alias of `--no-std`.
+* Add `--waiver-multiline` for context-sensitive `--waiver-output`.
 * Add error on illegal enum base type (#3010). [Iztok Jeras]
 * Add error on `wait` with missing `.triggered` (#4457).
 * Add error when improperly storing to parameter (#5147). [Gökçe Aydos]

bin/verilator

@@ -488,7 +488,8 @@ detailed descriptions of these arguments.
      +verilog2001ext+<ext>      Synonym for +1364-2001ext+<ext>
     --version                   Show program version and exits
     --vpi                       Enable VPI compiles
-    --waiver-output <filename>  Create a waiver file based on the linter warnings
+    --waiver-multiline          Create multiline --match for waivers
+    --waiver-output <filename>  Create a waiver file based on linter warnings
      -Wall                      Enable all style warnings
      -Werror-<message>          Convert warnings to errors
      -Wfuture-<message>         Disable unknown message warnings

docs/guide/exe_verilator.rst

@@ -1649,6 +1649,15 @@ Summary:
 
    Enable the use of VPI and linking against the :file:`verilated_vpi.cpp` files.
 
+.. option:: --waiver-multiline
+
+   When using :vlopt:`--waiver-output \<filename\>`, include a match
+   expression that includes the entire multiline error message as a match
+   regular expression, as opposed to the default of only matching the first
+   line of the error message.  This provides a starting point for creating
+   complex waivers, but such generated waivers will likely require editing
+   for brevity before being reused.
+
 .. option:: --waiver-output <filename>
 
    Generate a waiver file that contains all waiver statements to suppress

src/V3Ast.cpp

@@ -1423,8 +1423,13 @@ string AstNode::instanceStr() const {
     return "";
 }
 void AstNode::v3errorEnd(std::ostringstream& str) const VL_RELEASE(V3Error::s().m_mutex) {
+    // Don't look for instance name when warning is disabled.
+    // In case of large number of warnings, this can
+    // take significant amount of time
+    const string instanceStrExtra
+        = m_fileline->warnIsOff(V3Error::s().errorCode()) ? "" : instanceStr();
     if (!m_fileline) {
-        V3Error::v3errorEnd(str, instanceStr());
+        V3Error::v3errorEnd(str, instanceStrExtra);
     } else {
         std::ostringstream nsstr;
         nsstr << str.str();
@@ -1434,11 +1439,7 @@ void AstNode::v3errorEnd(std::ostringstream& str) const VL_RELEASE(V3Error::s().
             const_cast<AstNode*>(this)->dump(nsstr);
             nsstr << endl;
         }
-        // Don't look for instance name when warning is disabled.
-        // In case of large number of warnings, this can
-        // take significant amount of time
-        m_fileline->v3errorEnd(
-            nsstr, m_fileline->warnIsOff(V3Error::s().errorCode()) ? "" : instanceStr());
+        m_fileline->v3errorEnd(nsstr, instanceStrExtra);
     }
 }
 void AstNode::v3errorEndFatal(std::ostringstream& str) const VL_RELEASE(V3Error::s().m_mutex) {

src/V3Config.cpp

@@ -300,7 +300,11 @@ public:
         m_lastIgnore.it = m_ignLines.begin();
     }
     void addIgnoreMatch(V3ErrorCode code, const string& match) {
-        m_waivers.emplace_back(code, match);
+        // Since Verilator 5.031 the error message compared has context, so
+        // allow old rules to still match using a final '*'
+        string newMatch = match;
+        if (newMatch.empty() || newMatch.back() != '*') newMatch += '*';
+        m_waivers.emplace_back(code, newMatch);
     }
 
     void applyBlock(AstNodeBlock* nodep) {

src/V3Error.cpp

@@ -117,6 +117,8 @@ void V3ErrorGuarded::suppressThisWarning() VL_REQUIRES(m_mutex) {
 // cppcheck-has-bug-suppress constParameter
 void V3ErrorGuarded::v3errorEnd(std::ostringstream& sstr, const string& extra)
     VL_REQUIRES(m_mutex) {
+    // 'extra' is appended to the message, and is is excluded in check for
+    // duplicate messages. Currently used for reporting instance name.
 #if defined(__COVERITY__) || defined(__cppcheck__)
     if (m_errorCode == V3ErrorCode::EC_FATAL) __coverity_panic__(x);
 #endif

src/V3FileLine.cpp

@@ -413,24 +413,36 @@ bool FileLine::warnIsOff(V3ErrorCode code) const {
 // cppverilator-suppress constParameter
 void FileLine::v3errorEnd(std::ostringstream& sstr, const string& extra)
     VL_RELEASE(V3Error::s().m_mutex) {
-    std::ostringstream nsstr;
+    // 'extra' is appended to the message, and is is excluded in check for
+    // duplicate messages. Currently used for reporting instance name.
+    std::ostringstream nsstr;  // sstr with fileline prefix and context
+    std::ostringstream wsstr;  // sstr for waiver (no fileline) with context
     if (lastLineno()) nsstr << this;
     nsstr << sstr.str();
+    wsstr << sstr.str();
     nsstr << "\n";
-    std::ostringstream lstr;
+    wsstr << "\n";
+    std::ostringstream extrass;  // extra spaced out for prefix
     if (!extra.empty()) {
-        lstr << std::setw(ascii().length()) << " "
-             << ": " << extra;
+        extrass << std::setw(ascii().length()) << " "
+                << ": " << extra;
     }
-    m_waive = V3Config::waive(this, V3Error::s().errorCode(), sstr.str());
-    if (warnIsOff(V3Error::s().errorCode()) || m_waive) {
+    if (warnIsOff(V3Error::s().errorCode())) {
         V3Error::s().suppressThisWarning();
-    } else if (!V3Error::s().errorContexted()) {
-        nsstr << warnContextPrimary();
+    } else {
+        if (!V3Error::s().errorContexted()) {
+            const string add = warnContextPrimary();
+            wsstr << add;
+            nsstr << add;
+        }
+        m_waive = V3Config::waive(this, V3Error::s().errorCode(), wsstr.str());
+        if (m_waive) {
+            V3Error::s().suppressThisWarning();
+        } else {
+            V3Waiver::addEntry(V3Error::s().errorCode(), filename(), wsstr.str());
+        }
     }
-    if (!warnIsOff(V3Error::s().errorCode()) && !m_waive)
-        V3Waiver::addEntry(V3Error::s().errorCode(), filename(), sstr.str());
-    V3Error::v3errorEnd(nsstr, lstr.str());
+    V3Error::v3errorEnd(nsstr, extrass.str());
 }
 
 string FileLine::warnMore() const VL_REQUIRES(V3Error::s().m_mutex) {

src/V3Options.cpp

@@ -1707,6 +1707,7 @@ void V3Options::parseOptsList(FileLine* fl, const string& optdir, int argc,
         FileLine::globalWarnOff(V3ErrorCode::WIDTH, false);
         V3Error::pretendError(V3ErrorCode::WIDTH, false);
     });
+    DECL_OPTION("-waiver-multiline", OnOff, &m_waiverMultiline);
     DECL_OPTION("-waiver-output", Set, &m_waiverOutput);
 
     DECL_OPTION("-x-assign", CbVal, [this, fl](const char* valp) {

src/V3Options.h

@@ -297,6 +297,7 @@ private:
     bool m_underlineZero = false;   // main switch: --underline-zero; undocumented old Verilator 2
     bool m_verilate = true;         // main switch: --verilate
     bool m_vpi = false;             // main switch: --vpi
+    bool m_waiverMultiline = false;  // main switch: --waiver-multiline
     bool m_xInitialEdge = false;    // main switch: --x-initial-edge
     bool m_xmlOnly = false;         // main switch: --xml-only
     bool m_jsonOnly = false;        // main switch: --json-only
@@ -544,6 +545,7 @@ public:
     bool reportUnoptflat() const { return m_reportUnoptflat; }
     bool verilate() const { return m_verilate; }
     bool vpi() const { return m_vpi; }
+    bool waiverMultiline() const { return m_waiverMultiline; }
     bool xInitialEdge() const { return m_xInitialEdge; }
     bool xmlOnly() const { return m_xmlOnly; }
     bool jsonOnly() const { return m_jsonOnly; }

src/V3Waiver.cpp

@@ -19,21 +19,58 @@
 #include "V3Waiver.h"
 
 #include "V3File.h"
+#include "V3Global.h"
 #include "V3Options.h"
 
 #include <memory>
 #include <sstream>
 
-void V3Waiver::addEntry(V3ErrorCode errorCode, const std::string& filename, const std::string& str)
+void V3Waiver::addEntry(V3ErrorCode errorCode, const std::string& filename, const std::string& msg)
     VL_MT_SAFE_EXCLUDES(s_mutex) {
     if (filename == V3Options::getStdPackagePath()) return;
     const V3LockGuard lock{s_mutex};
+
+    string trimmsg = msg;
+    if (!v3Global.opt.waiverMultiline()) {
+        const size_t pos = trimmsg.find('\n');
+        trimmsg = trimmsg.substr(0, pos);
+        if (pos != std::string::npos) trimmsg += '*';
+    }
+    {  // Remove line numbers and context "\n [0-9] | ", "\n  ^[~]+"
+        string result;
+        for (const char* cp = trimmsg.c_str(); *cp; cp = *cp ? cp + 1 : cp) {
+            while (*cp == ' ' || isdigit(*cp)) ++cp;
+            if (*cp == '|') ++cp;
+            // ^~~~~
+            while (*cp == ' ' || *cp == '^') ++cp;
+            while (*cp == '~') ++cp;
+            while (*cp && *cp != '\n') result += *cp++;
+            while (*cp == '\n') result += *cp++;
+        }
+        trimmsg = result;
+    }
+    trimmsg += '*';
+    {  // "\n"->"*", " *"->"*", "* "->"*"
+        string result;
+        string add;
+        result.reserve(trimmsg.size());
+        for (const char& c : trimmsg) {
+            if (c == '*' || !std::isprint(c)) {
+                add = "*";
+            } else if (c == ' ') {
+                if (add != "*") add += c;
+            } else {
+                result += add + c;
+                add = "";
+            }
+        }
+        result += add;
+        trimmsg = result;
+    }
+
     std::stringstream entry;
-    const size_t pos = str.find('\n');
     entry << "lint_off -rule " << errorCode.ascii() << " -file \"*" << filename << "\" -match \""
-          << str.substr(0, pos);
-    if (pos != std::string::npos) entry << "*";
-    entry << "\"";
+          << trimmsg << "\"";
     s_waiverList.push_back(entry.str());
 }
 
@@ -46,9 +83,9 @@ void V3Waiver::write(const std::string& filename) VL_MT_SAFE_EXCLUDES(s_mutex) {
 
     *ofp << "`verilator_config\n\n";
 
-    *ofp << "// Below you find suggested waivers. You have three options:\n";
-    *ofp << "//   1. Fix the reason for the linter warning\n";
-    *ofp << "//   2. Keep the waiver permanently if you are sure this is okay\n";
+    *ofp << "// Below are suggested waivers. You have three options:\n";
+    *ofp << "//   1. Fix the reason for the linter warning in the Verilog sources\n";
+    *ofp << "//   2. Keep the waiver permanently if you are sure it is okay\n";
     *ofp << "//   3. Keep the waiver temporarily to suppress the output\n\n";
 
     const V3LockGuard lock{s_mutex};

src/V3Waiver.h

@@ -30,7 +30,7 @@ class V3Waiver final {
     static WaiverList s_waiverList VL_GUARDED_BY(s_mutex);
 
 public:
-    static void addEntry(V3ErrorCode errorCode, const string& filename, const std::string& str)
+    static void addEntry(V3ErrorCode errorCode, const string& filename, const std::string& msg)
         VL_MT_SAFE_EXCLUDES(s_mutex);
     static void write(const std::string& filename) VL_MT_SAFE_EXCLUDES(s_mutex);
 };

test_regress/t/t_waiveroutput.out

@@ -2,9 +2,10 @@
 
 `verilator_config
 
-// Below you find suggested waivers. You have three options:
-//   1. Fix the reason for the linter warning
-//   2. Keep the waiver permanently if you are sure this is okay
+// Below are suggested waivers. You have three options:
+//   1. Fix the reason for the linter warning in the Verilog sources
+//   2. Keep the waiver permanently if you are sure it is okay
 //   3. Keep the waiver temporarily to suppress the output
 
-// No waivers needed - great!
+// lint_off -rule UNUSEDSIGNAL -file "*t/t_waiveroutput.v" -match "Signal is not used: 'width_warn'*"
+

test_regress/t/t_waiveroutput.py

@@ -15,7 +15,7 @@ test.top_filename = "t/t_waiveroutput.v"
 out_filename = test.obj_dir + "/" + test.name + ".waiver_gen.out"
 waiver_filename = "t/" + test.name + ".vlt"
 
-test.compile(v_flags2=[waiver_filename, '--waiver-output', out_filename])
+test.lint(v_flags2=[waiver_filename, '-Wall', '-Wno-fatal', '--waiver-output', out_filename])
 
 test.files_identical(out_filename, test.golden_filename)
 

test_regress/t/t_waiveroutput_allgood.out

@@ -2,9 +2,9 @@
 
 `verilator_config
 
-// Below you find suggested waivers. You have three options:
-//   1. Fix the reason for the linter warning
-//   2. Keep the waiver permanently if you are sure this is okay
+// Below are suggested waivers. You have three options:
+//   1. Fix the reason for the linter warning in the Verilog sources
+//   2. Keep the waiver permanently if you are sure it is okay
 //   3. Keep the waiver temporarily to suppress the output
 
 // No waivers needed - great!

test_regress/t/t_waiveroutput_allgood.py

@@ -13,9 +13,9 @@ test.scenarios('vlt')
 test.top_filename = "t/t_waiveroutput.v"
 
 out_filename = test.obj_dir + "/" + test.name + ".waiver_gen.vlt"
-waiver_filename = "t/" + test.name + ".vlt"
 
-test.compile(v_flags2=[waiver_filename, '--waiver-output', out_filename])
+# Note no Wall
+test.lint(v_flags2=['-Wno-WIDTH', '--waiver-output', out_filename])
 
 test.files_identical(out_filename, test.golden_filename)
 

test_regress/t/t_waiveroutput_allgood.vlt

@@ -1,11 +0,0 @@
-// DESCRIPTION: Verilator: Verilog Test module
-//
-// This file ONLY is placed under the Creative Commons Public Domain, for
-// any use, without warranty, 2020 by Wilson Snyder.
-// SPDX-License-Identifier: CC0-1.0
-
-`verilator_config
-
-lint_off -rule WIDTH -file "*t/t_waiveroutput.v" -match "Operator ASSIGN expects 1 bits on the Assign RHS, but Assign RHS's CONST '2'h3' generates 2 bits."
-
-lint_off -rule UNUSED -file "*t/t_waiveroutput.v" -match "Signal is not used: 'width_warn'"

test_regress/t/t_waiveroutput_multiline.out

@@ -0,0 +1,11 @@
+// DESCRIPTION: Verilator output: Waivers generated with --waiver-output
+
+`verilator_config
+
+// Below are suggested waivers. You have three options:
+//   1. Fix the reason for the linter warning in the Verilog sources
+//   2. Keep the waiver permanently if you are sure it is okay
+//   3. Keep the waiver temporarily to suppress the output
+
+// lint_off -rule UNUSEDSIGNAL -file "*t/t_waiveroutput.v" -match "Signal is not used: 'width_warn'*reg width_warn = 2'b11;*"
+

test_regress/t/t_waiveroutput_multiline.py

@@ -10,15 +10,15 @@
 import vltest_bootstrap
 
 test.scenarios('vlt')
+test.top_filename = "t/t_waiveroutput.v"
 
-out_filename = test.obj_dir + "/" + test.name + "_waiver_gen.vlt"
-waiver_filename = test.obj_dir + "/" + test.name + "_waiver.vlt"
+out_filename = test.obj_dir + "/" + test.name + ".waiver_gen.out"
+waiver_filename = "t/t_waiveroutput.vlt"
 
-test.compile(v_flags2=['--waiver-output', out_filename], fails=True)
+test.lint(v_flags2=[
+    waiver_filename, '-Wall', '-Wno-fatal', '--waiver-multiline', '--waiver-output', out_filename
+])
 
-test.file_sed(out_filename, waiver_filename,
-              lambda line: re.sub(r'\/\/ lint_off', 'lint_off', line))
-
-test.compile(v_flags2=[waiver_filename])
+test.files_identical(out_filename, test.golden_filename)
 
 test.passes()

test_regress/t/t_waiveroutput_roundtrip.py

@@ -10,13 +10,15 @@
 import vltest_bootstrap
 
 test.scenarios('vlt')
-test.top_filename = "t/t_waiveroutput.v"
 
 out_filename = test.obj_dir + "/" + test.name + ".waiver_gen.out"
-waiver_filename = "t/" + test.name + ".vlt"
+waiver_filename = test.obj_dir + "/" + test.name + "_waiver.vlt"
 
-test.compile(v_flags2=['-Wall', waiver_filename, '--waiver-output', out_filename])
+test.lint(v_flags2=['-Wall', '-Wno-fatal', '--waiver-output', out_filename])
 
-test.files_identical(out_filename, test.golden_filename)
+test.file_sed(out_filename, waiver_filename,
+              lambda line: re.sub(r'\/\/ lint_off', 'lint_off', line))
+
+test.lint(v_flags2=[waiver_filename])
 
 test.passes()

test_regress/t/t_waiveroutput_wall.out

@@ -1,10 +0,0 @@
-// DESCRIPTION: Verilator output: Waivers generated with --waiver-output
-
-`verilator_config
-
-// Below you find suggested waivers. You have three options:
-//   1. Fix the reason for the linter warning
-//   2. Keep the waiver permanently if you are sure this is okay
-//   3. Keep the waiver temporarily to suppress the output
-
-// No waivers needed - great!

test_regress/t/t_waiveroutput_wall.vlt

@@ -1,11 +0,0 @@
-// DESCRIPTION: Verilator: Verilog Test module
-//
-// This file ONLY is placed under the Creative Commons Public Domain, for
-// any use, without warranty, 2020 by Wilson Snyder.
-// SPDX-License-Identifier: CC0-1.0
-
-`verilator_config
-
-lint_off -rule WIDTH -file "*t/t_waiveroutput.v" -match "Operator ASSIGN expects 1 bits on the Assign RHS, but Assign RHS's CONST '2'h3' generates 2 bits."
-
-lint_off -rule UNUSED -file "*t/t_waiveroutput.v" -match "Signal is not used: 'width_warn'"