Check for proper 'local' and 'protected' (#2228).

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]

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; }

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; }

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",

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
+            if (m_local) nodep->isHideLocal(true);
-            // Ignored for now: m_protected
+            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);

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");

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;
       //

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

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;

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