Fix empty string literals converting to string types (#3774).

Changes

@@ -27,6 +27,7 @@ Verilator 5.003 devel
 * Fix tracing parameters overridden with -G (#3723). [Iztok Jeras]
 * Fix wait 0.
 * Fix comparing ranged slices of unpacked arrays.
+* Fix empty string literals converting to string types (#3774). [miree]
 
 
 Verilator 5.002 2022-10-29

include/verilated.cpp

@@ -1812,16 +1812,14 @@ std::string VL_CVT_PACK_STR_NW(int lwords, const WDataInP lwp) VL_MT_SAFE {
     char destout[VL_VALUE_STRING_MAX_CHARS + 1];
     const int obits = lwords * VL_EDATASIZE;
     int lsb = obits - 1;
-    bool start = true;
     char* destp = destout;
     size_t len = 0;
     for (; lsb >= 0; --lsb) {
         lsb = (lsb / 8) * 8;  // Next digit
         const IData charval = VL_BITRSHIFT_W(lwp, lsb) & 0xff;
-        if (!start || charval) {
-            *destp++ = (charval == 0) ? ' ' : charval;
+        if (charval) {
+            *destp++ = static_cast<char>(charval);
             ++len;
-            start = false;  // Drop leading 0s
         }
     }
     return std::string{destout, len};

src/V3Number.cpp

@@ -100,15 +100,19 @@ void V3Number::v3errorEndFatal(const std::ostringstream& str) const VL_MT_SAFE {
 
 V3Number::V3Number(VerilogStringLiteral, AstNode* nodep, const string& str) {
     // Create a number using a verilog string as the value, thus 8 bits per character.
-    init(nodep, std::max<int>(str.length() * 8, 1));
-    m_data.m_fromString = true;
-    for (unsigned pos = 0; pos < str.length(); ++pos) {
-        const int topos = str.length() - 1 - pos;
-        ValueAndX& v = m_data.num()[topos / 4];
-        for (int bit = 0; bit < 8; ++bit) {
-            if (str[pos] & (1UL << bit)) { v.m_value |= (1UL << (bit + (topos % 4) * 8)); }
+    if (str.empty()) {  // IEEE 1800-2017 11.10.3 "" = "\000"
+        init(nodep, 8);
+    } else {
+        init(nodep, std::max<int>(str.length() * 8, 1));
+        for (unsigned pos = 0; pos < str.length(); ++pos) {
+            const int topos = str.length() - 1 - pos;
+            ValueAndX& v = m_data.num()[topos / 4];
+            for (int bit = 0; bit < 8; ++bit) {
+                if (str[pos] & (1UL << bit)) { v.m_value |= (1UL << (bit + (topos % 4) * 8)); }
+            }
         }
     }
+    m_data.m_fromString = true;
     opCleanThis(true);
 }
 
@@ -948,18 +952,15 @@ int64_t V3Number::toSQuad() const {
 
 string V3Number::toString() const {
     UASSERT(!isFourState(), "toString with 4-state " << *this);
-    // Spec says always drop leading zeros, this isn't quite right, we space pad.
+    // IEEE says \000 are removed
+    // If used to print a %s space padding is still required after call to here
     if (isString()) return m_data.str();
     int bit = width() - 1;
-    bool start = true;
-    while ((bit % 8) != 7) bit++;
-    string str;
+    while ((bit % 8) != 7) ++bit;
+    std::string str;
     for (; bit >= 0; bit -= 8) {
         const int v = bitsValue(bit - 7, 8);
-        if (!start || v) {
-            str += static_cast<char>((v == 0) ? ' ' : v);
-            start = false;  // Drop leading 0s
-        }
+        if (v) str += static_cast<char>(v);
     }
     return str;
 }

src/V3Width.cpp

@@ -374,7 +374,15 @@ private:
     void visit(AstRealToBits* nodep) override { visit_Ou64_Lr(nodep); }
 
     // Output integer, input string
-    void visit(AstLenN* nodep) override { visit_Os32_string(nodep); }
+    void visit(AstLenN* nodep) override {
+        // Widths: 32 bit out
+        UASSERT_OBJ(nodep->lhsp(), nodep, "For unary ops only!");
+        if (m_vup->prelim()) {
+            // See similar handling in visit_cmp_eq_gt where created
+            iterateCheckString(nodep, "LHS", nodep->lhsp(), BOTH);
+            nodep->dtypeSetSigned32();
+        }
+    }
     void visit(AstPutcN* nodep) override {
         // CALLER: str.putc()
         UASSERT_OBJ(nodep->rhsp() && nodep->thsp(), nodep, "For ternary ops only!");
@@ -5556,17 +5564,6 @@ private:
         }
     }
 
-    void visit_Os32_string(AstNodeUniop* nodep) {
-        // CALLER: LenN
-        // Widths: 32 bit out
-        UASSERT_OBJ(nodep->lhsp(), nodep, "For unary ops only!");
-        if (m_vup->prelim()) {
-            // See similar handling in visit_cmp_eq_gt where created
-            iterateCheckString(nodep, "LHS", nodep->lhsp(), BOTH);
-            nodep->dtypeSetSigned32();
-        }
-    }
-
     void visit_negate_not(AstNodeUniop* nodep, bool real_ok) {
         // CALLER: (real_ok=false) Not
         // CALLER: (real_ok=true) Negate - allow real numbers

src/verilog.y

@@ -5363,15 +5363,8 @@ str<strp>:                      // yaSTRING but with \{escapes} need decoded
 
 strAsInt<nodeExprp>:
                 yaSTRING
-                        { if ($1->empty()) {
-                              // else "" is not representable as number as is width 0
-                              // TODO all strings should be represented this way
-                              // until V3Width converts as/if needed to a numerical constant
-                              $$ = new AstConst{$<fl>1, AstConst::String{}, GRAMMARP->deQuote($<fl>1, *$1)};
-                          } else {
-                              $$ = new AstConst{$<fl>1, AstConst::VerilogStringLiteral{}, GRAMMARP->deQuote($<fl>1, *$1)};
-                          }
-                        }
+                        { // Numeric context, so IEEE 1800-2017 11.10.3 "" is a "\000"
+                          $$ = new AstConst{$<fl>1, AstConst::VerilogStringLiteral{}, GRAMMARP->deQuote($<fl>1, *$1)}; }
         ;
 
 strAsIntIgnore<nodeExprp>:          // strAsInt, but never matches for when expr shouldn't parse strings

test_regress/t/t_display.out

@@ -65,7 +65,7 @@ hello, from a concatenated string.
 hello, from a concatenated format string [0].
 extra argument:                    0
                    0 : pre argument after
-empty: ><
+empty: > <
 [0] Embedded tab '	' and <#013> return
 [0] Embedded
 multiline

test_regress/t/t_string_dyn_num.out

@@ -0,0 +1,4 @@
+istr='    ' istr%0=' ' sstr=''
+istr='  A ' istr%0='A ' sstr='A'
+istr='B A ' istr%0='B A ' sstr='BA'
+*-* All Finished *-*

test_regress/t/t_string_dyn_num.pl

@@ -0,0 +1,22 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2022 by Wilson Snyder. This program is free software; you
+# can redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+scenarios(simulator => 1);
+
+compile(
+    );
+
+execute(
+    check_finished => 1,
+    expect_filename => $Self->{golden_filename},
+    );
+
+ok(1);
+1;

test_regress/t/t_string_dyn_num.v

@@ -0,0 +1,69 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// Use this file as a template for submitting bugs, etc.
+// This module takes a single clock input, and should either
+//      $write("*-* All Finished *-*\n");
+//      $finish;
+// on success, or $stop.
+//
+// The code as shown applies a random vector to the Test
+// module, then calculates a CRC on the Test module's outputs.
+//
+// **If you do not wish for your code to be released to the public
+// please note it here, otherwise:**
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2022 by Wilson Snyder.
+// SPDX-License-Identifier: CC0-1.0
+
+module t(/*AUTOARG*/
+   // Inputs
+   clk
+   );
+   input clk;
+
+   integer cyc = 0;
+
+   reg [31:0] istr;
+   string     sstr;
+   string     v;
+
+   // Test loop
+   always @ (posedge clk) begin
+`ifdef TEST_VERBOSE
+      $write("[%0t] cyc==%0d istr='%s' sstr='%s'\n", $time, cyc, istr, sstr);
+`endif
+      cyc <= cyc + 1;
+      sstr <= string'(istr);  // Note takes another cycle
+      if (cyc < 10) begin
+         istr <= 32'h00_00_00_00;
+      end
+      else if (cyc == 13) begin
+         // These displays are needed to check padding of %s
+         $display("istr='%s' istr%%0='%0s' sstr='%s'", istr, istr, sstr);
+         if (sstr.len() != 0) $stop;
+         if (sstr != "") $stop;
+      end
+      else if (cyc == 20) begin
+         istr <= 32'h00_00_41_00;
+      end
+      else if (cyc == 23) begin
+         $display("istr='%s' istr%%0='%0s' sstr='%s'", istr, istr, sstr);
+         if (sstr.len() != 1) $stop;
+         if (sstr != "A") $stop;
+      end
+      else if (cyc == 30) begin
+         istr <= 32'h42_00_41_00;
+      end
+      else if (cyc == 33) begin
+         $display("istr='%s' istr%%0='%0s' sstr='%s'", istr, istr, sstr);
+         if (sstr.len() != 2) $stop;
+         if (sstr != "BA") $stop;
+      end
+      else if (cyc == 99) begin
+         $write("*-* All Finished *-*\n");
+         $finish;
+      end
+   end
+
+endmodule

test_regress/t/t_string_size.out

@@ -1,4 +1,7 @@
+> < == >< (or > < also legal)
+> < == >< (or > < also legal)
+>    < == >    <
+>   < == >   <
+>        < == >        <
 >< == ><
->< == ><
-> < == > <
 *-* All Finished *-*

test_regress/t/t_string_size.v

@@ -4,41 +4,84 @@
 // any use, without warranty, 2021 by wilson Snyder.
 // SPDX-License-Identifier: CC0-1.0
 
-module t;
-   parameter string ES = "";
-   parameter EI = "";  // B is an integer of width 8
+module t(/*AUTOARG*/
+   // Outputs
+   outempty64
+   );
+   output [63:0] outempty64;
+
    parameter string OS = "O";
    parameter OI = "O";  // B is an integer of width 8
 
+   // verilator lint_off WIDTH
+   parameter string EMPTYS = "";
+   parameter EMPTYI = "";  // B is an integer of width 8
+   parameter bit [23:0] EMPTY24 = "";
+   parameter bit [63:0] EMPTY64 = "";
+   // verilator lint_on WIDTH
    parameter bit [31:0] NEST = "NEST";
    parameter bit [31:0] TEST = "TEST";
-   bit [31:0] rest;
    string s;
 
+   // verilator lint_off WIDTH
+   assign outempty64 = "";
+   // verilator lint_on WIDTH
+
    initial begin
-      $display(">< == >%s<", "");
-      $display(">< == >%s<", ES);
-      $display("> < == >%s<", EI);
+      // IEEE: "Leading 0s are never printed" but that does not mean spaces are not
+      $display(">%s< == >< (or > < also legal)", "\000");
+      $display(">%s< == >< (or > < also legal)", "");
+      $display(">%s< == >    <", 32'h0);
+
+      // Numeric context, so IEEE 1800-2017 11.10.3 "" is a "\000"
+      if ($bits("") != 8) $stop;
+      if ("" != "\000") $stop;
 
-      if ($bits("") != 0) $stop;
       if ($bits("A") != 8) $stop;
-      if ($bits(ES) != 0) $stop;
-      if ($bits(EI) != 8) $stop;
-      if ($bits(OS) != 8) $stop;
-      if ($bits(OI) != 8) $stop;
 
-      if (ES == "TEST") $stop;  // Illegal in some simulators as not both strings
-      if (EI == "TEST") $stop;
+      s = "";
+      if (s.len != 0) $stop;
+
+      // IEEE 1800-2017 6.16 "\000" assigned to string is ignored
+      s = "\000yo\000";
+      if (s.len != 2) $stop;
+      if (s != "yo") $stop;
+
+      if ($bits(EMPTYI) != 8) $stop;
+      if (EMPTYI != "\000") $stop;
+      // verilator lint_off WIDTH
+      if (EMPTYI == "TEST") $stop;
+      if (EMPTYI == TEST) $stop;
+      // verilator lint_on WIDTH
+
+      if ($bits(EMPTY24) != 24) $stop;
+      if (EMPTY24 != 0) $stop;
+      $display(">%s< == >   <", EMPTY24);
+
+      if ($bits(EMPTY64) != 64) $stop;
+      if (EMPTY64 != 0) $stop;
+      $display(">%s< == >        <", EMPTY64);
+
+      if ($bits(EMPTYS) != 0) $stop;
+      if (EMPTYS == "TEST") $stop;  // Illegal in some simulators as not both strings
+      if (EMPTYS == TEST) $stop;
+      $display(">%s< == ><", EMPTYS);
+
+      if ($bits(OS) != 8) $stop;
+      if (OS != "O") $stop;
       if (OS == "TEST") $stop;  // Illegal in some simulators as not both strings
+      if (OS == TEST) $stop;
+
+      if ($bits(OI) != 8) $stop;
+      if (OI != "O") $stop;
+
       // verilator lint_off WIDTH
       if (OI == "TEST") $stop;
-      if (rest == "TEST") $stop;
-
-      if (ES == TEST) $stop;
-      if (EI == TEST) $stop;
-      if (OS == TEST) $stop;
       if (OI == TEST) $stop;
-      if (rest == TEST) $stop;
+      // verilator lint_on WIDTH
+
+      if ($bits(outempty64) != 64) $stop;
+      if (outempty64 != 64'h00_00_00_00_00_00_00_00) $stop;
 
       $write("*-* All Finished *-*\n");
       $finish;