diff --git a/Changes b/Changes index 05f5ca324..a394e3da6 100644 --- a/Changes +++ b/Changes @@ -30,6 +30,7 @@ Verilator 5.029 devel * Support inside array constraints (#5448). [Arkadiusz Kozdra, Antmicro Ltd.] * Support DPI imports and exports with double underscores (#5481). * Support ccache when compiling Verilated files with cmake. +* Support `local` and `protected` on `typedef` (#5460). * Add error on misused genvar (#408). [Alex Solomatnikov] * Add error on instances without parenthesis. * Add Docker pre-commit hook (#5238) (#5452). [Chris Bachhuber] diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index e2d890e43..c68217366 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -1714,12 +1714,16 @@ class AstTypedef final : public AstNode { string m_name; string m_tag; // Holds the string of the verilator tag -- used in XML output. bool m_attrPublic = false; + bool m_isHideLocal : 1; // Verilog local + bool m_isHideProtected : 1; // Verilog protected public: AstTypedef(FileLine* fl, const string& name, AstNode* attrsp, VFlagChildDType, AstNodeDType* dtp) : ASTGEN_SUPER_Typedef(fl) - , m_name{name} { + , m_name{name} + , m_isHideLocal{false} + , m_isHideProtected{false} { childDTypep(dtp); // Only for parser addAttrsp(attrsp); dtypep(nullptr); // V3Width will resolve @@ -1738,6 +1742,10 @@ public: void name(const string& flag) override { m_name = flag; } bool attrPublic() const { return m_attrPublic; } void attrPublic(bool flag) { m_attrPublic = flag; } + bool isHideLocal() const { return m_isHideLocal; } + void isHideLocal(bool flag) { m_isHideLocal = flag; } + bool isHideProtected() const { return m_isHideProtected; } + void isHideProtected(bool flag) { m_isHideProtected = flag; } void tag(const string& text) override { m_tag = text; } string tag() const override { return m_tag; } }; diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 0249e9ea2..ba631667c 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -2031,9 +2031,10 @@ void AstRefDType::dump(std::ostream& str) const { if (!s_recursing) { // Prevent infinite dump if circular typedefs s_recursing = true; str << " -> "; - if (const auto subp = typedefp()) { + if (const auto subp = subDTypep()) { + if (typedefp()) str << "typedef=" << static_cast(typedefp())<< " -> "; subp->dump(str); - } else if (const auto subp = subDTypep()) { + } else if (const auto subp = typedefp()) { subp->dump(str); } s_recursing = false; diff --git a/src/V3ParseImp.h b/src/V3ParseImp.h index 738323ffd..d54b714c9 100644 --- a/src/V3ParseImp.h +++ b/src/V3ParseImp.h @@ -74,11 +74,19 @@ struct VMemberQualifiers final { if (m_static) nodep->isStatic(true); if (m_virtual) nodep->isVirtual(true); if (m_const || m_rand || m_randc) { - nodep->v3error("Syntax error: 'const'/'rand'/'randc' not allowed before " - "function/task declaration"); + nodep->v3error("Syntax error: 'const'/'rand'/'randc' not allowed " + "before function/task declaration"); } } } + void applyToNodes(AstTypedef* nodep) const { + if (m_local) nodep->isHideLocal(true); + if (m_protected) nodep->isHideProtected(true); + if (m_static || m_virtual || m_rand || m_randc) { + nodep->v3error("Syntax error: 'static'/'virtual'/'rand'/'randc' not allowed " + "before typedef declaration"); + } + } void applyToNodes(AstVar* nodesp) const { for (AstVar* nodep = nodesp; nodep; nodep = VN_AS(nodep->nextp(), Var)) { if (m_rand) nodep->rand(VRandAttr::RAND); diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 35177ce13..debb24435 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -1853,13 +1853,11 @@ class WidthVisitor final : public VNVisitor { if (AstNodeDType* typeofDtp = VN_CAST(nodep->typeofp(), NodeDType)) { // It's directly a type, e.g. "type(int)" typeofDtp = iterateEditMoveDTypep(nodep, typeofDtp); // Changes typeofp - nodep->typedefp(nullptr); nodep->refDTypep(typeofDtp); } else { // Type comes from expression's type, e.g. "type(variable)" userIterateAndNext(nodep->typeofp(), WidthVP{SELF, BOTH}.p()); AstNode* const typeofp = nodep->typeofp(); - nodep->typedefp(nullptr); nodep->refDTypep(typeofp->dtypep()); VL_DO_DANGLING(typeofp->unlinkFrBack()->deleteTree(), typeofp); } @@ -1873,7 +1871,6 @@ class WidthVisitor final : public VNVisitor { // this node's childDTypep userIterate(nodep->subDTypep(), nullptr); nodep->refDTypep(iterateEditMoveDTypep(nodep, nodep->subDTypep())); - nodep->typedefp(nullptr); // Note until line above subDTypep() may have followed this // Widths are resolved, but special iterate to check for recursion userIterate(nodep->subDTypep(), nullptr); } @@ -1883,6 +1880,7 @@ class WidthVisitor final : public VNVisitor { nodep->dtypeFrom(nodep->subDTypep()); nodep->widthFromSub(nodep->subDTypep()); UINFO(4, "dtWidthed " << nodep << endl); + // No nodep->typedefp(nullptr) for now; V3WidthCommit needs to check accesses nodep->doingWidth(false); } void visit(AstTypedef* nodep) override { diff --git a/src/V3WidthCommit.cpp b/src/V3WidthCommit.cpp index bb5bad534..1c9e1640f 100644 --- a/src/V3WidthCommit.cpp +++ b/src/V3WidthCommit.cpp @@ -85,12 +85,15 @@ private: "Only rand_mode() and constraint_mode() can have no def"); return; } - if (const auto varp = VN_CAST(defp, Var)) { - local = varp->isHideLocal(); - prot = varp->isHideProtected(); - } else if (const auto ftaskp = VN_CAST(defp, NodeFTask)) { - local = ftaskp->isHideLocal(); - prot = ftaskp->isHideProtected(); + if (const auto anodep = VN_CAST(defp, Var)) { + local = anodep->isHideLocal(); + prot = anodep->isHideProtected(); + } else if (const auto anodep = VN_CAST(defp, NodeFTask)) { + local = anodep->isHideLocal(); + prot = anodep->isHideProtected(); + } else if (const auto anodep = VN_CAST(defp, Typedef)) { + local = anodep->isHideLocal(); + prot = anodep->isHideProtected(); } else { nodep->v3fatalSrc("ref to unhandled definition type " << defp->prettyTypeName()); } @@ -178,6 +181,12 @@ private: nodep->unlinkFrBack(); // Make non-child v3Global.rootp()->typeTablep()->addTypesp(nodep); } + void visit(AstRefDType* nodep) override { + visitIterateNodeDType(nodep); + if (!nodep->typedefp()) return; // Already checked and cleared + classEncapCheck(nodep, nodep->typedefp(), VN_CAST(nodep->classOrPackagep(), Class)); + nodep->typedefp(nullptr); // No longer needed + } void visitIterateNodeDType(AstNodeDType* nodep) { // Rather than use dtypeChg which may make new nodes, we edit in place, // as we don't need to preserve any widthMin's, and every dtype with the same width diff --git a/src/verilog.y b/src/verilog.y index 05304ad09..2871fad8c 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -186,14 +186,14 @@ public: } AstNode* createTypedef(FileLine* fl, const string& name, AstNode* attrsp, AstNodeDType* basep, AstNodeRange* rangep) { - AstNode* const nodep = new AstTypedef{fl, name, attrsp, VFlagChildDType{}, + AstTypedef* const nodep = new AstTypedef{fl, name, attrsp, VFlagChildDType{}, GRAMMARP->createArray(basep, rangep, false)}; SYMP->reinsert(nodep); PARSEP->tagNodep(nodep); return nodep; } AstNode* createTypedefFwd(FileLine* fl, const string& name) { - AstNode* const nodep = new AstTypedefFwd{fl, name}; + AstTypedefFwd* const nodep = new AstTypedefFwd{fl, name}; SYMP->reinsert(nodep); PARSEP->tagNodep(nodep); return nodep; @@ -2479,11 +2479,13 @@ data_declaration: // ==IEEE: data_declaration ; class_property: // ==IEEE: class_property, which is {property_qualifier} data_declaration - memberQualListE data_declarationVarClass { $$ = $2; $1.applyToNodes($2); } + memberQualListE data_declarationVarClass + { $$ = $2; $1.applyToNodes($2); } + | memberQualListE type_declaration + { $$ = $2; if (VN_IS($2, Typedef)) $1.applyToNodes(VN_AS($2, Typedef)); } // // UNSUP: Import needs to apply local/protected from memberQualList, and error on others - | memberQualListE type_declaration { $$ = $2; } - // // UNSUP: Import needs to apply local/protected from memberQualList, and error on others - | memberQualListE package_import_declaration { $$ = $2; } + | memberQualListE package_import_declaration + { $$ = $2; } // // IEEE: virtual_interface_declaration // // "yVIRTUAL yID yID" looks just like a data_declaration // // Therefore the virtual_interface_declaration term isn't used diff --git a/test_regress/t/t_class_local_typedef_bad.out b/test_regress/t/t_class_local_typedef_bad.out new file mode 100644 index 000000000..26653bffe --- /dev/null +++ b/test_regress/t/t_class_local_typedef_bad.out @@ -0,0 +1,14 @@ +%Error-ENCAPSULATED: t/t_class_local_typedef_bad.v:13:8: 't1' is hidden as 'local' within this context (IEEE 1800-2023 8.18) + 13 | Cls::t1 var1; + | ^~ + t/t_class_local_typedef_bad.v:13:8: ... Location of definition + 9 | local typedef bit t1; + | ^~ + ... For error description see https://verilator.org/warn/ENCAPSULATED?v=latest +%Error-ENCAPSULATED: t/t_class_local_typedef_bad.v:14:8: 't2' is hidden as 'protected' within this context (IEEE 1800-2023 8.18) + 14 | Cls::t2 var2; + | ^~ + t/t_class_local_typedef_bad.v:14:8: ... Location of definition + 10 | protected typedef bit t2; + | ^~ +%Error: Exiting due to diff --git a/test_regress/t/t_class_local_typedef_bad.py b/test_regress/t/t_class_local_typedef_bad.py new file mode 100755 index 000000000..31228c9a7 --- /dev/null +++ b/test_regress/t/t_class_local_typedef_bad.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_class_local_typedef_bad.v b/test_regress/t/t_class_local_typedef_bad.v new file mode 100755 index 000000000..1688e7266 --- /dev/null +++ b/test_regress/t/t_class_local_typedef_bad.v @@ -0,0 +1,16 @@ +// 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; + class Cls; + local typedef bit t1; + protected typedef bit t2; + endclass + + Cls::t1 var1; // BAD: access error expected + Cls::t2 var2; // BAD: access error expected + +endmodule diff --git a/test_regress/t/t_class_unsup_bad.out b/test_regress/t/t_class_unsup_bad.out index a3f592e5c..6f5f72792 100644 --- a/test_regress/t/t_class_unsup_bad.out +++ b/test_regress/t/t_class_unsup_bad.out @@ -1,4 +1,10 @@ -%Error: t/t_class_unsup_bad.v:28:24: Syntax error: 'const'/'rand'/'randc' not allowed before function/task declaration - 28 | const function void func_const; endfunction +%Error: t/t_class_unsup_bad.v:24:21: Syntax error: 'static'/'virtual'/'rand'/'randc' not allowed before typedef declaration + 24 | rand typedef int irand_t; + | ^~~~~~~ +%Error: t/t_class_unsup_bad.v:25:22: Syntax error: 'static'/'virtual'/'rand'/'randc' not allowed before typedef declaration + 25 | randc typedef int icrand_t; + | ^~~~~~~~ +%Error: t/t_class_unsup_bad.v:31:24: Syntax error: 'const'/'rand'/'randc' not allowed before function/task declaration + 31 | const function void func_const; endfunction | ^~~~~~~~~~ %Error: Exiting due to diff --git a/test_regress/t/t_class_unsup_bad.v b/test_regress/t/t_class_unsup_bad.v index dd5fd639b..ade12a18e 100644 --- a/test_regress/t/t_class_unsup_bad.v +++ b/test_regress/t/t_class_unsup_bad.v @@ -21,6 +21,9 @@ class C #(parameter P=1); rand int irand; randc int icrand; + rand typedef int irand_t; + randc typedef int icrand_t; + task classtask; endtask function int classfunc; endfunction virtual function void func_virtual; endfunction