From d9dcde60a6581ab80b1d55bbb43a0b4214f5a5c3 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 10 May 2025 13:29:30 -0400 Subject: [PATCH] Fix duplicate error first-lines, and some internal V3Error cleanups --- src/V3Error.cpp | 36 +++++++----- src/V3Error.h | 7 ++- src/V3LinkLevel.cpp | 6 +- test_regress/t/t_generate_fatal_bad.out | 77 ------------------------- test_regress/t/t_pp_line_bad.out | 6 -- 5 files changed, 29 insertions(+), 103 deletions(-) diff --git a/src/V3Error.cpp b/src/V3Error.cpp index 2d0d30688..1acd8e601 100644 --- a/src/V3Error.cpp +++ b/src/V3Error.cpp @@ -49,6 +49,14 @@ V3ErrorCode::V3ErrorCode(const char* msgp) { m_e = V3ErrorCode::EC_ERROR; } +string V3ErrorCode::url() const { + if (m_e < V3ErrorCode::EC_FIRST_NAMED) { + return "https://verilator.org/verilator_doc.html"s + "?v=" + PACKAGE_VERSION_NUMBER_STRING; + } else { + return "https://verilator.org/warn/"s + ascii() + "?v=" + PACKAGE_VERSION_NUMBER_STRING; + } +} + //###################################################################### // V3ErrorGuarded class functions // @@ -128,14 +136,18 @@ void V3ErrorGuarded::v3errorEnd(std::ostringstream& sstr, const string& extra) && (!debug() || debug() < 3 || m_errorCode.defaultsOff())) return; string msg = msgPrefix() + sstr.str(); + // If suppressed print only first line to reduce verbosity - if (m_errorSuppressed) { - string::size_type pos; - if ((pos = msg.find('\n')) != string::npos) { - msg.erase(pos, msg.length() - pos); - msg += "..."; - } + string firstLine = msg; + string::size_type pos; + if ((pos = firstLine.find('\n')) != string::npos) { + firstLine.erase(pos, firstLine.length() - pos); + firstLine += "..."; } + if (m_errorSuppressed) msg = firstLine; + // Suppress duplicate messages + if (!m_messages.insert(firstLine).second) return; + string msg_additional; { string::size_type pos; @@ -152,8 +164,6 @@ void V3ErrorGuarded::v3errorEnd(std::ostringstream& sstr, const string& extra) while ((pos = msg_additional.find("\n\n")) != string::npos) msg_additional.erase(pos + 1, 1); } - // Suppress duplicate messages - if (!m_messages.insert(msg).second) return; if (!extra.empty() && !m_errorSuppressed) { const string extraMsg = warnMore() + extra + "\n"; const size_t pos = msg.find('\n'); @@ -175,13 +185,9 @@ void V3ErrorGuarded::v3errorEnd(std::ostringstream& sstr, const string& extra) if (m_errorCode != V3ErrorCode::EC_FATALMANY // Not verbose on final too-many-errors error && !m_describedEachWarn[m_errorCode]) { m_describedEachWarn[m_errorCode] = true; - const string docUrl = "https://verilator.org/verilator_doc.html"s - + "?v=" + PACKAGE_VERSION_NUMBER_STRING; - const string warnUrl = "https://verilator.org/warn/"s + m_errorCode.ascii() - + "?v=" + PACKAGE_VERSION_NUMBER_STRING; if (m_errorCode >= V3ErrorCode::EC_FIRST_NAMED) { std::cerr << warnMore() << "... For " << (anError ? "error" : "warning") - << " description see " << warnUrl << endl; + << " description see " << m_errorCode.url() << endl; } else if (m_errCount >= 1 && (m_errorCode == V3ErrorCode::EC_FATAL || m_errorCode == V3ErrorCode::EC_FATALMANY @@ -194,7 +200,7 @@ void V3ErrorGuarded::v3errorEnd(std::ostringstream& sstr, const string& extra) << endl; } else if (!m_tellManual) { m_tellManual = true; - std::cerr << warnMore() << "... See the manual at " << docUrl + std::cerr << warnMore() << "... See the manual at " << m_errorCode.url() << " for more assistance." << endl; } if (!m_pretendError[m_errorCode] && !m_errorCode.hardError()) { @@ -202,7 +208,7 @@ void V3ErrorGuarded::v3errorEnd(std::ostringstream& sstr, const string& extra) << m_errorCode.ascii() << " */\" and lint_on around source to disable this message." << endl; if (m_errorCode.dangerous()) { - std::cerr << warnMore() << "*** See " << warnUrl + std::cerr << warnMore() << "*** See " << m_errorCode.url() << " before disabling this,\n"; std::cerr << warnMore() << "else you may end up with different sim results." << endl; diff --git a/src/V3Error.h b/src/V3Error.h index 591128e8e..fdae76e3a 100644 --- a/src/V3Error.h +++ b/src/V3Error.h @@ -34,6 +34,9 @@ #include #include +class V3Error; +class FileLine; + //###################################################################### class V3ErrorCode final { @@ -289,7 +292,7 @@ public: } return false; } - + string url() const; static bool unusedMsg(const char* msgp) { return 0 == VL_STRCASECMP(msgp, "UNUSED"); } }; constexpr bool operator==(const V3ErrorCode& lhs, const V3ErrorCode& rhs) { @@ -302,7 +305,6 @@ inline std::ostream& operator<<(std::ostream& os, const V3ErrorCode& rhs) { } // ###################################################################### -class V3Error; class V3ErrorGuarded final { // Should only be used by V3ErrorGuarded::m_mutex is already locked @@ -409,6 +411,7 @@ public: }; // ###################################################################### + class V3Error final { // Base class for any object that wants debugging and error reporting // CONSTRUCTORS diff --git a/src/V3LinkLevel.cpp b/src/V3LinkLevel.cpp index e90eda0a0..fce12adaf 100644 --- a/src/V3LinkLevel.cpp +++ b/src/V3LinkLevel.cpp @@ -53,11 +53,11 @@ void V3LinkLevel::modSortByLevel() { if (tops.size() >= 2) { const AstNode* const secp = tops[1]; // Complain about second one, as first often intended if (!secp->fileline()->warnIsOff(V3ErrorCode::MULTITOP)) { - auto warnTopModules = [](const std::string& warnMore, ModVec tops) + auto warnTopModules = [](const AstNode* const secp, ModVec tops) VL_REQUIRES(V3Error::s().m_mutex) -> std::string { std::stringstream ss; for (AstNode* alsop : tops) { - ss << warnMore << "... Top module " << alsop->prettyNameQ() << endl + ss << secp->warnMore() << "... Top module " << alsop->prettyNameQ() << endl << alsop->warnContextSecondary(); } return ss.str(); @@ -69,7 +69,7 @@ void V3LinkLevel::modSortByLevel() { "--top-module to select top." << V3Error::s().warnContextNone() << V3Error::warnAdditionalInfo() - << warnTopModules(secp->warnMore(), tops)); + << warnTopModules(secp, tops)); } } diff --git a/test_regress/t/t_generate_fatal_bad.out b/test_regress/t/t_generate_fatal_bad.out index d657d50bb..aa62b75d5 100644 --- a/test_regress/t/t_generate_fatal_bad.out +++ b/test_regress/t/t_generate_fatal_bad.out @@ -9,81 +9,4 @@ 13 | localparam integer BAZ = get_baz(BAR); | ^~~~~~~ ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. -%Error: t/t_generate_fatal_bad.v:13:29: Expecting expression to be constant, but can't determine constant for FUNCREF 'get_baz' - : ... note: In instance 't.genloop[1].foo_inst' - t/t_generate_fatal_bad.v:9:4: ... Location of non-constant STOP: $stop executed during function constification; maybe indicates assertion firing - t/t_generate_fatal_bad.v:13:29: ... Called from 'get_baz()' with parameters: - bar = ?32?h1 - 13 | localparam integer BAZ = get_baz(BAR); - | ^~~~~~~ -%Error: t/t_generate_fatal_bad.v:13:29: Expecting expression to be constant, but can't determine constant for FUNCREF 'get_baz' - : ... note: In instance 't.gen_l1[2].gen_l2[0].foo_inst2' - t/t_generate_fatal_bad.v:9:4: ... Location of non-constant STOP: $stop executed during function constification; maybe indicates assertion firing - t/t_generate_fatal_bad.v:13:29: ... Called from 'get_baz()' with parameters: - bar = 32'h2 - 13 | localparam integer BAZ = get_baz(BAR); - | ^~~~~~~ -%Error: t/t_generate_fatal_bad.v:13:29: Expecting expression to be constant, but can't determine constant for FUNCREF 'get_baz' - : ... note: In instance 't.gen_l1[2].gen_l2[1].foo_inst2' - t/t_generate_fatal_bad.v:9:4: ... Location of non-constant STOP: $stop executed during function constification; maybe indicates assertion firing - t/t_generate_fatal_bad.v:13:29: ... Called from 'get_baz()' with parameters: - bar = 32'h4 - 13 | localparam integer BAZ = get_baz(BAR); - | ^~~~~~~ -%Error: t/t_generate_fatal_bad.v:13:29: Expecting expression to be constant, but can't determine constant for FUNCREF 'get_baz' - : ... note: In instance 't.gen_l1[3].gen_l2[0].foo_inst2' - t/t_generate_fatal_bad.v:9:4: ... Location of non-constant STOP: $stop executed during function constification; maybe indicates assertion firing - t/t_generate_fatal_bad.v:13:29: ... Called from 'get_baz()' with parameters: - bar = 32'h3 - 13 | localparam integer BAZ = get_baz(BAR); - | ^~~~~~~ -%Error: t/t_generate_fatal_bad.v:13:29: Expecting expression to be constant, but can't determine constant for FUNCREF 'get_baz' - : ... note: In instance 't.gen_l1[3].gen_l2[1].foo_inst2' - t/t_generate_fatal_bad.v:9:4: ... Location of non-constant STOP: $stop executed during function constification; maybe indicates assertion firing - t/t_generate_fatal_bad.v:13:29: ... Called from 'get_baz()' with parameters: - bar = 32'h5 - 13 | localparam integer BAZ = get_baz(BAR); - | ^~~~~~~ -%Error: t/t_generate_fatal_bad.v:13:29: Expecting expression to be constant, but can't determine constant for FUNCREF 'get_baz' - : ... note: In instance 't.cond_true.foo_inst3' - t/t_generate_fatal_bad.v:9:4: ... Location of non-constant STOP: $stop executed during function constification; maybe indicates assertion firing - t/t_generate_fatal_bad.v:13:29: ... Called from 'get_baz()' with parameters: - bar = ?32?h6 - 13 | localparam integer BAZ = get_baz(BAR); - | ^~~~~~~ -%Error: t/t_generate_fatal_bad.v:13:29: Expecting expression to be constant, but can't determine constant for FUNCREF 'get_baz' - : ... note: In instance 't.genblk4.foo_inst4' - t/t_generate_fatal_bad.v:9:4: ... Location of non-constant STOP: $stop executed during function constification; maybe indicates assertion firing - t/t_generate_fatal_bad.v:13:29: ... Called from 'get_baz()' with parameters: - bar = ?32?h7 - 13 | localparam integer BAZ = get_baz(BAR); - | ^~~~~~~ -%Error: t/t_generate_fatal_bad.v:13:29: Expecting expression to be constant, but can't determine constant for FUNCREF 'get_baz' - : ... note: In instance 't.nested_loop[8].foo2_inst.foo2_loop[0].foo_in_foo2_inst' - t/t_generate_fatal_bad.v:9:4: ... Location of non-constant STOP: $stop executed during function constification; maybe indicates assertion firing - t/t_generate_fatal_bad.v:13:29: ... Called from 'get_baz()' with parameters: - bar = 32'h8 - 13 | localparam integer BAZ = get_baz(BAR); - | ^~~~~~~ -%Error: t/t_generate_fatal_bad.v:13:29: Expecting expression to be constant, but can't determine constant for FUNCREF 'get_baz' - : ... note: In instance 't.nested_loop[8].foo2_inst.foo2_loop[1].foo_in_foo2_inst' - t/t_generate_fatal_bad.v:9:4: ... Location of non-constant STOP: $stop executed during function constification; maybe indicates assertion firing - t/t_generate_fatal_bad.v:13:29: ... Called from 'get_baz()' with parameters: - bar = 32'h9 - 13 | localparam integer BAZ = get_baz(BAR); - | ^~~~~~~ -%Error: t/t_generate_fatal_bad.v:13:29: Expecting expression to be constant, but can't determine constant for FUNCREF 'get_baz' - : ... note: In instance 't.nested_loop[10].foo2_inst.foo2_loop[0].foo_in_foo2_inst' - t/t_generate_fatal_bad.v:9:4: ... Location of non-constant STOP: $stop executed during function constification; maybe indicates assertion firing - t/t_generate_fatal_bad.v:13:29: ... Called from 'get_baz()' with parameters: - bar = 32'ha - 13 | localparam integer BAZ = get_baz(BAR); - | ^~~~~~~ -%Error: t/t_generate_fatal_bad.v:13:29: Expecting expression to be constant, but can't determine constant for FUNCREF 'get_baz' - : ... note: In instance 't.nested_loop[10].foo2_inst.foo2_loop[1].foo_in_foo2_inst' - t/t_generate_fatal_bad.v:9:4: ... Location of non-constant STOP: $stop executed during function constification; maybe indicates assertion firing - t/t_generate_fatal_bad.v:13:29: ... Called from 'get_baz()' with parameters: - bar = 32'hb - 13 | localparam integer BAZ = get_baz(BAR); - | ^~~~~~~ %Error: Exiting due to diff --git a/test_regress/t/t_pp_line_bad.out b/test_regress/t/t_pp_line_bad.out index d2f6c2291..adce0e39f 100644 --- a/test_regress/t/t_pp_line_bad.out +++ b/test_regress/t/t_pp_line_bad.out @@ -2,27 +2,21 @@ 8 | `line 100 | ^ ... See the manual at https://verilator.org/verilator_doc.html?v=latest for more assistance. -%Error: t/t_pp_line_bad.v:8:1: `line was not properly formed with '`line number "filename" level' %Error: t/t_pp_line_bad.v:9:1: `line was not properly formed with '`line number "filename" level' 9 | `line 200 somefile | ^ -%Error: t/t_pp_line_bad.v:9:1: `line was not properly formed with '`line number "filename" level' %Error: t/t_pp_line_bad.v:10:1: `line was not properly formed with '`line number "filename" level' 10 | `line 300 "somefile 1 | ^ -%Error: t/t_pp_line_bad.v:10:1: `line was not properly formed with '`line number "filename" level' %Error: t/t_pp_line_bad.v:11:1: `line was not properly formed with '`line number "filename" level' 11 | `line 400 "some file" | ^ -%Error: t/t_pp_line_bad.v:11:1: `line was not properly formed with '`line number "filename" level' %Error: t/t_pp_line_bad.v:12:1: `line was not properly formed with '`line number "filename" level' 12 | `line 500 "somefile" 3 | ^ -%Error: t/t_pp_line_bad.v:12:1: `line was not properly formed with '`line number "filename" level' %Error: t/t_pp_line_bad.v:13:1: `line was not properly formed with '`line number "filename" level' 13 | `line 600 "some file" 3 | ^ -%Error: t/t_pp_line_bad.v:13:1: `line was not properly formed with '`line number "filename" level' %Error: t/t_pp_line_bad.v:7:1: Define or directive not defined: '`line' 7 | `line | ^~~~~