From ec45a77d932b8eae2094f49cbd1d62b9ef50a54b Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Thu, 2 May 2024 19:02:28 -0400 Subject: [PATCH] Fix macro expansion in strings per 1800-2023 (#5094). --- Changes | 1 + src/V3PreProc.cpp | 56 +++++++++++++++------------ test_regress/t/t_preproc.out | 20 +++++++--- test_regress/t/t_preproc.v | 12 +++++- test_regress/t/t_preproc_comments.out | 22 ++++++++--- 5 files changed, 73 insertions(+), 38 deletions(-) diff --git a/Changes b/Changes index eb6a64fc7..0868f7138 100644 --- a/Changes +++ b/Changes @@ -31,6 +31,7 @@ Verilator 5.025 devel * Fix CMake builds to export VERILATOR_ROOT (#5063). [Michael Bikovitsky] * Fix false ASSIGNIN on functions with explicit port map (#5069). * Fix DFG assertion with SystemC (#5076). [Geza Lore] +* Fix macro expansion in strings per 1800-2023 (#5094). [Geza Lore] Verilator 5.024 2024-04-05 diff --git a/src/V3PreProc.cpp b/src/V3PreProc.cpp index e930d94c7..a84c39c30 100644 --- a/src/V3PreProc.cpp +++ b/src/V3PreProc.cpp @@ -688,43 +688,43 @@ string V3PreProcImp::defineSubst(VDefineRef* refp) { { // Parse substitution define using arguments string argName; bool quote = false; + bool triquote = false; bool backslashesc = false; // In \.....{space} block // Note we go through the loop once more at the nullptr end-of-string for (const char* cp = value.c_str(); (*cp) || argName != ""; cp = (*cp ? cp + 1 : cp)) { // UINFO(4, "CH "<<*cp<<" an "<second; - if (subst == "") { - // Normally `` is removed later, but with no token after, we're otherwise - // stuck, so remove proceeding `` - if (out.size() >= 2 && out.substr(out.size() - 2) == "``") { - out = out.substr(0, out.size() - 2); + if (!quote && !triquote) { + if (std::isalpha(*cp) || *cp == '_' + || *cp == '$' // Won't replace system functions, since no $ in argValueByName + || (argName != "" && (std::isdigit(*cp) || *cp == '$'))) { + argName += *cp; + continue; + } + if (argName != "") { + // Found a possible variable substitution + const auto iter = argValueByName.find(argName); + if (iter != argValueByName.end()) { + // Substitute + const string subst = iter->second; + if (subst == "") { + // Normally `` is removed later, but with no token after, we're otherwise + // stuck, so remove proceeding `` + if (out.size() >= 2 && out.substr(out.size() - 2) == "``") { + out = out.substr(0, out.size() - 2); + } + } else { + out += subst; } } else { - out += subst; + out += argName; } - } else { - out += argName; + argName = ""; } - argName = ""; - } - if (!quote) { // Check for `` only after we've detected end-of-argname if (cp[0] == '`' && cp[1] == '`') { if (backslashesc) { @@ -772,6 +772,12 @@ string V3PreProcImp::defineSubst(VDefineRef* refp) { out += cp[0]; continue; } + if (cp[0] == '"' && cp[1] == '"' && cp[2] == '"') { + triquote = !triquote; + out += "\"\"\""; + cp += 2; + continue; + } if (*cp == '"') quote = !quote; if (*cp) out += *cp; } diff --git a/test_regress/t/t_preproc.out b/test_regress/t/t_preproc.out index ac0130d3f..f6fbc86cb 100644 --- a/test_regress/t/t_preproc.out +++ b/test_regress/t/t_preproc.out @@ -148,7 +148,7 @@ firstline comma","line LLZZ firstline comma","line `line 74 "t/t_preproc.v" 0 -x y LLZZ "x" y +x y LLZZ "a" y `line 77 "t/t_preproc.v" 0 @@ -471,7 +471,7 @@ x,y)--bee submacro has comma paren -$display("10 %d %d", $bits(foo), 10); +$display("bits %d %d", $bits(foo), 10); `line 316 "t/t_preproc.v" 0 @@ -854,13 +854,13 @@ module t; - initial $write("GOT='%s' EXP='%s'\n", "foo bar baz", "foo bar baz"); + initial $write("GOT='%s' EXP='%s'\n", "foo name baz", "foo bar baz"); - initial $write("GOT='%s' EXP='%s'\n", "foo `A(bar) baz", "foo `A(bar) baz"); + initial $write("GOT='%s' EXP='%s'\n", "foo name baz", "foo `A(bar) baz"); @@ -1030,6 +1030,16 @@ Second line""" `line 711 "t/t_preproc.v" 0 + +`line 714 "t/t_preproc.v" 0 + +bar "foo foo foo" bar + +bar """foo foo foo""" bar + +`line 719 "t/t_preproc.v" 0 + + predef 0 0 predef 1 1 @@ -1050,4 +1060,4 @@ predef 2 2 -`line 733 "t/t_preproc.v" 0 +`line 741 "t/t_preproc.v" 0 diff --git a/test_regress/t/t_preproc.v b/test_regress/t/t_preproc.v index d07a42994..0659316ad 100644 --- a/test_regress/t/t_preproc.v +++ b/test_regress/t/t_preproc.v @@ -72,7 +72,7 @@ Line_Preproc_Check `__LINE__ comma","line) `define withquote(a, bar) a bar LLZZ "a" bar -`withquote( x , y) // Simulators disagree here; some substitute "a" others do not +`withquote( x , y) // IEEE 1800-2023 clarified that "a" not to substitute `define noparam (a,b) `noparam(a,b) @@ -566,7 +566,7 @@ module t; `undef DEF_NO_EXPAND //----- // bug441 derivative - // SHOULD(simulator-dependant): Quotes doesn't prevent arguments from expanding (like backslashes above) + // Clarified in IEEE 1800-2023: Quotes prevent arguments from expanding `define STR(name) "foo name baz" initial $write("GOT='%s' EXP='%s'\n", `STR(bar), "foo bar baz"); `undef STR @@ -708,6 +708,14 @@ Second line""" `define IDENTITY(arg) ``arg `IDENTITY("string argument") +//====================================================================== +// See issue #5094 - IEEE 1800-2023 clarified proper behavior + +`define MAC_WITH_STR(foo) foo "foo foo foo" foo +`MAC_WITH_STR(bar) +`define MAC_WITH_3STR(foo) foo """foo foo foo""" foo +`MAC_WITH_3STR(bar) + //====================================================================== // IEEE mandated predefines `undefineall // undefineall should have no effect on these diff --git a/test_regress/t/t_preproc_comments.out b/test_regress/t/t_preproc_comments.out index aad586f77..4c6cf38bb 100644 --- a/test_regress/t/t_preproc_comments.out +++ b/test_regress/t/t_preproc_comments.out @@ -148,7 +148,7 @@ firstline comma","line LLZZ firstline comma","line `line 74 "t/t_preproc.v" 0 -x y LLZZ "x" y // Simulators disagree here; some substitute "a" others do not +x y LLZZ "a" y // IEEE 1800-2023 clarified that "a" not to substitute `line 77 "t/t_preproc.v" 0 @@ -471,7 +471,7 @@ x,y)--bee submacro has comma paren //====================================================================== // bug191 -$display("10 %d %d", $bits(foo), 10); +$display("bits %d %d", $bits(foo), 10); `line 316 "t/t_preproc.v" 0 //====================================================================== @@ -857,15 +857,15 @@ module t; //----- // bug441 derivative - // SHOULD(simulator-dependant): Quotes doesn't prevent arguments from expanding (like backslashes above) + // Clarified in IEEE 1800-2023: Quotes prevent arguments from expanding - initial $write("GOT='%s' EXP='%s'\n", "foo bar baz", "foo bar baz"); + initial $write("GOT='%s' EXP='%s'\n", "foo name baz", "foo bar baz"); //----- // RULE: Because there are quotes after substituting STR, the `A does NOT expand - initial $write("GOT='%s' EXP='%s'\n", "foo `A(bar) baz", "foo `A(bar) baz"); + initial $write("GOT='%s' EXP='%s'\n", "foo name baz", "foo `A(bar) baz"); //---- // bug845 @@ -1034,6 +1034,16 @@ Second line""" `line 711 "t/t_preproc.v" 0 //====================================================================== +// See issue #5094 - IEEE 1800-2023 clarified proper behavior + +`line 714 "t/t_preproc.v" 0 + +bar "foo foo foo" bar + +bar """foo foo foo""" bar + +`line 719 "t/t_preproc.v" 0 +//====================================================================== // IEEE mandated predefines // undefineall should have no effect on these predef 0 0 @@ -1055,4 +1065,4 @@ predef 2 2 // After `undefineall above, for testing --dump-defines -`line 733 "t/t_preproc.v" 0 +`line 741 "t/t_preproc.v" 0