Fix dtype of condition operation on class objects (#4345) (#4352)

src/V3AstNodeExpr.h

@@ -360,14 +360,8 @@ class AstNodeCond VL_NOT_FINAL : public AstNodeTriop {
     // @astgen alias op2 := thenp
     // @astgen alias op3 := elsep
 protected:
-    AstNodeCond(VNType t, FileLine* fl, AstNodeExpr* condp, AstNodeExpr* thenp, AstNodeExpr* elsep)
+    AstNodeCond(VNType t, FileLine* fl, AstNodeExpr* condp, AstNodeExpr* thenp,
-        : AstNodeTriop{t, fl, condp, thenp, elsep} {
+                AstNodeExpr* elsep);
-        if (thenp) {
-            dtypeFrom(thenp);
-        } else if (elsep) {
-            dtypeFrom(elsep);
-        }
-    }
 
 public:
     ASTGEN_MEMBERS_AstNodeCond;

src/V3AstNodes.cpp

@@ -25,6 +25,7 @@
 #include "V3Hasher.h"
 #include "V3PartitionGraph.h"  // Just for mtask dumping
 #include "V3String.h"
+#include "V3Width.h"
 
 #include "V3Ast__gen_macros.h"  // Generated by 'astgen'
 
@@ -134,6 +135,20 @@ string AstCCall::selfPointerProtect(bool useSelfForThis) const {
     return VIdProtect::protectWordsIf(sp, protect());
 }
 
+AstNodeCond::AstNodeCond(VNType t, FileLine* fl, AstNodeExpr* condp, AstNodeExpr* thenp,
+                         AstNodeExpr* elsep)
+    : AstNodeTriop{t, fl, condp, thenp, elsep} {
+    UASSERT_OBJ(thenp, this, "No thenp expression");
+    UASSERT_OBJ(elsep, this, "No elsep expression");
+    if (thenp->isClassHandleValue() && elsep->isClassHandleValue()) {
+        // Get the most-deriving class type that both arguments can be casted to.
+        AstNodeDType* const commonClassTypep = V3Width::getCommonClassTypep(thenp, elsep);
+        UASSERT_OBJ(commonClassTypep, this, "No common base class exists");
+        dtypep(commonClassTypep);
+    } else {
+        dtypeFrom(thenp);
+    }
+}
 void AstNodeCond::numberOperate(V3Number& out, const V3Number& lhs, const V3Number& rhs,
                                 const V3Number& ths) {
     if (lhs.isNeqZero()) {

src/V3Width.cpp

@@ -513,7 +513,8 @@ private:
                 AstNodeDType* commonClassTypep = nullptr;
                 if (nodep->thenp()->isClassHandleValue() && nodep->elsep()->isClassHandleValue()) {
                     // Get the most-deriving class type that both arguments can be casted to.
-                    commonClassTypep = getCommonClassTypep(nodep->thenp(), nodep->elsep());
+                    commonClassTypep
+                        = V3Width::getCommonClassTypep(nodep->thenp(), nodep->elsep());
                 }
                 if (commonClassTypep) {
                     nodep->dtypep(commonClassTypep);
@@ -7337,41 +7338,9 @@ private:
         if (nodep->subDTypep()) return hasOpenArrayIterateDType(nodep->subDTypep()->skipRefp());
         return false;
     }
-    AstNodeDType* getCommonClassTypep(AstNode* nodep1, AstNode* nodep2) {
-        // Return the class type that both nodep1 and nodep2 are castable to.
-        // If both are null, return the type of null constant.
-        // If one is a class and one is null, return AstClassRefDType that points to that class.
-        // If no common class type exists, return nullptr.
-
-        // First handle cases with null values and when one class is a super class of the other.
-        if (VN_IS(nodep1, Const)) std::swap(nodep1, nodep2);
-        const Castable castable = computeCastable(nodep1->dtypep(), nodep2->dtypep(), nodep2);
-        if (castable == SAMEISH || castable == COMPATIBLE) {
-            return nodep1->dtypep()->cloneTree(false);
-        } else if (castable == DYNAMIC_CLASS) {
-            return nodep2->dtypep()->cloneTree(false);
-        }
-
-        AstClassRefDType* classDtypep1 = VN_CAST(nodep1->dtypep(), ClassRefDType);
-        while (classDtypep1) {
-            const Castable castable = computeCastable(classDtypep1, nodep2->dtypep(), nodep2);
-            if (castable == COMPATIBLE) return classDtypep1->cloneTree(false);
-            AstClassExtends* const extendsp = classDtypep1->classp()->extendsp();
-            classDtypep1 = extendsp ? VN_AS(extendsp->dtypep(), ClassRefDType) : nullptr;
-        }
-        return nullptr;
-    }
 
     //----------------------------------------------------------------------
     // METHODS - casting
-    static Castable computeCastable(const AstNodeDType* toDtp, const AstNodeDType* fromDtp,
-                                    const AstNode* fromConstp) {
-        const auto castable = computeCastableImp(toDtp, fromDtp, fromConstp);
-        UINFO(9, "  castable=" << castable << "  for " << toDtp << endl);
-        UINFO(9, "     =?= " << fromDtp << endl);
-        UINFO(9, "     const= " << fromConstp << endl);
-        return castable;
-    }
     static Castable computeCastableImp(const AstNodeDType* toDtp, const AstNodeDType* fromDtp,
                                        const AstNode* fromConstp) {
         const Castable castable = UNSUPPORTED;
@@ -7536,6 +7505,15 @@ public:
         return userIterateSubtreeReturnEdits(nodep, WidthVP{SELF, BOTH}.p());
     }
     ~WidthVisitor() override = default;
+
+    static Castable computeCastable(const AstNodeDType* toDtp, const AstNodeDType* fromDtp,
+                                    const AstNode* fromConstp) {
+        const auto castable = computeCastableImp(toDtp, fromDtp, fromConstp);
+        UINFO(9, "  castable=" << castable << "  for " << toDtp << endl);
+        UINFO(9, "     =?= " << fromDtp << endl);
+        UINFO(9, "     const= " << fromConstp << endl);
+        return castable;
+    }
 };
 
 //######################################################################
@@ -7554,6 +7532,33 @@ void V3Width::width(AstNetlist* nodep) {
     V3Global::dumpCheckGlobalTree("width", 0, dumpTreeLevel() >= 3);
 }
 
+AstNodeDType* V3Width::getCommonClassTypep(AstNode* nodep1, AstNode* nodep2) {
+    // Return the class type that both nodep1 and nodep2 are castable to.
+    // If both are null, return the type of null constant.
+    // If one is a class and one is null, return AstClassRefDType that points to that class.
+    // If no common class type exists, return nullptr.
+
+    // First handle cases with null values and when one class is a super class of the other.
+    if (VN_IS(nodep1, Const)) std::swap(nodep1, nodep2);
+    const Castable castable
+        = WidthVisitor::computeCastable(nodep1->dtypep(), nodep2->dtypep(), nodep2);
+    if (castable == SAMEISH || castable == COMPATIBLE) {
+        return nodep1->dtypep();
+    } else if (castable == DYNAMIC_CLASS) {
+        return nodep2->dtypep();
+    }
+
+    AstClassRefDType* classDtypep1 = VN_CAST(nodep1->dtypep(), ClassRefDType);
+    while (classDtypep1) {
+        const Castable castable
+            = WidthVisitor::computeCastable(classDtypep1, nodep2->dtypep(), nodep2);
+        if (castable == COMPATIBLE) return classDtypep1;
+        AstClassExtends* const extendsp = classDtypep1->classp()->extendsp();
+        classDtypep1 = extendsp ? VN_AS(extendsp->dtypep(), ClassRefDType) : nullptr;
+    }
+    return nullptr;
+}
+
 //! Single node parameter propagation
 //! Smaller step... Only do a single node for parameter propagation
 AstNode* V3Width::widthParamsEdit(AstNode* nodep) {

src/V3Width.h

@@ -22,6 +22,7 @@
 
 class AstNetlist;
 class AstNode;
+class AstNodeDType;
 
 //============================================================================
 
@@ -33,6 +34,8 @@ public:
     // Final step... Mark all widths as equal
     static void widthCommit(AstNetlist* nodep);
 
+    static AstNodeDType* getCommonClassTypep(AstNode* nodep1, AstNode* nodep2);
+
     // For use only in WidthVisitor
     // Replace AstSelBit, etc with AstSel/AstArraySel
     // Returns replacement node if nodep was deleted, or null if none.

test_regress/t/t_class_if_assign.pl

@@ -0,0 +1,21 @@
+#!/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,
+    );
+
+ok(1);
+1;

test_regress/t/t_class_if_assign.v

@@ -0,0 +1,43 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed into the Public Domain, for any use,
+// without warranty, 2023 by Antmicro Ltd.
+// SPDX-License-Identifier: CC0-1.0
+
+class Cls;
+   int x;
+   function new;
+      x = 1;
+   endfunction
+endclass
+
+class ExtendCls extends Cls;
+   function new;
+      x = 2;
+   endfunction
+endclass
+
+class AnotherExtendCls extends Cls;
+   function new;
+      x = 3;
+   endfunction
+endclass
+
+module t (/*AUTOARG*/);
+   initial begin
+      Cls cls = new;
+      ExtendCls ext_cls = new;
+      AnotherExtendCls an_ext_cls = new;
+
+      if (cls.x == 1) cls = ext_cls;
+      else cls = an_ext_cls;
+      if (cls.x != 2) $stop;
+
+      if (cls.x == 1) cls = ext_cls;
+      else cls = an_ext_cls;
+      if (cls.x != 3) $stop;
+
+      $write("*-* All Finished *-*\n");
+      $finish;
+   end
+endmodule