From 221e4ff6fe5ce002bbd422ad5c25ac7eace70c72 Mon Sep 17 00:00:00 2001
From: Wilson Snyder <wsnyder@wsnyder.org>
Date: Sat, 7 Oct 2017 14:01:18 -0400
Subject: [PATCH] Fix `` expansion of empty `defines, bug1225.

---
 Changes                      |  2 +
 src/V3PreProc.cpp            | 36 +++++++++++++-
 test_regress/t/t_preproc.out | 94 +++++++++++++++++++++++++++++++++++-
 test_regress/t/t_preproc.v   | 77 +++++++++++++++++++++++++++++
 4 files changed, 207 insertions(+), 2 deletions(-)

diff --git a/Changes b/Changes
index 88711eb62..a0fba546c 100644
--- a/Changes
+++ b/Changes
@@ -29,6 +29,8 @@ The contributors that suggested a given feature are shown in []. Thanks!
 
 ****  Fix conditional slices and add related optimizations.
 
+****  Fix `` expansion of empty `defines, bug1225. [Odd Magne Reitan]
+
 
 * Verilator 3.912 2017-09-23
 
diff --git a/src/V3PreProc.cpp b/src/V3PreProc.cpp
index 744fb6508..c30f89763 100644
--- a/src/V3PreProc.cpp
+++ b/src/V3PreProc.cpp
@@ -205,6 +205,13 @@ private:
     int getFinalToken(string& buf);
 
     ProcState state() const { return m_states.top(); }
+    bool stateIsDefname() const {
+        return state()==ps_DEFNAME_UNDEF
+            || state()==ps_DEFNAME_DEFINE
+            || state()==ps_DEFNAME_IFDEF
+            || state()==ps_DEFNAME_IFNDEF
+            || state()==ps_DEFNAME_ELSIF;
+    }
     void statePush(ProcState state) {
 	m_states.push(state);
     }
@@ -644,7 +651,15 @@ string V3PreProcImp::defineSubst(V3DefineRef* refp) {
 		if (iter != argValueByName.end()) {
 		    // Substitute
 		    string subst = iter->second;
-		    out += subst;
+		    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 += argName;
 		}
@@ -657,6 +672,7 @@ string V3PreProcImp::defineSubst(V3DefineRef* refp) {
 			// Don't put out the ``, we're forming an escape which will not expand further later
 		    } else {
 			out += "``";   // `` must get removed later, as `FOO```BAR must pre-expand FOO and BAR
+			// See also removal in empty substitutes above
 		    }
 		    cp++;
 		    continue;
@@ -1079,7 +1095,16 @@ int V3PreProcImp::getStateToken() {
 		m_defRefs.pop(); VL_DANGLING(refp);
 		if (m_defRefs.empty()) {
 		    statePop();
+		    if (state() == ps_JOIN) {  // Handle {left}```FOO(ARG) where `FOO(ARG) might be empty
+			if (m_joinStack.empty()) fatalSrc("`` join stack empty, but in a ``");
+			string lhs = m_joinStack.top(); m_joinStack.pop();
+			out = lhs+out;
+			UINFO(5,"``-end-defarg Out:"<<out<<endl);
+			statePop();
+		    }
 		    if (!m_off) unputDefrefString(out);
+		    // Prevent problem when EMPTY="" in `ifdef NEVER `define `EMPTY
+		    else if (stateIsDefname()) unputDefrefString("__IF_OFF_IGNORED_DEFINE");
 		    m_lexp->m_parenLevel = 0;
 		}
 		else {  // Finished a defref inside a upper defref
@@ -1287,7 +1312,16 @@ int V3PreProcImp::getStateToken() {
 		    //NOP: out = m_preprocp->defSubstitute(out);
 		    if (m_defRefs.empty()) {
 			// Just output the substitution
+			if (state() == ps_JOIN) {  // Handle {left}```FOO where `FOO might be empty
+			    if (m_joinStack.empty()) fatalSrc("`` join stack empty, but in a ``");
+			    string lhs = m_joinStack.top(); m_joinStack.pop();
+			    out = lhs+out;
+			    UINFO(5,"``-end-defref Out:"<<out<<endl);
+			    statePop();
+			}
 			if (!m_off) unputDefrefString(out);
+			// Prevent problem when EMPTY="" in `ifdef NEVER `define `EMPTY
+			else if (stateIsDefname()) unputDefrefString("__IF_OFF_IGNORED_DEFINE");
 		    } else {
 			// Inside another define.
 			// Can't subst now, or
diff --git a/test_regress/t/t_preproc.out b/test_regress/t/t_preproc.out
index 0234528f6..34d43330f 100644
--- a/test_regress/t/t_preproc.out
+++ b/test_regress/t/t_preproc.out
@@ -800,6 +800,98 @@ module t;
 `line 531 "t/t_preproc.v" 0
        $display("%s%s","a1","b2c3\n");
 endmodule
+
+`line 534 "t/t_preproc.v" 0
+ 
+ 
+
+`line 537 "t/t_preproc.v" 0
+ 
+ 
+$display("RAM0");
+$display("CPU");
+
+`line 542 "t/t_preproc.v" 0
+ 
+ 
+ 
+ 
+
+`line 547 "t/t_preproc.v" 0
+ 
+XXE_FAMILY = XXE_
+ 
+ 
+     $display("XXE_ is defined");
+
+
+`line 554 "t/t_preproc.v" 0
+ 
+XYE_FAMILY = XYE_
+ 
+ 
+     $display("XYE_ is defined");
+
+
+`line 561 "t/t_preproc.v" 0
+ 
+XXS_FAMILY = XXS_some
+ 
+ 
+     $display("XXS_some is defined");
+
+
+`line 568 "t/t_preproc.v" 0
+ 
+XYS_FAMILY = XYS_foo
+ 
+ 
+     $display("XYS_foo is defined");
+
+
+`line 575 "t/t_preproc.v" 0
+ 
+
+`line 577 "t/t_preproc.v" 0
+ 
+  
+  
+  
+  
+     
+ 
+
+`line 585 "t/t_preproc.v" 0
+  
+  
+  
+  
+     
+ 
+
+`line 592 "t/t_preproc.v" 0
+  
+  
+  
+  
+     
+ 
+
+`line 599 "t/t_preproc.v" 0
+  
+  
+  
+  
+     
+ 
+
+`line 606 "t/t_preproc.v" 0
+  
+
+`line 608 "t/t_preproc.v" 0
+  
+
+`line 610 "t/t_preproc.v" 0
  
  
    
@@ -820,4 +912,4 @@ predef 1 1
 predef 2 2
  
 
-`line 553 "t/t_preproc.v" 2
+`line 630 "t/t_preproc.v" 2
diff --git a/test_regress/t/t_preproc.v b/test_regress/t/t_preproc.v
index 54f896b41..0111fdc19 100644
--- a/test_regress/t/t_preproc.v
+++ b/test_regress/t/t_preproc.v
@@ -530,6 +530,83 @@ module t;
        $display("%s%s",a,`"b``c``\n`")
    initial `BUG915("a1",b2,c3);
 endmodule
+
+//======================================================================
+//bug1225
+
+`define X_ITEM(SUB,UNIT) `X_STRING(SUB``UNIT)
+`define X_STRING(A) `"A`"
+$display(`X_ITEM(RAM,0));
+$display(`X_ITEM(CPU,));
+
+`define EMPTY
+`define EMPTYP(foo)
+`define SOME some
+`define SOMEP(foo) foo
+
+`define XXE_FAMILY XXE_```EMPTY
+XXE_FAMILY = `XXE_FAMILY
+`define XXE_```EMPTY
+`ifdef XXE_
+     $display("XXE_ is defined");
+`endif
+
+`define XYE_FAMILY XYE_```EMPTYP(foo)
+XYE_FAMILY = `XYE_FAMILY
+`define XYE_```EMPTYP(foo)
+`ifdef XYE_
+     $display("XYE_ is defined");
+`endif
+
+`define XXS_FAMILY XXS_```SOME
+XXS_FAMILY = `XXS_FAMILY
+`define XXS_```SOME
+`ifdef XXS_some
+     $display("XXS_some is defined");
+`endif
+
+`define XYS_FAMILY XYS_```SOMEP(foo)
+XYS_FAMILY = `XYS_FAMILY
+`define XYS_```SOMEP(foo)
+`ifdef XYS_foo
+     $display("XYS_foo is defined");
+`endif
+
+//====
+
+`ifdef NEVER
+ `define NXE_FAMILY NXE_```EMPTY
+NXE_FAMILY = `NXE_FAMILY
+ `define NXE_```EMPTY
+ `ifdef NXE_
+     $display("NXE_ is defined");
+ `endif
+
+ `define NYE_FAMILY NYE_```EMPTYP(foo)
+NYE_FAMILY = `NYE_FAMILY
+ `define NYE_```EMPTYP(foo)
+ `ifdef NYE_
+     $display("NYE_ is defined");
+ `endif
+
+ `define NXS_FAMILY NXS_```SOME
+NXS_FAMILY = `NXS_FAMILY
+ `define NXS_```SOME
+ `ifdef NXS_some
+     $display("NXS_some is defined");
+ `endif
+
+ `define NYS_FAMILY NYS_```SOMEP(foo)
+NYS_FAMILY = `NYS_FAMILY
+ `define NYS_```SOMEP(foo)
+ `ifdef NYS_foo
+     $display("NYS_foo is defined");
+ `endif
+
+ `include `EMPTY
+
+`endif // NEVER
+
 //======================================================================
 // IEEE mandated predefines
 `undefineall  // undefineall should have no effect on these