From d0ec6092b39f5ea06cc0d1c0b99d3580cb22d3dc Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 28 Sep 2024 20:55:22 -0400 Subject: [PATCH] Change package import/export to link post-parsing, prep for later commit. --- src/V3AstNodeOther.h | 34 ++++++++++++++++++++--- src/V3AstNodes.cpp | 18 ++++++++++-- src/V3LinkCells.cpp | 22 ++++++++++++++- src/V3LinkDot.cpp | 7 ++--- src/verilog.y | 4 +-- test_regress/t/t_dist_warn_coverage.py | 1 + test_regress/t/t_lint_import_name_bad.out | 2 +- test_regress/t/t_package_export_bad2.out | 4 +++ test_regress/t/t_package_export_bad2.py | 16 +++++++++++ test_regress/t/t_package_export_bad2.v | 16 +++++++++++ test_regress/t/t_package_import_bad2.out | 4 +++ test_regress/t/t_package_import_bad2.py | 16 +++++++++++ test_regress/t/t_package_import_bad2.v | 16 +++++++++++ 13 files changed, 146 insertions(+), 14 deletions(-) create mode 100644 test_regress/t/t_package_export_bad2.out create mode 100755 test_regress/t/t_package_export_bad2.py create mode 100644 test_regress/t/t_package_export_bad2.v create mode 100644 test_regress/t/t_package_import_bad2.out create mode 100755 test_regress/t/t_package_import_bad2.py create mode 100644 test_regress/t/t_package_import_bad2.v diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index d5e38efc9..591d5f5b7 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -1339,19 +1339,32 @@ class AstPackageExport final : public AstNode { // A package export declaration // // @astgen ptr := m_packagep : Optional[AstPackage] // Package hierarchy - string m_name; + string m_name; // What imported e.g. "*" + string m_pkgName; // Module the cell instances public: AstPackageExport(FileLine* fl, AstPackage* packagep, const string& name) : ASTGEN_SUPER_PackageExport(fl) , m_name{name} - , m_packagep{packagep} {} + , m_packagep{packagep} { + pkgNameFrom(); + } + AstPackageExport(FileLine* fl, const string& pkgName, const string& name) + : ASTGEN_SUPER_PackageExport(fl) + , m_name{name} + , m_pkgName{pkgName} + , m_packagep{nullptr} {} ASTGEN_MEMBERS_AstPackageExport; void dump(std::ostream& str) const override; void dumpJson(std::ostream& str) const override; string name() const override VL_MT_STABLE { return m_name; } + string pkgName() const VL_MT_STABLE { return m_pkgName; } + string prettyPkgNameQ() const { return "'" + prettyName(pkgName()) + "'"; } AstPackage* packagep() const { return m_packagep; } void packagep(AstPackage* nodep) { m_packagep = nodep; } + +private: + void pkgNameFrom(); }; class AstPackageExportStarStar final : public AstNode { // A package export *::* declaration @@ -1365,19 +1378,32 @@ class AstPackageImport final : public AstNode { // A package import declaration // // @astgen ptr := m_packagep : Optional[AstPackage] // Package hierarchy - string m_name; + string m_name; // What imported e.g. "*" + string m_pkgName; // Module the cell instances public: AstPackageImport(FileLine* fl, AstPackage* packagep, const string& name) : ASTGEN_SUPER_PackageImport(fl) , m_name{name} - , m_packagep{packagep} {} + , m_packagep{packagep} { + pkgNameFrom(); + } + AstPackageImport(FileLine* fl, const string& pkgName, const string& name) + : ASTGEN_SUPER_PackageImport(fl) + , m_name{name} + , m_pkgName{pkgName} + , m_packagep{nullptr} {} ASTGEN_MEMBERS_AstPackageImport; void dump(std::ostream& str) const override; void dumpJson(std::ostream& str) const override; string name() const override VL_MT_STABLE { return m_name; } + string pkgName() const VL_MT_STABLE { return m_pkgName; } + string prettyPkgNameQ() const { return "'" + prettyName(pkgName()) + "'"; } AstPackage* packagep() const { return m_packagep; } void packagep(AstPackage* nodep) { m_packagep = nodep; } + +private: + void pkgNameFrom(); }; class AstPin final : public AstNode { // A port or parameter assignment on an instantiation diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 5b66bef45..0249e9ea2 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -2196,14 +2196,28 @@ void AstNodeModule::dumpJson(std::ostream& str) const { } void AstPackageExport::dump(std::ostream& str) const { this->AstNode::dump(str); - str << " -> " << packagep(); + if (packagep()) { + str << " -> " << packagep(); + } else { + str << " ->UNLINKED:" << pkgName(); + } } void AstPackageExport::dumpJson(std::ostream& str) const { dumpJsonGen(str); } +void AstPackageExport::pkgNameFrom() { + if (packagep()) m_pkgName = packagep()->name(); +} void AstPackageImport::dump(std::ostream& str) const { this->AstNode::dump(str); - str << " -> " << packagep(); + if (packagep()) { + str << " -> " << packagep(); + } else { + str << " ->UNLINKED:" << pkgName(); + } } void AstPackageImport::dumpJson(std::ostream& str) const { dumpJsonGen(str); } +void AstPackageImport::pkgNameFrom() { + if (packagep()) m_pkgName = packagep()->name(); +} void AstPatMember::dump(std::ostream& str) const { this->AstNodeExpr::dump(str); if (isDefault()) str << " [DEFAULT]"; diff --git a/src/V3LinkCells.cpp b/src/V3LinkCells.cpp index c5ed8bc35..c240cf76a 100644 --- a/src/V3LinkCells.cpp +++ b/src/V3LinkCells.cpp @@ -247,10 +247,30 @@ class LinkCellsVisitor final : public VNVisitor { // Note cannot do modport resolution here; modports are allowed underneath generates } + void visit(AstPackageExport* nodep) override { + // Package Import: We need to do the package before the use of a package + iterateChildren(nodep); + if (!nodep->packagep()) { + AstNodeModule* const modp = resolveModule(nodep, nodep->pkgName()); + if (AstPackage* const pkgp = VN_CAST(modp, Package)) nodep->packagep(pkgp); + if (!nodep->packagep()) { + nodep->v3error("Export package not found: " << nodep->prettyPkgNameQ()); + return; + } + } + } void visit(AstPackageImport* nodep) override { // Package Import: We need to do the package before the use of a package iterateChildren(nodep); - UASSERT_OBJ(nodep->packagep(), nodep, "Unlinked package"); // Parser should set packagep + if (!nodep->packagep()) { + AstNodeModule* const modp = resolveModule(nodep, nodep->pkgName()); + if (AstPackage* const pkgp = VN_CAST(modp, Package)) nodep->packagep(pkgp); + // If not found, V3LinkDot will report errors + if (!nodep->packagep()) { + nodep->v3error("Import package not found: " << nodep->prettyPkgNameQ()); + return; + } + } new V3GraphEdge{&m_graph, vertex(m_modp), vertex(nodep->packagep()), 1, false}; } diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index 140e71ec1..42d593cb0 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -1496,6 +1496,7 @@ class LinkDotFindVisitor final : public VNVisitor { } void visit(AstPackageImport* nodep) override { UINFO(4, " Link: " << nodep << endl); + if (!nodep->packagep()) return; // Errored in V3LinkCells VSymEnt* const srcp = m_statep->getNodeSym(nodep->packagep()); if (nodep->name() == "*") { if (nodep->packagep() != v3Global.rootp()->stdPackagep()) { @@ -1506,10 +1507,7 @@ class LinkDotFindVisitor final : public VNVisitor { } } else { VSymEnt* const impp = srcp->findIdFlat(nodep->name()); - if (!impp) { - nodep->v3error("Import object not found: '" << nodep->packagep()->prettyName() - << "::" << nodep->prettyName() << "'"); - } + if (!impp) { nodep->v3error("Import object not found: " << nodep->prettyPkgNameQ()); } } m_curSymp->importFromPackage(m_statep->symsp(), srcp, nodep->name()); UINFO(9, " Link Done: " << nodep << endl); @@ -1517,6 +1515,7 @@ class LinkDotFindVisitor final : public VNVisitor { } void visit(AstPackageExport* nodep) override { UINFO(9, " Link: " << nodep << endl); + if (!nodep->packagep()) return; // Errored in V3LinkCells VSymEnt* const srcp = m_statep->getNodeSym(nodep->packagep()); if (nodep->name() != "*") { VSymEnt* const impp = srcp->findIdFlat(nodep->name()); diff --git a/src/verilog.y b/src/verilog.y index cf1a36b18..05304ad09 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -1325,7 +1325,7 @@ package_import_item: // ==IEEE: package_import_item $$ = nullptr; $1->v3error("Importing from missing package '" << *$1 << "'"); } else { - $$ = new AstPackageImport{$2, VN_CAST($1, Package), *$3}; + $$ = new AstPackageImport{$2, *$1, *$3}; SYMP->importItem($1, *$3); } } ; @@ -1348,7 +1348,7 @@ package_export_itemList: package_export_item: // ==IEEE: package_export_item idCC yP_COLONCOLON package_import_itemObj - { $$ = new AstPackageExport{$3, VN_CAST($1, Package), *$3}; + { $$ = new AstPackageExport{$3, *$1, *$3}; if ($1) SYMP->exportItem($1, *$3); } ; diff --git a/test_regress/t/t_dist_warn_coverage.py b/test_regress/t/t_dist_warn_coverage.py index ebeba7be6..3ad81cedc 100755 --- a/test_regress/t/t_dist_warn_coverage.py +++ b/test_regress/t/t_dist_warn_coverage.py @@ -22,6 +22,7 @@ for s in [ 'EOF in unterminated string', # Instead get normal unterminated 'Enum names without values only allowed on numeric types', # Hard to hit 'Enum ranges must be integral, per spec', # Hard to hit + 'Import package not found: ', # Errors earlier, until future parser released 'Return with return value isn\'t underneath a function', # Hard to hit, get other bad return messages 'Syntax error: Range \':\', \'+:\' etc are not allowed in the instance ', # Instead get syntax error 'Syntax error parsing real: \'', # Instead can't lex the number diff --git a/test_regress/t/t_lint_import_name_bad.out b/test_regress/t/t_lint_import_name_bad.out index 36a7cf18e..03aadb71a 100644 --- a/test_regress/t/t_lint_import_name_bad.out +++ b/test_regress/t/t_lint_import_name_bad.out @@ -1,4 +1,4 @@ -%Error: t/t_lint_import_name_bad.v:11:12: Import object not found: 'defs::sigs' +%Error: t/t_lint_import_name_bad.v:11:12: Import object not found: 'defs' 11 | import defs::sigs; | ^~ %Error: Exiting due to diff --git a/test_regress/t/t_package_export_bad2.out b/test_regress/t/t_package_export_bad2.out new file mode 100644 index 000000000..82934dfb3 --- /dev/null +++ b/test_regress/t/t_package_export_bad2.out @@ -0,0 +1,4 @@ +%Error: t/t_package_export_bad2.v:12:18: Export package not found: 'Pkg1b' + 12 | export Pkg1b::*; + | ^ +%Error: Exiting due to diff --git a/test_regress/t/t_package_export_bad2.py b/test_regress/t/t_package_export_bad2.py new file mode 100755 index 000000000..31228c9a7 --- /dev/null +++ b/test_regress/t/t_package_export_bad2.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# 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 + +import vltest_bootstrap + +test.scenarios('linter') + +test.lint(fails=True, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_package_export_bad2.v b/test_regress/t/t_package_export_bad2.v new file mode 100644 index 000000000..e2075d7a9 --- /dev/null +++ b/test_regress/t/t_package_export_bad2.v @@ -0,0 +1,16 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2024 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +package Pkg1; +endpackage + +package Pkg10; + // verilator lint_off PKGNODECL + export Pkg1b::*; // BAD - typo in package name +endpackage + +module t (/*AUTOARG*/); +endmodule diff --git a/test_regress/t/t_package_import_bad2.out b/test_regress/t/t_package_import_bad2.out new file mode 100644 index 000000000..a0aed8c9a --- /dev/null +++ b/test_regress/t/t_package_import_bad2.out @@ -0,0 +1,4 @@ +%Error: t/t_package_import_bad2.v:12:11: Importing from missing package 'Pkg1b' + 12 | import Pkg1b::*; + | ^~~~~ +%Error: Exiting due to diff --git a/test_regress/t/t_package_import_bad2.py b/test_regress/t/t_package_import_bad2.py new file mode 100755 index 000000000..31228c9a7 --- /dev/null +++ b/test_regress/t/t_package_import_bad2.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# 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 + +import vltest_bootstrap + +test.scenarios('linter') + +test.lint(fails=True, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_package_import_bad2.v b/test_regress/t/t_package_import_bad2.v new file mode 100644 index 000000000..fd8501c51 --- /dev/null +++ b/test_regress/t/t_package_import_bad2.v @@ -0,0 +1,16 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2024 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +package Pkg1; +endpackage + +package Pkg10; + // verilator lint_off PKGNODECL + import Pkg1b::*; // BAD - typo in package name +endpackage + +module t (/*AUTOARG*/); +endmodule