From 10a6b566efd554d83b01ea3059fa472e30274218 Mon Sep 17 00:00:00 2001
From: Wilson Snyder <wsnyder@wsnyder.org>
Date: Sat, 7 Dec 2019 15:53:34 -0500
Subject: [PATCH] Fix error on unpacked concatenations, bug1627.

---
 Changes                                  |  2 ++
 src/V3Width.cpp                          | 27 ++++++++++++++----
 test_regress/t/t_packed_concat.v         | 28 +++++++++++++++++++
 test_regress/t/t_packed_concat_bad.out   | 14 ++++++++++
 test_regress/t/t_packed_concat_bad.pl    | 18 ++++++++++++
 test_regress/t/t_packed_concat_bad.v     | 27 ++++++++++++++++++
 test_regress/t/t_queue_unsup_bad.out     | 32 ++++++++++++++++++++++
 test_regress/t/t_unpacked_concat_bad.out | 35 ++++++++++++++----------
 test_regress/t/t_unpacked_concat_bad.pl  |  4 +--
 test_regress/t/t_unpacked_concat_bad.v   |  9 ++++--
 10 files changed, 169 insertions(+), 27 deletions(-)
 create mode 100644 test_regress/t/t_packed_concat.v
 create mode 100644 test_regress/t/t_packed_concat_bad.out
 create mode 100755 test_regress/t/t_packed_concat_bad.pl
 create mode 100644 test_regress/t/t_packed_concat_bad.v

diff --git a/Changes b/Changes
index 5847181e8..dd33a7d94 100644
--- a/Changes
+++ b/Changes
@@ -42,6 +42,8 @@ The contributors that suggested a given feature are shown in []. Thanks!
 
 ****  Fix interface reference tracing, bug1595. [Todd Strader]
 
+****  Fix error on unpacked concatenations, bug1627. [Driss Hafdi]
+
 
 * Verilator 4.022 2019-11-10
 
diff --git a/src/V3Width.cpp b/src/V3Width.cpp
index 584129d44..7087eac87 100644
--- a/src/V3Width.cpp
+++ b/src/V3Width.cpp
@@ -388,6 +388,13 @@ private:
         //   signed: Unsigned  (11.8.1)
         //   width: LHS + RHS
         if (m_vup->prelim()) {
+            AstNodeDType* vdtypep = m_vup->dtypeNullp();
+            if (vdtypep && (VN_IS(vdtypep, AssocArrayDType)
+                            || VN_IS(vdtypep, AssocArrayDType)
+                            || VN_IS(vdtypep, QueueDType))) {
+                nodep->v3error("Unsupported: Concatenation to form "<<vdtypep->prettyTypeName());
+            }
+
             iterateCheckSizedSelf(nodep, "LHS", nodep->lhsp(), SELF, BOTH);
             iterateCheckSizedSelf(nodep, "RHS", nodep->rhsp(), SELF, BOTH);
             nodep->dtypeSetLogicUnsized(nodep->lhsp()->width() + nodep->rhsp()->width(),
@@ -459,6 +466,12 @@ private:
         //   LHS, RHS is self-determined
         //   width: value(LHS) * width(RHS)
         if (m_vup->prelim()) {
+            AstNodeDType* vdtypep = m_vup->dtypeNullp();
+            if (vdtypep && (VN_IS(vdtypep, AssocArrayDType)
+                            || VN_IS(vdtypep, QueueDType)
+                            || VN_IS(vdtypep, UnpackArrayDType))) {
+                nodep->v3error("Unsupported: Replication to form "<<vdtypep->prettyTypeName());
+            }
             iterateCheckSizedSelf(nodep, "LHS", nodep->lhsp(), SELF, BOTH);
             iterateCheckSizedSelf(nodep, "RHS", nodep->rhsp(), SELF, BOTH);
             V3Const::constifyParamsEdit(nodep->rhsp());  // rhsp may change
@@ -1014,11 +1027,13 @@ private:
             uint32_t msbdim = dim.first + dim.second;
             if (!nodep->dimp() || msbdim < 1) {
                 int dim = 1;
-                AstConst* newp = dimensionValue(nodep->fromp()->dtypep(), nodep->attrType(), dim);
+                AstConst* newp = dimensionValue(nodep->fileline(),
+                                                nodep->fromp()->dtypep(), nodep->attrType(), dim);
                 nodep->replaceWith(newp); nodep->deleteTree(); VL_DANGLING(nodep);
             } else if (VN_IS(nodep->dimp(), Const)) {
                 int dim = VN_CAST(nodep->dimp(), Const)->toSInt();
-                AstConst* newp = dimensionValue(nodep->fromp()->dtypep(), nodep->attrType(), dim);
+                AstConst* newp = dimensionValue(nodep->fileline(),
+                                                nodep->fromp()->dtypep(), nodep->attrType(), dim);
                 nodep->replaceWith(newp); nodep->deleteTree(); VL_DANGLING(nodep);
             }
             else {  // Need a runtime lookup table.  Yuk.
@@ -4180,7 +4195,7 @@ private:
         return nodep;
     }
 
-    AstConst* dimensionValue(AstNodeDType* nodep, AstAttrType attrType, int dim) {
+    AstConst* dimensionValue(FileLine* fileline, AstNodeDType* nodep, AstAttrType attrType, int dim) {
         // Return the dimension value for the specified attribute and constant dimension
         AstNodeDType* dtypep = nodep->skipRefp();
         VNumRange declRange;  // ranged() set false
@@ -4251,7 +4266,7 @@ private:
             nodep->v3fatalSrc("Missing DIM ATTR type case");
             break;
         }
-        if (!valp) valp = new AstConst(nodep->fileline(), AstConst::Signed32(), val);
+        if (!valp) valp = new AstConst(fileline, AstConst::Signed32(), val);
         UINFO(9," $dimension "<<attrType.ascii()
               <<"("<<cvtToHex(dtypep)<<","<<dim<<")="<<valp<<endl);
         return valp;
@@ -4277,9 +4292,9 @@ private:
         // Add to root, as don't know module we are in, and aids later structure sharing
         v3Global.rootp()->dollarUnitPkgAddp()->addStmtp(varp);
         // Element 0 is a non-index and has speced values
-        initp->addValuep(dimensionValue(nodep, attrType, 0));
+        initp->addValuep(dimensionValue(nodep->fileline(), nodep, attrType, 0));
         for (unsigned i=1; i<msbdim+1; ++i) {
-            initp->addValuep(dimensionValue(nodep, attrType, i));
+            initp->addValuep(dimensionValue(nodep->fileline(), nodep, attrType, i));
         }
         userIterate(varp, NULL);  // May have already done $unit so must do this var
         m_tableMap.insert(make_pair(make_pair(nodep, attrType), varp));
diff --git a/test_regress/t/t_packed_concat.v b/test_regress/t/t_packed_concat.v
new file mode 100644
index 000000000..30cf7bbac
--- /dev/null
+++ b/test_regress/t/t_packed_concat.v
@@ -0,0 +1,28 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2019 by Driss Hafdi
+
+module t (/*AUTOARG*/
+   // Inputs
+   clk
+   );
+
+   input clk;
+
+   typedef logic [15:0] count_t;
+   typedef bit [31:0]   bit_int_t;
+
+   // bug1627
+   localparam bit_int_t [1:0] count_bits = {2{$bits(count_t)}};
+   localparam bit_int_t [1:0] count_bitsc = {$bits(count_t), $bits(count_t)};
+
+   initial begin
+      if (count_bits[0] != 16) $stop;
+      if (count_bits[1] != 16) $stop;
+      if (count_bitsc[0] != 16) $stop;
+      if (count_bitsc[1] != 16) $stop;
+      $write("*-* All Finished *-*\n");
+      $finish;
+   end
+endmodule
diff --git a/test_regress/t/t_packed_concat_bad.out b/test_regress/t/t_packed_concat_bad.out
new file mode 100644
index 000000000..d7f88ead6
--- /dev/null
+++ b/test_regress/t/t_packed_concat_bad.out
@@ -0,0 +1,14 @@
+%Warning-WIDTHCONCAT: t/t_packed_concat_bad.v:16: Unsized numbers/parameters not allowed in replications.
+                                                : ... In instance t
+   localparam bit_int_t [1:0] count_bits = {2{$bits(count_t)}};
+                                              ^~~~~
+                      ... Use "/* verilator lint_off WIDTHCONCAT */" and lint_on around source to disable this message.
+%Warning-WIDTHCONCAT: t/t_packed_concat_bad.v:17: Unsized numbers/parameters not allowed in concatenations.
+                                                : ... In instance t
+   localparam bit_int_t [1:0] count_bitsc = {$bits(count_t), $bits(count_t)};
+                                             ^~~~~
+%Warning-WIDTHCONCAT: t/t_packed_concat_bad.v:17: Unsized numbers/parameters not allowed in replications.
+                                                : ... In instance t
+   localparam bit_int_t [1:0] count_bitsc = {$bits(count_t), $bits(count_t)};
+                                                           ^
+%Error: Exiting due to
diff --git a/test_regress/t/t_packed_concat_bad.pl b/test_regress/t/t_packed_concat_bad.pl
new file mode 100755
index 000000000..46d54cf79
--- /dev/null
+++ b/test_regress/t/t_packed_concat_bad.pl
@@ -0,0 +1,18 @@
+#!/usr/bin/perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2019 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.
+
+scenarios(simulator => 1);
+
+compile(
+    fails => 1,
+    expect_filename => $Self->{golden_filename},
+    );
+
+ok(1);
+1;
diff --git a/test_regress/t/t_packed_concat_bad.v b/test_regress/t/t_packed_concat_bad.v
new file mode 100644
index 000000000..fcae999d3
--- /dev/null
+++ b/test_regress/t/t_packed_concat_bad.v
@@ -0,0 +1,27 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2019 by Driss Hafdi
+
+module t (/*AUTOARG*/
+   // Inputs
+   clk
+   );
+
+   input clk;
+
+   typedef logic [15:0] count_t;
+   typedef bit [31:0]   bit_int_t;
+
+   localparam bit_int_t [1:0] count_bits = {2{$bits(count_t)}};
+   localparam bit_int_t [1:0] count_bitsc = {$bits(count_t), $bits(count_t)};
+
+   initial begin
+      if (count_bits[0] != 16) $stop;
+      if (count_bits[1] != 16) $stop;
+      if (count_bitsc[0] != 16) $stop;
+      if (count_bitsc[1] != 16) $stop;
+      $write("*-* All Finished *-*\n");
+      $finish;
+   end
+endmodule
diff --git a/test_regress/t/t_queue_unsup_bad.out b/test_regress/t/t_queue_unsup_bad.out
index 61d5258c0..6e37baf6f 100644
--- a/test_regress/t/t_queue_unsup_bad.out
+++ b/test_regress/t/t_queue_unsup_bad.out
@@ -1,3 +1,7 @@
+%Error: t/t_queue_unsup_bad.v:20: Unsupported: Replication to form QUEUEDTYPE
+                                : ... In instance t
+      q = {"q", "b", "c", "d", "e", "f"};
+          ^
 %Error: t/t_queue_unsup_bad.v:23: Unsupported: Queue .delete(index) method, as is O(n) complexity and slow.
                                 : ... In instance t
       q.delete(1);
@@ -10,6 +14,26 @@
                                 : ... In instance t
       q.insert(2, "ins2");
         ^~~~~~
+%Error: t/t_queue_unsup_bad.v:33: Unsupported: Replication to form QUEUEDTYPE
+                                : ... In instance t
+      q = {q, "f1"};   
+          ^
+%Error: t/t_queue_unsup_bad.v:34: Unsupported: Replication to form QUEUEDTYPE
+                                : ... In instance t
+      q = {q, "f2"};   
+          ^
+%Error: t/t_queue_unsup_bad.v:35: Unsupported: Replication to form QUEUEDTYPE
+                                : ... In instance t
+      q = {"b1", q};   
+          ^
+%Error: t/t_queue_unsup_bad.v:36: Unsupported: Replication to form QUEUEDTYPE
+                                : ... In instance t
+      q = {"b2", q};   
+          ^
+%Error: t/t_queue_unsup_bad.v:37: Unsupported: Replication to form QUEUEDTYPE
+                                : ... In instance t
+      q = {q[0], q[2:$]};   
+          ^
 %Error: t/t_queue_unsup_bad.v:37: Unsupported/illegal unbounded ('$') in this context.
                                 : ... In instance t
       q = {q[0], q[2:$]};   
@@ -26,8 +50,16 @@
                                 : ... In instance t
       q = {q[0], q[2:$]};   
                   ^
+%Error: t/t_queue_unsup_bad.v:41: Unsupported: Replication to form QUEUEDTYPE
+                                : ... In instance t
+         string ai[$] = { "Foo", "Bar" };
+                        ^
 %Error: t/t_queue_unsup_bad.v:46: Unsupported: Assignment pattern applies against non struct/union: QUEUEDTYPE
                                 : ... In instance t
          q = '{ "BB", "CC" };   
              ^~
+%Error: t/t_queue_unsup_bad.v:49: Unsupported: Replication to form QUEUEDTYPE
+                                : ... In instance t
+         q = { "BB", "CC" };   
+             ^
 %Error: Exiting due to
diff --git a/test_regress/t/t_unpacked_concat_bad.out b/test_regress/t/t_unpacked_concat_bad.out
index 5a027afdc..a1f4b32d9 100644
--- a/test_regress/t/t_unpacked_concat_bad.out
+++ b/test_regress/t/t_unpacked_concat_bad.out
@@ -1,17 +1,22 @@
-%Warning-WIDTHCONCAT: t/t_typedef_logic_in_concat_bad.v:12: Unsized numbers/parameters not allowed in replications.
-                                                          : ... In instance t
-  typedef logic [15:0] count_t;
-          ^~~~~
-                      ... Use "/* verilator lint_off WIDTHCONCAT */" and lint_on around source to disable this message.
-%Error: Exiting due to 1 warning(s)
-
-# This warning message is pretty misleading. Instead of pointing out to the call to $bits() without a cast, it points to the type declaration of the argument to bits. It would be more useful if it looked like this:
-
-%Warning-WIDTHCONCAT: t/t_typedef_logic_in_concat_bad.v:15: Unsized numbers/parameters not allowed in replications.
-                                                      : ... In instance t
-  localparam bit_int_t [1:0] count_bits = {2{$bits(count_t)}};
+%Error: t/t_unpacked_concat_bad.v:16: Unsupported: Replication to form UNPACKARRAYDTYPE
+                                    : ... In instance t
+   localparam bit_int_t count_bits [1:0] = {2{$bits(count_t)}};
+                                             ^
+%Warning-WIDTHCONCAT: t/t_unpacked_concat_bad.v:16: Unsized numbers/parameters not allowed in replications.
+                                                  : ... In instance t
+   localparam bit_int_t count_bits [1:0] = {2{$bits(count_t)}};
                                               ^~~~~
                       ... Use "/* verilator lint_off WIDTHCONCAT */" and lint_on around source to disable this message.
-%Error: Exiting due to 1 warning(s)
-
-
+%Error: t/t_unpacked_concat_bad.v:17: Unsupported: Replication to form UNPACKARRAYDTYPE
+                                    : ... In instance t
+   localparam bit_int_t count_bitsc [1:0] = {$bits(count_t), $bits(count_t)};
+                                            ^
+%Warning-WIDTHCONCAT: t/t_unpacked_concat_bad.v:17: Unsized numbers/parameters not allowed in concatenations.
+                                                  : ... In instance t
+   localparam bit_int_t count_bitsc [1:0] = {$bits(count_t), $bits(count_t)};
+                                             ^~~~~
+%Warning-WIDTHCONCAT: t/t_unpacked_concat_bad.v:17: Unsized numbers/parameters not allowed in replications.
+                                                  : ... In instance t
+   localparam bit_int_t count_bitsc [1:0] = {$bits(count_t), $bits(count_t)};
+                                                           ^
+%Error: Exiting due to
diff --git a/test_regress/t/t_unpacked_concat_bad.pl b/test_regress/t/t_unpacked_concat_bad.pl
index 3dac40252..49151b6cd 100755
--- a/test_regress/t/t_unpacked_concat_bad.pl
+++ b/test_regress/t/t_unpacked_concat_bad.pl
@@ -7,9 +7,7 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di
 # Lesser General Public License Version 3 or the Perl Artistic License
 # Version 2.0.
 
-scenarios(linter => 1);
-
-$Self->{vlt_all} and unsupported("Verilator unsupported, bug1627");
+scenarios(vlt => 1);
 
 lint(
     fails => 1,
diff --git a/test_regress/t/t_unpacked_concat_bad.v b/test_regress/t/t_unpacked_concat_bad.v
index 685a5e5ca..90947af52 100644
--- a/test_regress/t/t_unpacked_concat_bad.v
+++ b/test_regress/t/t_unpacked_concat_bad.v
@@ -13,11 +13,14 @@ module t (/*AUTOARG*/
    typedef logic [15:0] count_t;
    typedef bit [31:0]   bit_int_t;
 
-   localparam bit_int_t [1:0] count_bits = {2{$bits(count_t)}};
+   localparam bit_int_t count_bits [1:0] = {2{$bits(count_t)}};
+   localparam bit_int_t count_bitsc [1:0] = {$bits(count_t), $bits(count_t)};
 
    initial begin
-      $write("%d\n", count_bits[0]);
-      $write("%d\n", count_bits[1]);
+      if (count_bits[0] != 16) $stop;
+      if (count_bits[1] != 16) $stop;
+      if (count_bitsc[0] != 16) $stop;
+      if (count_bitsc[1] != 16) $stop;
       $write("*-* All Finished *-*\n");
       $finish;
    end