diff --git a/Changes b/Changes index 155200731..b0f62fc5a 100644 --- a/Changes +++ b/Changes @@ -9,6 +9,8 @@ The contributors that suggested a given feature are shown in []. Thanks! *** Support 'with item.index'. +*** Check for proper 'local' and 'protected' (#2228). + **** Fix trace signal names getting hashed (#2643). [Barbara Gigerl] **** Fix unpacked array parameters near functions (#2639). [Anderson Ignacio da Silva] diff --git a/src/V3Ast.h b/src/V3Ast.h index 9910125d1..e8436767c 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -2688,6 +2688,8 @@ private: bool m_dpiOpenChild : 1; // DPI import open array child wrapper bool m_dpiTask : 1; // DPI import task (vs. void function) bool m_isConstructor : 1; // Class constructor + bool m_isHideLocal : 1; // Verilog local + bool m_isHideProtected : 1; // Verilog protected bool m_pure : 1; // DPI import pure (vs. virtual pure) bool m_pureVirtual : 1; // Pure virtual bool m_virtual : 1; // Virtual method in class @@ -2708,6 +2710,8 @@ public: , m_dpiOpenChild{false} , m_dpiTask{false} , m_isConstructor{false} + , m_isHideLocal{false} + , m_isHideProtected{false} , m_pure{false} , m_pureVirtual{false} , m_virtual{false} { @@ -2766,6 +2770,10 @@ public: bool dpiTask() const { return m_dpiTask; } void isConstructor(bool flag) { m_isConstructor = flag; } bool isConstructor() const { return m_isConstructor; } + 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 pure(bool flag) { m_pure = flag; } bool pure() const { return m_pure; } void pureVirtual(bool flag) { m_pureVirtual = flag; } diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 8ac7382f9..3dfca497b 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -1897,6 +1897,8 @@ private: bool m_isPullup : 1; // Tri1 bool m_isIfaceParent : 1; // dtype is reference to interface present in this module bool m_isDpiOpenArray : 1; // DPI import open array + bool m_isHideLocal : 1; // Verilog local + bool m_isHideProtected : 1; // Verilog protected bool m_noReset : 1; // Do not do automated reset/randomization bool m_noSubst : 1; // Do not substitute out references bool m_overridenParam : 1; // Overridden parameter by #(...) or defparam @@ -1934,6 +1936,8 @@ private: m_isPullup = false; m_isIfaceParent = false; m_isDpiOpenArray = false; + m_isHideLocal = false; + m_isHideProtected = false; m_noReset = false; m_noSubst = false; m_overridenParam = false; @@ -2083,6 +2087,10 @@ public: void funcReturn(bool flag) { m_funcReturn = flag; } void isDpiOpenArray(bool flag) { m_isDpiOpenArray = flag; } bool isDpiOpenArray() const { return m_isDpiOpenArray; } + 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 noReset(bool flag) { m_noReset = flag; } bool noReset() const { return m_noReset; } void noSubst(bool flag) { m_noSubst = flag; } diff --git a/src/V3Error.h b/src/V3Error.h index fcf166a75..5e88ba6c2 100644 --- a/src/V3Error.h +++ b/src/V3Error.h @@ -52,6 +52,7 @@ public: I_DEF_NETTYPE_WIRE, // `default_nettype is WIRE (false=NONE) // Error codes: E_DETECTARRAY, // Error: Unsupported: Can't detect changes on arrayed variable + E_ENCAPSULATED, // Error: local/protected violation E_PORTSHORT, // Error: Output port is connected to a constant, electrical short E_UNSUPPORTED, // Error: Unsupported (generally) E_TASKNSVAR, // Error: Task I/O not simple @@ -147,7 +148,7 @@ public: // Boolean " I_CELLDEFINE", " I_COVERAGE", " I_TRACING", " I_LINT", " I_DEF_NETTYPE_WIRE", // Errors - "DETECTARRAY", "PORTSHORT", "UNSUPPORTED", "TASKNSVAR", + "DETECTARRAY", "ENCAPSULATED", "PORTSHORT", "UNSUPPORTED", "TASKNSVAR", // Warnings " EC_FIRST_WARN", "ALWCOMBORDER", "ASSIGNDLY", "ASSIGNIN", diff --git a/src/V3ParseImp.h b/src/V3ParseImp.h index ee58a7eeb..eb5dc8a6f 100644 --- a/src/V3ParseImp.h +++ b/src/V3ParseImp.h @@ -70,8 +70,8 @@ struct VMemberQualifiers { } void applyToNodes(AstNodeFTask* nodesp) const { for (AstNodeFTask* nodep = nodesp; nodep; nodep = VN_CAST(nodep->nextp(), NodeFTask)) { - // Ignored for now: m_local - // Ignored for now: m_protected + if (m_local) nodep->isHideLocal(true); + if (m_protected) nodep->isHideProtected(true); if (m_virtual) nodep->isVirtual(true); if (m_automatic) nodep->lifetime(VLifetime::AUTOMATIC); if (m_static) nodep->lifetime(VLifetime::STATIC); @@ -83,10 +83,10 @@ struct VMemberQualifiers { } void applyToNodes(AstVar* nodesp) const { for (AstVar* nodep = nodesp; nodep; nodep = VN_CAST(nodep->nextp(), Var)) { - // Ignored for now: m_local - // Ignored for now: m_protected // Ignored for now: m_rand // Ignored for now: m_randc + if (m_local) nodep->isHideLocal(true); + if (m_protected) nodep->isHideProtected(true); if (m_automatic) nodep->lifetime(VLifetime::AUTOMATIC); if (m_static) nodep->lifetime(VLifetime::STATIC); if (m_const) nodep->isConst(true); diff --git a/src/V3WidthCommit.h b/src/V3WidthCommit.h index 2176bc8cd..3abe211ba 100644 --- a/src/V3WidthCommit.h +++ b/src/V3WidthCommit.h @@ -69,6 +69,9 @@ class WidthCommitVisitor final : public AstNVisitor { // AstVar::user1p -> bool, processed AstUser1InUse m_inuser1; + // STATE + AstNodeModule* m_modp = nullptr; + public: // METHODS static AstConst* newIfConstCommitSize(AstConst* nodep) { @@ -112,7 +115,56 @@ private: } return nodep; } + void classEncapCheck(AstNode* nodep, AstNode* defp, AstClass* defClassp) { + // Call on non-local class to check local/protected status and complain + bool local = false; + bool prot = false; + 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(); + } else { + nodep->v3fatalSrc("ref to unhandled definition type " << defp->prettyTypeName()); + } + if (local || prot) { + auto refClassp = VN_CAST(m_modp, Class); + const char* how = nullptr; + if (local && defClassp && refClassp != defClassp) { + how = "'local'"; + } else if (prot && defClassp && !classExtendedRecurse(refClassp, defClassp)) { + how = "'protected'"; + } + if (how) { + UINFO(9, "refclass " << refClassp << endl); + UINFO(9, "defclass " << defClassp << endl); + nodep->v3warn(E_ENCAPSULATED, nodep->prettyNameQ() + << " is hidden as " << how + << " within this context (IEEE 1800-2017 8.18)\n" + << nodep->warnContextPrimary() << endl + << nodep->warnOther() + << "... Location of definition" << endl + << defp->warnContextSecondary()); + } + } + } + static bool classExtendedRecurse(const AstClass* refClassp, const AstClass* defClassp) { + // Return true if refClassp is an extends class of defClassp + if (!refClassp || !defClassp) return false; + if (refClassp == defClassp) return true; + return classExtendedRecurse(refClassp->extendsp()->classp(), defClassp); + } + // VISITORS + virtual void visit(AstNodeModule* nodep) override { + VL_RESTORER(m_modp); + { + m_modp = nodep; + iterateChildren(nodep); + editDType(nodep); + } + } virtual void visit(AstConst* nodep) override { UASSERT_OBJ(nodep->dtypep(), nodep, "No dtype"); iterate(nodep->dtypep()); // Do datatype first @@ -153,6 +205,23 @@ private: nodep->virtRefDTypep(editOneDType(nodep->virtRefDTypep())); nodep->virtRefDType2p(editOneDType(nodep->virtRefDType2p())); } + virtual void visit(AstNodeVarRef* nodep) override { + iterateChildren(nodep); + editDType(nodep); + classEncapCheck(nodep, nodep->varp(), VN_CAST(nodep->classOrPackagep(), Class)); + } + virtual void visit(AstNodeFTaskRef* nodep) override { + iterateChildren(nodep); + editDType(nodep); + classEncapCheck(nodep, nodep->taskp(), VN_CAST(nodep->classOrPackagep(), Class)); + } + virtual void visit(AstMemberSel* nodep) override { + iterateChildren(nodep); + editDType(nodep); + if (auto* classrefp = VN_CAST(nodep->fromp()->dtypep(), ClassRefDType)) { + classEncapCheck(nodep, nodep->varp(), classrefp->classp()); + } // else might be struct, etc + } virtual void visit(AstNodePreSel* nodep) override { // LCOV_EXCL_LINE // This check could go anywhere after V3Param nodep->v3fatalSrc("Presels should have been removed before this point"); diff --git a/test_regress/t/t_class_local.v b/test_regress/t/t_class_local.v index cd20f4eb0..0a5b6cf78 100644 --- a/test_regress/t/t_class_local.v +++ b/test_regress/t/t_class_local.v @@ -19,7 +19,7 @@ class Cls; task check; Cls o; if (m_pub != 1) $stop; - if (m_loc != 10) $stop; + if (m_loc != 2) $stop; if (m_prot != 20) $stop; f_pub(); // Ok f_loc(); // Ok @@ -56,10 +56,6 @@ module t (/*AUTOARG*/); c = new; e = new; if (c.m_pub != 1) $stop; - if (c.m_loc != 2) $stop; - c.m_loc = 10; - if (c.m_loc != 10) $stop; - if (c.m_prot != 20) $stop; // if (mod_c.A != 10) $stop; // diff --git a/test_regress/t/t_class_local_bad.out b/test_regress/t/t_class_local_bad.out new file mode 100644 index 000000000..102a251d1 --- /dev/null +++ b/test_regress/t/t_class_local_bad.out @@ -0,0 +1,106 @@ +%Error-ENCAPSULATED: t/t_class_local_bad.v:71:20: 'm_loc' is hidden as 'local' within this context (IEEE 1800-2017 8.18) + : ... In instance t + 71 | bad(); if (c.m_loc != 2) $stop; + | ^~~~~ + t/t_class_local_bad.v:71:20: ... Location of definition + 15 | local int m_loc = 2; + | ^~~~~ +%Error-ENCAPSULATED: t/t_class_local_bad.v:72:20: 'm_prot' is hidden as 'protected' within this context (IEEE 1800-2017 8.18) + : ... In instance t + 72 | bad(); if (c.m_prot != 20) $stop; + | ^~~~~~ + t/t_class_local_bad.v:72:20: ... Location of definition + 16 | protected int m_prot = 3; + | ^~~~~~ +%Error-ENCAPSULATED: t/t_class_local_bad.v:74:20: 'm_loc' is hidden as 'local' within this context (IEEE 1800-2017 8.18) + : ... In instance t + 74 | bad(); if (e.m_loc != 2) $stop; + | ^~~~~ + t/t_class_local_bad.v:74:20: ... Location of definition + 15 | local int m_loc = 2; + | ^~~~~ +%Error-ENCAPSULATED: t/t_class_local_bad.v:75:20: 'm_prot' is hidden as 'protected' within this context (IEEE 1800-2017 8.18) + : ... In instance t + 75 | bad(); if (e.m_prot != 20) $stop; + | ^~~~~~ + t/t_class_local_bad.v:75:20: ... Location of definition + 16 | protected int m_prot = 3; + | ^~~~~~ +%Error-ENCAPSULATED: t/t_class_local_bad.v:77:16: 'f_loc' is hidden as 'local' within this context (IEEE 1800-2017 8.18) + : ... In instance t + 77 | bad(); c.f_loc(); + | ^~~~~ + t/t_class_local_bad.v:77:16: ... Location of definition + 18 | local task f_loc; endtask + | ^~~~~ +%Error-ENCAPSULATED: t/t_class_local_bad.v:78:16: 'f_prot' is hidden as 'protected' within this context (IEEE 1800-2017 8.18) + : ... In instance t + 78 | bad(); c.f_prot(); + | ^~~~~~ + t/t_class_local_bad.v:78:16: ... Location of definition + 19 | protected task f_prot; endtask + | ^~~~~~ +%Error-ENCAPSULATED: t/t_class_local_bad.v:80:16: 's_loc' is hidden as 'local' within this context (IEEE 1800-2017 8.18) + : ... In instance t + 80 | bad(); c.s_loc(); + | ^~~~~ + t/t_class_local_bad.v:80:16: ... Location of definition + 21 | static local task s_loc; endtask + | ^~~~~ +%Error-ENCAPSULATED: t/t_class_local_bad.v:81:16: 's_prot' is hidden as 'protected' within this context (IEEE 1800-2017 8.18) + : ... In instance t + 81 | bad(); c.s_prot(); + | ^~~~~~ + t/t_class_local_bad.v:81:16: ... Location of definition + 22 | static protected task s_prot; endtask + | ^~~~~~ +%Error-ENCAPSULATED: t/t_class_local_bad.v:83:19: 's_loc' is hidden as 'local' within this context (IEEE 1800-2017 8.18) + : ... In instance t + 83 | bad(); Cls::s_loc(); + | ^~~~~ + t/t_class_local_bad.v:83:19: ... Location of definition + 21 | static local task s_loc; endtask + | ^~~~~ +%Error-ENCAPSULATED: t/t_class_local_bad.v:84:19: 's_prot' is hidden as 'protected' within this context (IEEE 1800-2017 8.18) + : ... In instance t + 84 | bad(); Cls::s_prot(); + | ^~~~~~ + t/t_class_local_bad.v:84:19: ... Location of definition + 22 | static protected task s_prot; endtask + | ^~~~~~ +%Error-ENCAPSULATED: t/t_class_local_bad.v:47:18: 'm_loc' is hidden as 'local' within this context (IEEE 1800-2017 8.18) + : ... In instance t + 47 | bad(); if (m_loc != 10) $stop; + | ^~~~~ + t/t_class_local_bad.v:47:18: ... Location of definition + 15 | local int m_loc = 2; + | ^~~~~ +%Error-ENCAPSULATED: t/t_class_local_bad.v:50:14: 'f_loc' is hidden as 'local' within this context (IEEE 1800-2017 8.18) + : ... In instance t + 50 | bad(); f_loc(); + | ^~~~~ + t/t_class_local_bad.v:50:14: ... Location of definition + 18 | local task f_loc; endtask + | ^~~~~ +%Error-ENCAPSULATED: t/t_class_local_bad.v:53:16: 'f_loc' is hidden as 'local' within this context (IEEE 1800-2017 8.18) + : ... In instance t + 53 | bad(); o.f_loc(); + | ^~~~~ + t/t_class_local_bad.v:53:16: ... Location of definition + 18 | local task f_loc; endtask + | ^~~~~ +%Error-ENCAPSULATED: t/t_class_local_bad.v:56:14: 's_loc' is hidden as 'local' within this context (IEEE 1800-2017 8.18) + : ... In instance t + 56 | bad(); s_loc(); + | ^~~~~ + t/t_class_local_bad.v:56:14: ... Location of definition + 21 | static local task s_loc; endtask + | ^~~~~ +%Error-ENCAPSULATED: t/t_class_local_bad.v:59:19: 's_loc' is hidden as 'local' within this context (IEEE 1800-2017 8.18) + : ... In instance t + 59 | bad(); Cls::s_loc(); + | ^~~~~ + t/t_class_local_bad.v:59:19: ... Location of definition + 21 | static local task s_loc; endtask + | ^~~~~ +%Error: Exiting due to diff --git a/test_regress/t/t_class_local_bad.pl b/test_regress/t/t_class_local_bad.pl new file mode 100755 index 000000000..7be596e0f --- /dev/null +++ b/test_regress/t/t_class_local_bad.pl @@ -0,0 +1,19 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2020 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(linter => 1); + +lint( + fails => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_class_local_bad.v b/test_regress/t/t_class_local_bad.v new file mode 100644 index 000000000..636b89f43 --- /dev/null +++ b/test_regress/t/t_class_local_bad.v @@ -0,0 +1,89 @@ +// 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 + +// Let context messages easily know if given line is expected ok or bad +task ok; +endtask +task bad; +endtask + +class Cls; + int m_pub = 1; + local int m_loc = 2; + protected int m_prot = 3; + task f_pub; endtask + local task f_loc; endtask + protected task f_prot; endtask + static task s_pub; endtask + static local task s_loc; endtask + static protected task s_prot; endtask + task check; + Cls o; + ok(); if (m_pub != 1) $stop; + ok(); if (m_loc != 10) $stop; + ok(); if (m_prot != 20) $stop; + ok(); f_pub(); + ok(); f_loc(); + ok(); f_prot(); + ok(); o.f_pub(); + ok(); o.f_loc(); + ok(); o.f_prot(); + ok(); s_pub(); + ok(); s_loc(); + ok(); s_prot(); + ok(); Cls::s_pub(); + ok(); Cls::s_loc(); + ok(); Cls::s_prot(); + endtask +endclass + +class Ext extends Cls; + task check; + Ext o; + ok(); if (m_pub != 1) $stop; + bad(); if (m_loc != 10) $stop; + ok(); if (m_prot != 20) $stop; + ok(); f_pub(); + bad(); f_loc(); + ok(); f_prot(); + ok(); o.f_pub(); + bad(); o.f_loc(); + ok(); o.f_prot(); + ok(); s_pub(); + bad(); s_loc(); + ok(); s_prot(); + ok(); Cls::s_pub(); + bad(); Cls::s_loc(); + ok(); Cls::s_prot(); + endtask +endclass + +module t (/*AUTOARG*/); + initial begin + Cls c; + Ext e; + c = new; + e = new; + ok(); if (c.m_pub != 1) $stop; + bad(); if (c.m_loc != 2) $stop; + bad(); if (c.m_prot != 20) $stop; + ok(); if (e.m_pub != 1) $stop; + bad(); if (e.m_loc != 2) $stop; + bad(); if (e.m_prot != 20) $stop; + ok(); c.f_pub(); + bad(); c.f_loc(); + bad(); c.f_prot(); + ok(); c.s_pub(); + bad(); c.s_loc(); + bad(); c.s_prot(); + ok(); Cls::s_pub(); + bad(); Cls::s_loc(); + bad(); Cls::s_prot(); + // + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule