Fix `new this` (#5909).

Changes

@@ -62,6 +62,7 @@ Verilator 5.035 devel
 * Fix process comparisons (#5896).
 * Fix ccache with clang (#5899). [Geza Lore]
 * Fix delayed assignment malformed LHS assertion (#5904).
+* Fix `new this` (#5909).
 
 
 Verilator 5.034 2025-02-24

docs/CONTRIBUTORS

@@ -26,6 +26,7 @@ Arkadiusz Kozdra
 Arthur Rosa
 Aylon Chaim Porat
 Bartłomiej Chmiel
+Brian Li
 Cameron Kirk
 Chih-Mao Chen
 Chris Bachhuber

docs/guide/warnings.rst

@@ -472,6 +472,30 @@ List Of Warnings
    correctly.
 
 
+.. option:: DEFOVERRIDE
+
+   Warns that a macro definition within the code is being overridden by a command line directive:
+
+   For example, running Verilator with :code:`<+define+\<DUP\>=\<def2\>>` and
+
+   .. code-block:: sv
+      :linenos:
+      :emphasize-lines: 1
+
+         `define DUP def2 //<--- Warning
+
+   Results in:
+
+   .. code-block::
+         %Warning-DEFOVERRIDE: example.v1:20: Overriding define: 'DEF' with value: 'def2' to existing command line define value: 'def1'
+                      ... Location of previous definition, with value: '50'
+
+   While not explicitly stated in the IEEE 1800-2023 standard, this warning
+   tracks with the other simulators' behavior of overriding macro
+   definitions within code files with the definition passed in through
+   the command line.
+
+
 .. option:: DEFPARAM
 
    Warns that the :code:`defparam` statement was deprecated in IEEE 1364-2001,
@@ -1261,6 +1285,7 @@ List Of Warnings
    Ignoring this warning will only suppress the lint check; it will
    simulate correctly.
 
+
 .. option:: PINCONNECTEMPTY
 
    .. TODO better example

include/verilated.cpp

@@ -3476,7 +3476,7 @@ void VerilatedHierarchy::remove(VerilatedScope* fromp, VerilatedScope* top) {
 void VerilatedAssertOneThread::fatal_different() VL_MT_SAFE {
     VL_FATAL_MT(__FILE__, __LINE__, "",
                 "Routine called that is single threaded, but called from"
-                " a different thread then the expected constructing thread");
+                " a different thread than the expected constructing thread");
 }
 #endif
 

src/V3AstNodeDType.h

@@ -120,15 +120,7 @@ public:
     // Iff has second dtype, set as generic node function
     virtual void virtRefDType2p(AstNodeDType* nodep) {}
     // Assignable equivalence.  Calls skipRefToNonRefp() during comparisons.
-    bool similarDType(const AstNodeDType* samep) const {
-        const AstNodeDType* nodep = this;
-        nodep = nodep->skipRefToNonRefp();
-        samep = samep->skipRefToNonRefp();
-        if (nodep == samep) return true;
-        if (nodep->type() != samep->type()) return false;
-        return nodep->similarDTypeNode(samep);
-    }
-
+    bool similarDType(const AstNodeDType* samep) const;
     // Iff has a non-null subDTypep(), as generic node function
     virtual AstNodeDType* subDTypep() const VL_MT_STABLE { return nullptr; }
     virtual bool isFourstate() const;
@@ -581,7 +573,11 @@ public:
         const AstClassRefDType* const asamep = VN_DBG_AS(samep, ClassRefDType);
         return (m_classp == asamep->m_classp && m_classOrPackagep == asamep->m_classOrPackagep);
     }
-    bool similarDTypeNode(const AstNodeDType* samep) const override { return sameNode(samep); }
+    bool similarDTypeNode(const AstNodeDType* samep) const override {
+        // Doesn't need to compare m_classOrPackagep
+        const AstClassRefDType* const asamep = VN_DBG_AS(samep, ClassRefDType);
+        return m_classp == asamep->m_classp;
+    }
     void dump(std::ostream& str = std::cout) const override;
     void dumpJson(std::ostream& str = std::cout) const override;
     void dumpSmall(std::ostream& str) const override;

src/V3AstNodes.cpp

@@ -848,6 +848,15 @@ const AstNodeDType* AstNodeDType::skipRefIterp(bool skipConst, bool skipEnum,
     }
 }
 
+bool AstNodeDType::similarDType(const AstNodeDType* samep) const {
+    const AstNodeDType* nodep = this;
+    nodep = nodep->skipRefToNonRefp();
+    samep = samep->skipRefToNonRefp();
+    if (nodep == samep) return true;
+    if (nodep->type() != samep->type()) return false;
+    return nodep->similarDTypeNode(samep);
+}
+
 bool AstNodeDType::isFourstate() const { return basicp() && basicp()->isFourstate(); }
 
 class AstNodeDType::CTypeRecursed final {

src/V3Error.h

@@ -90,6 +90,7 @@ public:
         CONTASSREG,     // Continuous assignment on reg
         COVERIGN,       // Coverage ignored
         DECLFILENAME,   // Declaration doesn't match filename
+        DEFOVERRIDE,    // Overriding existing define macro through command line
         DEFPARAM,       // Style: Defparam
         DEPRECATED,     // Feature will be deprecated
         ENCAPSULATED,   // Error: local/protected violation
@@ -196,7 +197,7 @@ public:
             "BLKANDNBLK", "BLKLOOPINIT", "BLKSEQ", "BSSPACE",
             "CASEINCOMPLETE", "CASEOVERLAP", "CASEWITHX", "CASEX", "CASTCONST", "CDCRSTLOGIC", "CLKDATA",
             "CMPCONST", "COLONPLUS", "COMBDLY", "CONSTRAINTIGN", "CONTASSREG", "COVERIGN",
-            "DECLFILENAME", "DEFPARAM", "DEPRECATED",
+            "DECLFILENAME", "DEFOVERRIDE", "DEFPARAM", "DEPRECATED",
             "ENCAPSULATED", "ENDLABEL", "ENUMVALUE", "EOFNEWLINE", "GENCLK",
             "GENUNNAMED", "HIERBLOCK",
             "IFDEPTH", "IGNOREDRETURN",

src/V3PreProc.cpp

@@ -211,6 +211,7 @@ private:
     string defineSubst(VDefineRef* refp);
 
     bool defExists(const string& name);
+    bool defCmdline(const string& name);
     string defValue(const string& name);
     string defParams(const string& name);
     FileLine* defFileline(const string& name);
@@ -316,6 +317,11 @@ bool V3PreProcImp::defExists(const string& name) {
     const auto iter = m_defines.find(name);
     return (iter != m_defines.end());
 }
+bool V3PreProcImp::defCmdline(const string& name) {
+    const auto iter = m_defines.find(name);
+    if (iter == m_defines.end()) { return false; }
+    return iter->second.cmdline();
+}
 string V3PreProcImp::defValue(const string& name) {
     const auto iter = m_defines.find(name);
     if (iter == m_defines.end()) {
@@ -345,21 +351,37 @@ void V3PreProcImp::define(FileLine* fl, const string& name, const string& value,
                                                                   << "' (IEEE 1800-2023 22.5.1)");
     } else {
         if (defExists(name)) {
-            if (!(defValue(name) == value
-                  && defParams(name) == params)) {  // Duplicate defs are OK
-                fl->v3warn(REDEFMACRO, "Redefining existing define: '"
-                                           << name << "', with different value: '" << value
-                                           << (params == "" ? "" : " ") << params << "'\n"
-                                           << fl->warnContextPrimary() << '\n'
-                                           << defFileline(name)->warnOther()
-                                           << "... Location of previous definition, with value: '"
-                                           << defValue(name)
-                                           << (defParams(name).empty() ? "" : " ")
-                                           << defParams(name) << "'\n"
-                                           << defFileline(name)->warnContextSecondary());
+            if (defCmdline(name) && !cmdline) {
+                fl->v3warn(DEFOVERRIDE,
+                           "Overriding define: '"
+                               << name << "' with value: '" << value
+                               << "' to existing command line define value: '" << defValue(name)
+                               << (params == "" ? "" : " ") << params << "'\n"
+                               << fl->warnContextPrimary() << '\n'
+                               << defFileline(name)->warnOther()
+                               << "... Location of previous definition, with value: '"
+                               << defValue(name) << (defParams(name).empty() ? "" : " ")
+                               << defParams(name) << "'\n"
+                               << defFileline(name)->warnContextSecondary());
+                return;
+            } else {
+                if (!(defValue(name) == value
+                      && defParams(name) == params)) {  // Duplicate defs are OK
+                    fl->v3warn(REDEFMACRO,
+                               "Redefining existing define: '"
+                                   << name << "', with different value: '" << value
+                                   << (params == "" ? "" : " ") << params << "'\n"
+                                   << fl->warnContextPrimary() << '\n'
+                                   << defFileline(name)->warnOther()
+                                   << "... Location of previous definition, with value: '"
+                                   << defValue(name) << (defParams(name).empty() ? "" : " ")
+                                   << defParams(name) << "'\n"
+                                   << defFileline(name)->warnContextSecondary());
+                }
+                undef(name);
             }
-            undef(name);
         }
+
         m_defines.emplace(name, VDefine{fl, value, params, cmdline});
     }
 }

src/V3Width.cpp

@@ -4328,7 +4328,7 @@ class WidthVisitor final : public VNVisitor {
         userIterateChildren(nodep, WidthVP{SELF, BOTH}.p());
         if (!similarDTypeRecurse(nodep->dtypep(), nodep->rhsp()->dtypep())) {
             nodep->rhsp()->v3error("New-as-copier passed different data type '"
-                                   << nodep->dtypep()->prettyTypeName() << "' then expected '"
+                                   << nodep->dtypep()->prettyTypeName() << "' than expected '"
                                    << nodep->rhsp()->dtypep()->prettyTypeName() << "'");
         }
     }

test_regress/t/t_class_copy_bad.out

@@ -1,4 +1,4 @@
-%Error: t/t_class_copy_bad.v:19:16: New-as-copier passed different data type 'CLASSREFDTYPE 'Cls'' then expected 'CLASSREFDTYPE 'Other''
+%Error: t/t_class_copy_bad.v:19:16: New-as-copier passed different data type 'CLASSREFDTYPE 'Cls'' than expected 'CLASSREFDTYPE 'Other''
                                   : ... note: In instance 't'
    19 |       c1 = new co;   
       |                ^~

test_regress/t/t_class_new_this.v

@@ -21,16 +21,26 @@ class Testcase implements ICls;
    virtual function string get();
       return "In ICls";
    endfunction
+   function Testcase clone();
+      Testcase a = new this;
+      return a;
+   endfunction
 endclass
 
 module t(/*AUTOARG*/);
 
    initial begin
       Testcase test;
+      Testcase cloned;
       test = new;
       if (test.cls.name != "test_class") $stop;
       if (test.cls.icls.get() != "In ICls") $stop;
+
+      cloned = test.clone();
+      if (cloned.cls.name != "test_class") $stop;
+
       test.cls.icls = null; // Prevent leak
+
       $write("*-* All Finished *-*\n");
       $finish;
    end

test_regress/t/t_define_override.out

@@ -0,0 +1,9 @@
+%Warning-REDEFMACRO: Redefining existing define: 'TEST_MACRO', with different value: '50'
+                     ... Location of previous definition, with value: '20'
+                     ... For warning description see https://verilator.org/warn/REDEFMACRO?v=latest
+                     ... Use "/* verilator lint_off REDEFMACRO */" and lint_on around source to disable this message.
+%Warning-DEFOVERRIDE: t/t_define_override.v:9:23: Overriding define: 'TEST_MACRO' with value: '10' to existing command line define value: '50'
+                      ... Location of previous definition, with value: '50'
+%Warning-DEFOVERRIDE: t/t_define_override.v:10:24: Overriding define: 'TEST_MACRO' with value: '100' to existing command line define value: '50'
+                      ... Location of previous definition, with value: '50'
+%Error: Exiting due to

test_regress/t/t_define_override.py

@@ -0,0 +1,18 @@
+#!/usr/bin/env python3
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2025 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
+
+import vltest_bootstrap
+
+test.scenarios('linter')
+
+test.lint(verilator_flags2=["+define+TEST_MACRO=20 +define+TEST_MACRO=50"],
+          fails=True,
+          expect_filename=test.golden_filename)
+
+test.passes()

test_regress/t/t_define_override.v

@@ -0,0 +1,20 @@
+// DESCRIPTION: Verilator: Multiple `defines while using +define+
+// as a command-line argument as well
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2025 by Wilson Snyder.
+// SPDX-License-Identifier: CC0-1.0
+
+`define TEST_MACRO 10
+`define TEST_MACRO 100
+
+`define STRINGIFY(x) `"x`"
+
+module test (
+);
+    initial begin
+        $display("TEST_MACRO %s", `STRINGIFY(`TEST_MACRO));
+        $finish;
+    end
+
+endmodule

test_regress/t/t_define_override_empty.out

@@ -0,0 +1 @@
+TEST_MACRO  

test_regress/t/t_define_override_empty.py

@@ -0,0 +1,21 @@
+#!/usr/bin/env python3
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2025 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
+
+import vltest_bootstrap
+
+test.scenarios('linter')
+test.top_filename = "t/t_define_override.v"
+
+#test.lint(verilator_flags2=["+define+TEST_MACRO"], fails=True, expect_filename=test.golden_filename)
+
+test.compile(verilator_flags2=["-Wno-DEFOVERRIDE -Wno-REDEFMACRO +define+TEST_MACRO"])
+
+test.execute(expect_filename=test.golden_filename)
+
+test.passes()

test_regress/t/t_define_override_output.out

@@ -0,0 +1 @@
+TEST_MACRO 50

test_regress/t/t_define_override_output.py

@@ -0,0 +1,21 @@
+#!/usr/bin/env python3
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2025 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
+
+import vltest_bootstrap
+
+test.scenarios('vlt')
+test.top_filename = "t/t_define_override.v"
+
+test.compile(verilator_flags2=[
+    "-Wno-DEFOVERRIDE -Wno-REDEFMACRO +define+TEST_MACRO=20 +define+TEST_MACRO=50"
+])
+
+test.execute(expect_filename=test.golden_filename)
+
+test.passes()