From 24b097b228dc4c6638cd28588aacde484000978a Mon Sep 17 00:00:00 2001
From: Wilson Snyder <wsnyder@wsnyder.org>
Date: Sun, 11 Aug 2024 10:09:05 -0400
Subject: [PATCH] Improve new class error (#5359)

---
 src/V3Width.cpp                        | 13 ++++++----
 test_regress/t/t_class_new_ref_bad.out | 13 ++++++++++
 test_regress/t/t_class_new_ref_bad.pl  | 19 +++++++++++++++
 test_regress/t/t_class_new_ref_bad.v   | 33 ++++++++++++++++++++++++++
 4 files changed, 74 insertions(+), 4 deletions(-)
 create mode 100644 test_regress/t/t_class_new_ref_bad.out
 create mode 100755 test_regress/t/t_class_new_ref_bad.pl
 create mode 100644 test_regress/t/t_class_new_ref_bad.v

diff --git a/src/V3Width.cpp b/src/V3Width.cpp
index 84c567177..308675510 100644
--- a/src/V3Width.cpp
+++ b/src/V3Width.cpp
@@ -4102,7 +4102,9 @@ class WidthVisitor final : public VNVisitor {
             AstClassRefDType* const refp
                 = m_vup ? VN_CAST(m_vup->dtypeNullSkipRefp(), ClassRefDType) : nullptr;
             if (!refp) {  // e.g. int a = new;
-                nodep->v3error("new() not expected in this context");
+                nodep->v3error("new() assignment not legal to non-class data type "
+                               + (m_vup->dtypeNullp() ? m_vup->dtypep()->prettyDTypeNameQ() : ""));
+                nodep->dtypep(m_vup->dtypep());
                 return;
             }
             nodep->dtypep(refp);
@@ -4137,7 +4139,9 @@ class WidthVisitor final : public VNVisitor {
         if (nodep->didWidthAndSet()) return;
         AstClassRefDType* const refp = VN_CAST(m_vup->dtypeNullSkipRefp(), ClassRefDType);
         if (!refp) {  // e.g. int a = new;
-            nodep->v3error("new() not expected in this context");
+            nodep->v3error("new() cannot copy from non-class data type "
+                           + (m_vup->dtypeNullp() ? m_vup->dtypep()->prettyDTypeNameQ() : ""));
+            nodep->dtypep(m_vup->dtypep());
             return;
         }
         nodep->dtypep(refp);
@@ -4936,16 +4940,17 @@ class WidthVisitor final : public VNVisitor {
         assertAtStatement(nodep);
         // if (debug()) nodep->dumpTree("-  AssignPre: ");
         {
-            // if (debug()) nodep->dumpTree("-    assin:: ");
+            if (debug()) nodep->dumpTree("-    assin:: ");
             userIterateAndNext(nodep->lhsp(), WidthVP{SELF, BOTH}.p());
             UASSERT_OBJ(nodep->lhsp()->dtypep(), nodep, "How can LHS be untyped?");
             UASSERT_OBJ(nodep->lhsp()->dtypep()->widthSized(), nodep, "How can LHS be unsized?");
             nodep->dtypeFrom(nodep->lhsp());
             //
             // AstPattern needs to know the proposed data type of the lhs, so pass on the prelim
+            if (debug()) nodep->dumpTree("-    assrhs: ");
             userIterateAndNext(nodep->rhsp(), WidthVP{nodep->dtypep(), PRELIM}.p());
             //
-            // if (debug()) nodep->dumpTree("-    assign: ");
+            if (debug()) nodep->dumpTree("-    assign: ");
             AstNodeDType* const lhsDTypep
                 = nodep->lhsp()->dtypep();  // Note we use rhsp for context determined
             iterateCheckAssign(nodep, "Assign RHS", nodep->rhsp(), FINAL, lhsDTypep);
diff --git a/test_regress/t/t_class_new_ref_bad.out b/test_regress/t/t_class_new_ref_bad.out
new file mode 100644
index 000000000..61aeb7fb8
--- /dev/null
+++ b/test_regress/t/t_class_new_ref_bad.out
@@ -0,0 +1,13 @@
+%Error: t/t_class_new_ref_bad.v:16:24: new() assignment not legal to non-class data type 'int'
+                                     : ... note: In instance 't'
+   16 |       txn_type_t txn = new;
+      |                        ^~~
+%Error: t/t_class_new_ref_bad.v:17:25: new() cannot copy from non-class data type 'int'
+                                     : ... note: In instance 't'
+   17 |       txn_type_t copy = new txn;
+      |                         ^~~
+%Error: t/t_class_new_ref_bad.v:26:21: Assign RHS expects a CLASSREFDTYPE 'Base', got BASICDTYPE 'int'
+                                     : ... note: In instance 't'
+   26 |       Base b = Cls::generate_txn();
+      |                     ^~~~~~~~~~~~
+%Error: Exiting due to
diff --git a/test_regress/t/t_class_new_ref_bad.pl b/test_regress/t/t_class_new_ref_bad.pl
new file mode 100755
index 000000000..a60503a1f
--- /dev/null
+++ b/test_regress/t/t_class_new_ref_bad.pl
@@ -0,0 +1,19 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2003 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(linter => 1);
+
+lint(
+    fails => 1,
+    expect_filename => $Self->{golden_filename},
+    );
+
+ok(1);
+1;
diff --git a/test_regress/t/t_class_new_ref_bad.v b/test_regress/t/t_class_new_ref_bad.v
new file mode 100644
index 000000000..f463fc841
--- /dev/null
+++ b/test_regress/t/t_class_new_ref_bad.v
@@ -0,0 +1,33 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2024 by Wilson Snyder.
+// SPDX-License-Identifier: CC0-1.0
+
+class Base;
+endclass
+
+class Cls extends Base;
+   typedef int txn_type_t;  // Bad type
+
+   rand txn_type_t req_txn_type;
+
+   static function txn_type_t generate_txn();
+      txn_type_t txn = new;
+      txn_type_t copy = new txn;
+      return txn;
+   endfunction
+
+endclass
+
+module t(/*AUTOARG*/);
+
+   initial begin
+      Base b = Cls::generate_txn();
+      $display("%p", b);
+
+      $write("*-* All Finished *-*\n");
+      $finish;
+   end
+
+endmodule