Fix macro expansion in strings per 1800-2023 (#5094).

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

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 "<<argName<<endl);
-            if (!quote && *cp == '\\') {
+            if (!quote && !triquote && *cp == '\\') {
                 backslashesc = true;
             } else if (std::isspace(*cp)) {
                 backslashesc = false;
             }
-            // We don't check for quotes; some simulators expand even inside quotes
+            if (!quote && !triquote) {
-            if (std::isalpha(*cp) || *cp == '_'
+                if (std::isalpha(*cp) || *cp == '_'
-                || *cp == '$'  // Won't replace system functions, since no $ in argValueByName
+                    || *cp == '$'  // Won't replace system functions, since no $ in argValueByName
-                || (argName != "" && (std::isdigit(*cp) || *cp == '$'))) {
+                    || (argName != "" && (std::isdigit(*cp) || *cp == '$'))) {
-                argName += *cp;
+                    argName += *cp;
-                continue;
+                    continue;
-            }
+                }
-            if (argName != "") {
+                if (argName != "") {
-                // Found a possible variable substitution
+                    // Found a possible variable substitution
-                const auto iter = argValueByName.find(argName);
+                    const auto iter = argValueByName.find(argName);
-                if (iter != argValueByName.end()) {
+                    if (iter != argValueByName.end()) {
-                    // Substitute
+                        // Substitute
-                    const string subst = iter->second;
+                        const string subst = iter->second;
-                    if (subst == "") {
+                        if (subst == "") {
-                        // Normally `` is removed later, but with no token after, we're otherwise
+                            // Normally `` is removed later, but with no token after, we're otherwise
-                        // stuck, so remove proceeding ``
+                            // stuck, so remove proceeding ``
-                        if (out.size() >= 2 && out.substr(out.size() - 2) == "``") {
+                            if (out.size() >= 2 && out.substr(out.size() - 2) == "``") {
-                            out = out.substr(0, out.size() - 2);
+                                out = out.substr(0, out.size() - 2);
+                            }
+                        } else {
+                            out += subst;
                         }
                     } else {
-                        out += subst;
+                        out += argName;
                     }
-                } else {
+                    argName = "";
-                    out += 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;
         }

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

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

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