Fix constructor parameters in inheritance hierarchies (#6036) (#6070)

This commit is contained in:
Petr Nohavica 2025-07-11 19:10:36 +02:00 committed by GitHub
parent 58b867c39c
commit 0982260d3b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 174 additions and 45 deletions

View File

@ -57,28 +57,20 @@ 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) {
void recurseImplements(AstClass* nodep) {
// In SystemVerilog, we have two inheritance chains:
// - extends of concrete clasess: mapped to non-virtual C++ inheritance
// as there is only single ancestor allowed
// - implements of concrete classes / extends of interface classes: mapped
// to virtual inheritance to allow diamond patterns with multiple ancestors
if (nodep->useVirtualPublic()) return; // Short-circuit to exit diamond cycles
if (nodep->isInterfaceClass()) {
nodep->useVirtualPublic(true);
for (const AstClassExtends* extp = nodep->extendsp(); extp;
extp = VN_AS(extp->nextp(), ClassExtends)) {
(void)recurseImplements(extp->classp(), true);
}
}
return setit;
for (const AstClassExtends* extp = nodep->extendsp(); extp;
extp = VN_AS(extp->nextp(), ClassExtends)) {
recurseImplements(extp->classp());
}
}
// VISITORS
@ -89,7 +81,7 @@ class ClassVisitor final : public VNVisitor {
nodep->name(m_prefix + nodep->name());
nodep->unlinkFrBack();
v3Global.rootp()->addModulesp(nodep);
(void)recurseImplements(nodep, false);
recurseImplements(nodep);
// Make containing package
// Note origName is the same as the class origName so errors look correct
AstClassPackage* const packagep

View File

@ -261,33 +261,61 @@ public:
if (const AstCNew* const cnewp = getSuperNewCallRecursep(nodep->nextp())) return cnewp;
return nullptr;
}
void putConstructorSubinit(const AstClass* classp, AstCFunc* cfuncp, bool top,
std::set<AstClass*>& doneClassesr) {
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, doneClassesr);
void putConstructorSubinit(const AstClass* classp, AstCFunc* cfuncp) {
// Virtual bases in depth-first left-to-right order
std::vector<AstClass*> virtualBases;
std::unordered_set<AstClass*> doneClasses;
collectVirtualBasesRecursep(classp, virtualBases);
for (AstClass* vbase : virtualBases) {
if (doneClasses.count(vbase)) continue;
puts(doneClasses.empty() ? "" : "\n , ");
doneClasses.emplace(vbase);
puts(prefixNameProtect(vbase));
if (constructorNeedsProcess(vbase)) {
puts("(vlProcess, vlSymsp)");
} else {
puts("(vlSymsp)");
}
// Diamond pattern with same base class twice?
if (doneClassesr.find(extp->classp()) != doneClassesr.end()) continue;
puts(doneClassesr.empty() ? "" : "\n , ");
doneClassesr.emplace(extp->classp());
}
const AstCNew* const superNewCallp =
getSuperNewCallRecursep(cfuncp->stmtsp());
// Direct non-virtual bases in declaration order
for (const AstClassExtends* extp = classp->extendsp(); extp;
extp = VN_AS(extp->nextp(), ClassExtends)) {
if (extp->classp()->useVirtualPublic()) continue;
if (doneClasses.count(extp->classp())) continue;
puts(doneClasses.empty() ? "" : "\n , ");
doneClasses.emplace(extp->classp());
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");
// Handle super.new() args for the concrete parent
if (!extp->classp()->isInterfaceClass() && superNewCallp) {
putCommaIterateNext(superNewCallp->argsp(), true);
}
puts(")");
top = false;
}
}
void collectVirtualBasesRecursep(const AstClass* classp,
std::vector<AstClass*>& virtualBases) {
std::set<const AstClass*> visited;
collectVirtualBasesRecursep(classp, virtualBases /*ref*/, visited /*ref*/);
}
void collectVirtualBasesRecursep(const AstClass* classp,
std::vector<AstClass*>& virtualBases,
std::set<const AstClass*>& visited) {
if (visited.count(classp)) return;
visited.emplace(classp);
for (const AstClassExtends* extp = classp->extendsp(); extp;
extp = VN_AS(extp->nextp(), ClassExtends)) {
// Depth-first: recurse into this base first
collectVirtualBasesRecursep(extp->classp(), virtualBases, visited);
if (extp->classp()->useVirtualPublic()) {
virtualBases.push_back(extp->classp());
}
}
}
@ -314,8 +342,7 @@ public:
const AstClass* const classp = VN_CAST(nodep->scopep()->modp(), Class);
if (nodep->isConstructor() && classp && classp->extendsp()) {
puts("\n : ");
std::set<AstClass*> doneClasses;
putConstructorSubinit(classp, nodep, true, doneClasses /*ref*/);
putConstructorSubinit(classp, nodep);
}
}
puts(" {\n");

View File

@ -564,16 +564,20 @@ 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)) {
const string virtpub = classp->useVirtualPublic() ? "virtual public " : "public ";
puts(" : " + virtpub);
puts(" : ");
if (classp->extendsp()) {
bool needComma = false;
for (const AstClassExtends* extp = classp->extendsp(); extp;
extp = VN_AS(extp->nextp(), ClassExtends)) {
extp = VN_AS(extp->nextp(), ClassExtends)) {
if (needComma) puts(", ");
// Use virtual only for interfaces for class inheritance
// (extends)
puts(extp->classp()->useVirtualPublic() ? "virtual public " : "public ");
putns(extp, prefixNameProtect(extp->classp()));
if (extp->nextp()) puts(", " + virtpub);
needComma = true;
}
} else {
puts("VlClass");
puts("public virtual VlClass");
}
} else {
puts(" final : public VerilatedModule");

View File

@ -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('simulator')
test.compile()
test.execute()
test.passes()

View File

@ -0,0 +1,88 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty, 2025 by Petr Nohavica
// SPDX-License-Identifier: CC0-1.0
`define stop $stop
`define checkh(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0);
`define checks(gotv,expv) do if ((gotv) != (expv)) begin $write("%%Error: %s:%0d: got='%s' exp='%s'\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0);
interface class IBottomMid;
pure virtual function void moo(int i);
endclass
interface class IBottom;
pure virtual function bit foo();
endclass
interface class IMid extends IBottomMid;
pure virtual function string bar();
endclass
class bottom_class implements IBottom;
string name;
function new(string name);
this.name = name;
endfunction
virtual function bit foo();
$display("%s", name);
endfunction
endclass
class middle_class extends bottom_class implements IMid, IBottom;
function new(string name);
super.new($sformatf("middle %0s", name));
endfunction
virtual function bit foo();
$display("%s", name);
return 0;
endfunction
virtual function void moo(int i);
$display("moo: %d", i);
endfunction
virtual function string bar();
return name;
endfunction
endclass
class top_class extends middle_class;
int i;
function new(string name, int i);
super.new($sformatf("%0s %0d", name, i));
this.i = i;
endfunction
endclass
class sky_class extends top_class;
function new(string name);
super.new(name, 42);
endfunction
endclass
module t;
initial begin
sky_class s = new("ahoj");
bottom_class b = s;
top_class t = s;
IMid im;
`checks( b.name, "middle ahoj 42" );
`checks( s.name, "middle ahoj 42" );
`checks( t.name, "middle ahoj 42" );
`checkh( t.i, 42);
`checks(s.bar(), "middle ahoj 42");
im = s;
im.moo(42);
$finish;
end
endmodule