Fix compilation error on multi-inherited interface class usage (#4819).

Changes

@@ -29,6 +29,7 @@ Verilator 5.021 devel
 * Fix delays using wrong timeunit when modules inlined (#4806). [Paul Wright]
 * Fix warnings in verilated_sc_trace.h for Clang. (#4807) (#4827). [Anthony Donlon]
 * Fix null pointer dereference (#4810) (#4825). [Adrian Sampson]
+* Fix compilation error on multi-inherited interface class usage (#4819).
 * Fix maybe-uninitialized compiler warning (#4820) (#4822). [Larry Doolittle]
 * Fix mis-splitting of dump control functions (#4821). [Fan Shupei]
 * Fix wrong utimes() parameter (#4829). [Szymon Gizler]

src/V3AstNodeOther.h

@@ -593,7 +593,6 @@ class AstCFunc final : public AstNode {
     string m_cname;  // C name, for dpiExports
     string m_rtnType;  // void, bool, or other return type
     string m_argTypes;  // Argument types
-    string m_baseCtors;  // Base class constructor
     string m_ifdef;  // #ifdef symbol around this function
     VBoolOrUnknown m_isConst;  // Function is declared const (*this not changed)
     bool m_isStatic : 1;  // Function is static (no need for a 'this' pointer)
@@ -656,8 +655,7 @@ public:
     bool same(const AstNode* samep) const override {
         const AstCFunc* const asamep = VN_DBG_AS(samep, CFunc);
         return ((isTrace() == asamep->isTrace()) && (rtnTypeVoid() == asamep->rtnTypeVoid())
-                && (argTypes() == asamep->argTypes()) && (baseCtors() == asamep->baseCtors())
-                && isLoose() == asamep->isLoose()
+                && (argTypes() == asamep->argTypes()) && isLoose() == asamep->isLoose()
                 && (!(dpiImportPrototype() || dpiExportImpl()) || name() == asamep->name()));
     }
     //
@@ -689,8 +687,6 @@ public:
     void funcPublic(bool flag) { m_funcPublic = flag; }
     void argTypes(const string& str) { m_argTypes = str; }
     string argTypes() const { return m_argTypes; }
-    void baseCtors(const string& str) { m_baseCtors = str; }
-    string baseCtors() const { return m_baseCtors; }
     void ifdef(const string& str) { m_ifdef = str; }
     string ifdef() const { return m_ifdef; }
     bool isConstructor() const { return m_isConstructor; }
@@ -2255,8 +2251,9 @@ class AstClass final : public AstNodeModule {
     bool m_extended = false;  // Is extension or extended by other classes
     bool m_interfaceClass = false;  // Interface class
     bool m_needRNG = false;  // Need RNG, uses srandom/randomize
-    bool m_virtual = false;  // Virtual class
     bool m_parameterized = false;  // Parameterized class
+    bool m_useVirtualPublic = false;  // Subclasses need virtual public as uses interface class
+    bool m_virtual = false;  // Virtual class
 
 public:
     AstClass(FileLine* fl, const string& name)
@@ -2274,12 +2271,14 @@ public:
     void isExtended(bool flag) { m_extended = flag; }
     bool isInterfaceClass() const { return m_interfaceClass; }
     void isInterfaceClass(bool flag) { m_interfaceClass = flag; }
+    bool isParameterized() const { return m_parameterized; }
+    void isParameterized(bool flag) { m_parameterized = flag; }
     bool isVirtual() const { return m_virtual; }
     void isVirtual(bool flag) { m_virtual = flag; }
     bool needRNG() const { return m_needRNG; }
     void needRNG(bool flag) { m_needRNG = flag; }
-    bool isParameterized() const { return m_parameterized; }
-    void isParameterized(bool flag) { m_parameterized = flag; }
+    bool useVirtualPublic() const { return m_useVirtualPublic; }
+    void useVirtualPublic(bool flag) { m_useVirtualPublic = flag; }
     // Return true if this class is an extension of base class (SLOW)
     // Accepts nullptrs
     static bool isClassExtendedFrom(const AstClass* refClassp, const AstClass* baseClassp);

src/V3AstNodes.cpp

@@ -1406,6 +1406,7 @@ void AstClass::dump(std::ostream& str) const {
     if (isExtended()) str << " [EXT]";
     if (isInterfaceClass()) str << " [IFCCLS]";
     if (isVirtual()) str << " [VIRT]";
+    if (useVirtualPublic()) str << " [VIRPUB]";
 }
 void AstClassExtends::dump(std::ostream& str) const {
     this->AstNode::dump(str);

src/V3Class.cpp

@@ -49,12 +49,39 @@ class ClassVisitor final : public VNVisitor {
 
     // METHODS
 
+    bool recurseImplements(AstClass* nodep, bool setit) {
+        // Returns true to set useVirtualPublic().
+        // If there's an implements of an interface class then we have
+        // multiple classes that point to same object, that need same
+        // VlClass (the diamond problem). C++ will require we use 'virtual
+        // public' for VlClass.  So, we need the interface class, and all
+        // classes above, and any below using any implements to use
+        // 'virtual public' via useVirtualPublic().
+        if (nodep->useVirtualPublic()) return true;  // Short-circuit
+        if (nodep->isInterfaceClass()) setit = true;
+        for (const AstClassExtends* extp = nodep->extendsp(); extp;
+             extp = VN_AS(extp->nextp(), ClassExtends)) {
+            if (recurseImplements(extp->classp(), setit)) setit = true;
+        }
+        if (setit) {
+            nodep->useVirtualPublic(true);
+            for (const AstClassExtends* extp = nodep->extendsp(); extp;
+                 extp = VN_AS(extp->nextp(), ClassExtends)) {
+                (void)recurseImplements(extp->classp(), true);
+            }
+        }
+        return setit;
+    }
+
+    // VISITORS
+
     void visit(AstClass* nodep) override {
         if (nodep->user1SetOnce()) return;
         // Move this class
         nodep->name(m_prefix + nodep->name());
         nodep->unlinkFrBack();
         v3Global.rootp()->addModulesp(nodep);
+        (void)recurseImplements(nodep, false);
         // Make containing package
         // Note origName is the same as the class origName so errors look correct
         AstClassPackage* const packagep

src/V3EmitCFunc.h

@@ -248,6 +248,32 @@ public:
         return nullptr;
     }
 
+    void putConstructorSubinit(const AstClass* classp, AstCFunc* cfuncp, bool top, bool& firstr) {
+        for (const AstClassExtends* extp = classp->extendsp(); extp;
+             extp = VN_AS(extp->nextp(), ClassExtends)) {
+            if (extp->classp()->useVirtualPublic()) {
+                // It's a c++ virtual class (diamond relation)
+                // Must get the subclasses initialized first
+                putConstructorSubinit(extp->classp(), cfuncp, false, firstr);
+            }
+            puts(firstr ? "" : "\n, ");
+            firstr = false;
+            puts(prefixNameProtect(extp->classp()));
+            if (constructorNeedsProcess(extp->classp())) {
+                puts("(vlProcess, vlSymsp");
+            } else {
+                puts("(vlSymsp");
+            }
+            if (top) {
+                const AstCNew* const superNewCallp = getSuperNewCallRecursep(cfuncp->stmtsp());
+                UASSERT_OBJ(superNewCallp, cfuncp, "super.new call not found");
+                putCommaIterateNext(superNewCallp->argsp(), true);
+            }
+            puts(")");
+            top = false;
+        }
+    }
+
     // VISITORS
     using EmitCConstInit::visit;
     void visit(AstCFunc* nodep) override {
@@ -265,20 +291,13 @@ public:
         if (nodep->isInline()) puts("VL_INLINE_OPT ");
         emitCFuncHeader(nodep, m_modp, /* withScope: */ true);
 
-        if (!nodep->baseCtors().empty()) {
-            puts(": ");
-            puts(nodep->baseCtors());
+        if (nodep->isConstructor()) {
             const AstClass* const classp = VN_CAST(nodep->scopep()->modp(), Class);
-            // Find call to super.new to get the arguments
-            if (classp && constructorNeedsProcess(classp)) {
-                puts("(vlProcess, vlSymsp");
-            } else {
-                puts("(vlSymsp");
+            if (nodep->isConstructor() && classp && classp->extendsp()) {
+                puts("\n    : ");
+                bool first = true;
+                putConstructorSubinit(classp, nodep, true, first /*ref*/);
             }
-            const AstCNew* const superNewCallp = getSuperNewCallRecursep(nodep->stmtsp());
-            UASSERT_OBJ(superNewCallp, nodep, "super.new call not found");
-            putCommaIterateNext(superNewCallp->argsp(), true);
-            puts(")");
         }
         puts(" {\n");
 

src/V3EmitCHeaders.cpp

@@ -316,9 +316,14 @@ class EmitCHeader final : public EmitCConstInit {
         if (!VN_IS(modp, Class)) puts("alignas(VL_CACHE_LINE_BYTES) ");
         puts(prefixNameProtect(modp));
         if (const AstClass* const classp = VN_CAST(modp, Class)) {
-            puts(" : public ");
+            const string virtpub = classp->useVirtualPublic() ? "virtual public " : "public ";
+            puts(" : " + virtpub);
             if (classp->extendsp()) {
-                puts(prefixNameProtect(classp->extendsp()->classp()));
+                for (const AstClassExtends* extp = classp->extendsp(); extp;
+                     extp = VN_AS(extp->nextp(), ClassExtends)) {
+                    puts(prefixNameProtect(extp->classp()));
+                    if (extp->nextp()) puts(", " + virtpub);
+                }
             } else {
                 puts("VlClass");
             }

src/V3Task.cpp

@@ -1223,13 +1223,7 @@ class TaskVisitor final : public VNVisitor {
         }
         cfuncp->isVirtual(nodep->isVirtual());
         cfuncp->dpiPure(nodep->dpiPure());
-        if (nodep->name() == "new") {
-            cfuncp->isConstructor(true);
-            AstClass* const classp = m_statep->getClassp(nodep);
-            if (classp->extendsp()) {
-                cfuncp->baseCtors(EmitCBase::prefixNameProtect(classp->extendsp()->classp()));
-            }
-        }
+        if (nodep->name() == "new") cfuncp->isConstructor(true);
         if (cfuncp->dpiExportImpl()) cfuncp->cname(nodep->cname());
 
         if (!nodep->dpiImport() && !nodep->taskPublic()) {

test_regress/t/t_class_imp2.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 2024 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_imp2.v

@@ -0,0 +1,47 @@
+// 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
+
+module t(/*AUTOARG*/);
+
+   interface class Courier;
+      pure virtual function void deliver();
+   endclass
+   class Person implements Courier;
+      virtual function void deliver();
+         $display("slow delivery");
+      endfunction
+   endclass
+
+   interface class Seats;
+      pure virtual function int seats();
+   endclass
+
+   class Vehicle;
+   endclass
+
+   class Car extends Vehicle implements Courier, Seats;
+      virtual function void deliver();
+         $display("fast delivery");
+      endfunction
+      virtual function int seats(); return 4; endfunction
+   endclass
+
+   class MetaCar extends Car;
+   endclass
+
+   function void do_delivery(Courier courier);
+      courier.deliver();
+   endfunction
+
+   initial begin
+      MetaCar car;
+      car = new();
+      do_delivery(car);
+      if (car.seats() != 4) $stop;
+      $write("*-* All Finished *-*\n");
+      $finish;
+   end
+endmodule