From f858dd44ac3fcc037f94b97a2678802d0412484f Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 5 Dec 2020 16:23:20 -0500 Subject: [PATCH] Fix :: references to forward classes --- src/V3AstNodes.cpp | 6 ++--- src/V3AstNodes.h | 27 +++++++++++--------- src/V3LinkDot.cpp | 41 ++++++++++++++++++++++-------- test_regress/t/t_class_fwd_cc.pl | 13 ++++++++++ test_regress/t/t_class_fwd_cc.v | 22 ++++++++++++++++ test_regress/t/t_class_mod_bad.out | 2 +- 6 files changed, 85 insertions(+), 26 deletions(-) create mode 100755 test_regress/t/t_class_fwd_cc.pl create mode 100644 test_regress/t/t_class_fwd_cc.v diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index a8f0bcba0..55af1229f 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -1605,10 +1605,10 @@ void AstParseRef::dump(std::ostream& str) const { } void AstClassOrPackageRef::dump(std::ostream& str) const { this->AstNode::dump(str); - if (classOrPackagep()) { str << " cpkg=" << nodeAddr(classOrPackagep()); } + if (classOrPackageNodep()) str << " cpkg=" << nodeAddr(classOrPackageNodep()); str << " -> "; - if (classOrPackagep()) { - classOrPackagep()->dump(str); + if (classOrPackageNodep()) { + classOrPackageNodep()->dump(str); } else { str << "UNLINKED"; } diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 909b741fa..3bbddb483 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -2967,36 +2967,39 @@ public: class AstClassOrPackageRef final : public AstNode { private: string m_name; - AstNode* m_classOrPackagep; // Package hierarchy + // Node not NodeModule to appease some early parser usage + AstNode* m_classOrPackageNodep; // Package hierarchy public: - AstClassOrPackageRef(FileLine* fl, const string& name, AstNode* classOrPackagep, + AstClassOrPackageRef(FileLine* fl, const string& name, AstNode* classOrPackageNodep, AstNode* paramsp) : ASTGEN_SUPER(fl) , m_name{name} - , m_classOrPackagep{classOrPackagep} { + , m_classOrPackageNodep{classOrPackageNodep} { addNOp4p(paramsp); } ASTNODE_NODE_FUNCS(ClassOrPackageRef) // METHODS virtual const char* broken() const override { - BROKEN_RTN(m_classOrPackagep && !m_classOrPackagep->brokeExists()); + BROKEN_RTN(m_classOrPackageNodep && !m_classOrPackageNodep->brokeExists()); return nullptr; } virtual void cloneRelink() override { - if (m_classOrPackagep && m_classOrPackagep->clonep()) { - m_classOrPackagep = m_classOrPackagep->clonep(); + if (m_classOrPackageNodep && m_classOrPackageNodep->clonep()) { + m_classOrPackageNodep = m_classOrPackageNodep->clonep(); } } virtual bool same(const AstNode* samep) const override { - return (m_classOrPackagep - == static_cast(samep)->m_classOrPackagep); + return (m_classOrPackageNodep + == static_cast(samep)->m_classOrPackageNodep); } - virtual V3Hash sameHash() const override { return V3Hash(m_classOrPackagep); } + virtual V3Hash sameHash() const override { return V3Hash(m_classOrPackageNodep); } virtual void dump(std::ostream& str = std::cout) const override; virtual string name() const override { return m_name; } // * = Var name - AstNodeModule* classOrPackagep() const { return VN_CAST(m_classOrPackagep, NodeModule); } - AstPackage* packagep() const { return VN_CAST(classOrPackagep(), Package); } - void classOrPackagep(AstNode* nodep) { m_classOrPackagep = nodep; } + AstNode* classOrPackageNodep() const { return m_classOrPackageNodep; } + void classOrPackageNodep(AstNode* nodep) { m_classOrPackageNodep = nodep; } + AstNodeModule* classOrPackagep() const { return VN_CAST(m_classOrPackageNodep, NodeModule); } + AstPackage* packagep() const { return VN_CAST(classOrPackageNodep(), Package); } + void classOrPackagep(AstNodeModule* nodep) { m_classOrPackageNodep = nodep; } AstPin* paramsp() const { return VN_CAST(op4p(), Pin); } }; diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index 5edb41a03..413636d81 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -1476,6 +1476,28 @@ private: // We're done with implicit gates VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep); } + virtual void visit(AstClassOrPackageRef* nodep) override { + if (auto* fwdp = VN_CAST(nodep->classOrPackageNodep(), TypedefFwd)) { + // Relink forward definitions to the "real" definition + VSymEnt* foundp = m_statep->getNodeSym(fwdp)->findIdFallback(fwdp->name()); + if (foundp && (VN_IS(foundp->nodep(), Class) || VN_IS(foundp->nodep(), Package))) { + nodep->classOrPackagep(VN_CAST(foundp->nodep(), NodeModule)); + } else if (foundp && VN_IS(foundp->nodep(), ParamTypeDType)) { + UASSERT(m_statep->forPrimary(), "Param types should have been resolved"); + nodep->classOrPackageNodep(foundp->nodep()); + } else { + if (foundp) UINFO(1, "found nodep = " << foundp->nodep() << endl); + nodep->v3error( + "Forward typedef used as class/package does not resolve to class/package: " + << nodep->prettyNameQ() << '\n' + << nodep->warnContextPrimary() << '\n' + << (foundp ? nodep->warnMore() + "... Object with matching name\n" + + foundp->nodep()->warnContextSecondary() + : "")); + } + } + iterateChildren(nodep); + } virtual void visit(AstTypedefFwd* nodep) override { VSymEnt* foundp = m_statep->getNodeSym(nodep)->findIdFallback(nodep->name()); if (!foundp && v3Global.opt.pedantic()) { @@ -1487,7 +1509,8 @@ private: << nodep->prettyNameQ()); } // We only needed the forward declaration in order to parse correctly. - VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep); + // Delete later as may be ClassOrPackageRef's still pointing to it + VL_DO_DANGLING(pushDeletep(nodep->unlinkFrBack()), nodep); } virtual void visit(AstNode* nodep) override { iterateChildren(nodep); } @@ -2563,9 +2586,6 @@ private: nodep->v3warn(E_UNSUPPORTED, "Unsupported: " << AstNode::prettyNameQ(cpackagerefp->name())); } - if (cpackagerefp->paramsp()) { - nodep->v3warn(E_UNSUPPORTED, "Unsupported: parameterized packages for task calls"); - } UASSERT_OBJ(cpackagerefp->classOrPackagep(), m_ds.m_dotp->lhsp(), "Bad package link"); nodep->classOrPackagep(cpackagerefp->classOrPackagep()); m_ds.m_dotPos = DP_SCOPE; @@ -2895,8 +2915,10 @@ private: if (!VN_IS(nodep->classOrPackagep(), Class) && !VN_IS(nodep->classOrPackagep(), Package)) { cpackagerefp->v3error( - "'::' expected to reference a class/package but referenced " - << nodep->classOrPackagep()->prettyTypeName() << '\n' + "'::' expected to reference a class/package but referenced '" + << (nodep->classOrPackagep() ? nodep->classOrPackagep()->prettyTypeName() + : "") + << "'\n" << cpackagerefp->warnMore() + "... Suggest '.' instead of '::'"); } } else { @@ -2908,10 +2930,9 @@ private: if (m_ds.m_dotp && m_ds.m_dotPos == DP_PACKAGE) { UASSERT_OBJ(VN_IS(m_ds.m_dotp->lhsp(), ClassOrPackageRef), m_ds.m_dotp->lhsp(), "Bad package link"); - UASSERT_OBJ(VN_CAST(m_ds.m_dotp->lhsp(), ClassOrPackageRef)->classOrPackagep(), - m_ds.m_dotp->lhsp(), "Bad package link"); - nodep->classOrPackagep( - VN_CAST(m_ds.m_dotp->lhsp(), ClassOrPackageRef)->classOrPackagep()); + auto* cpackagerefp = VN_CAST(m_ds.m_dotp->lhsp(), ClassOrPackageRef); + UASSERT_OBJ(cpackagerefp->classOrPackagep(), m_ds.m_dotp->lhsp(), "Bad package link"); + nodep->classOrPackagep(cpackagerefp->classOrPackagep()); m_ds.m_dotPos = DP_SCOPE; m_ds.m_dotp = nullptr; } else { diff --git a/test_regress/t/t_class_fwd_cc.pl b/test_regress/t/t_class_fwd_cc.pl new file mode 100755 index 000000000..7314796b8 --- /dev/null +++ b/test_regress/t/t_class_fwd_cc.pl @@ -0,0 +1,13 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(vlt => 1); + +lint( + ); + +ok(1); +1; diff --git a/test_regress/t/t_class_fwd_cc.v b/test_regress/t/t_class_fwd_cc.v new file mode 100644 index 000000000..dce477fdf --- /dev/null +++ b/test_regress/t/t_class_fwd_cc.v @@ -0,0 +1,22 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2020 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +package Pkg; + typedef class Fwd; + virtual class Virt; + pure virtual function Fwd get_root(); + endclass + class Ext extends Virt; + virtual function Fwd get_root(); + return Fwd::m_uvm_get_root(); + endfunction + endclass + class Fwd; + function Fwd m_uvm_get_root(); + return null; + endfunction + endclass +endpackage diff --git a/test_regress/t/t_class_mod_bad.out b/test_regress/t/t_class_mod_bad.out index 8a24ba164..08ae3c20d 100644 --- a/test_regress/t/t_class_mod_bad.out +++ b/test_regress/t/t_class_mod_bad.out @@ -1,4 +1,4 @@ -%Error: t/t_class_mod_bad.v:21:7: '::' expected to reference a class/package but referenced MODULE 'M' +%Error: t/t_class_mod_bad.v:21:7: '::' expected to reference a class/package but referenced 'MODULE 'M'' : ... Suggest '.' instead of '::' 21 | M::Cls p; | ^