Fix access to non-overridden base class variable (#2654).

Changes

@@ -13,6 +13,8 @@ The contributors that suggested a given feature are shown in []. Thanks!
 
 ****  Fix unpacked array parameters near functions (#2639). [Anderson Ignacio da Silva]
 
+****  Fix access to non-overridden base class variable (#2654). [Tobias Rosenkranz]
+
 
 * Verilator 4.104 2020-11-14
 

src/V3Ast.h

@@ -2300,7 +2300,8 @@ private:
     AstVarScope* m_varScopep = nullptr;  // Varscope for hierarchy
     AstNodeModule* m_classOrPackagep = nullptr;  // Package hierarchy
     string m_name;  // Name of variable
-    string m_hiername;  // Scope converted into name-> for emitting
+    string m_hiernameToProt;  // Scope converted into name-> for emitting
+    string m_hiernameToUnprot;  // Scope converted into name-> for emitting
     bool m_hierThis = false;  // Hiername points to "this" function
 
 public:
@@ -2331,9 +2332,11 @@ public:
     void varp(AstVar* varp);
     AstVarScope* varScopep() const { return m_varScopep; }
     void varScopep(AstVarScope* varscp) { m_varScopep = varscp; }
-    string hiername() const { return m_hiername; }
+    string hiernameToProt() const { return m_hiernameToProt; }
+    void hiernameToProt(const string& hn) { m_hiernameToProt = hn; }
+    string hiernameToUnprot() const { return m_hiernameToUnprot; }
+    void hiernameToUnprot(const string& hn) { m_hiernameToUnprot = hn; }
     string hiernameProtect() const;
-    void hiername(const string& hn) { m_hiername = hn; }
     bool hierThis() const { return m_hierThis; }
     void hierThis(bool flag) { m_hierThis = flag; }
     AstNodeModule* classOrPackagep() const { return m_classOrPackagep; }
@@ -2620,7 +2623,8 @@ class AstNodeCCall VL_NOT_FINAL : public AstNodeStmt {
     // A call of a C++ function, perhaps a AstCFunc or perhaps globally named
     // Functions are not statements, while tasks are. AstNodeStmt needs isStatement() to deal.
     AstCFunc* m_funcp;
-    string m_hiername;
+    string m_hiernameToProt;
+    string m_hiernameToUnprot;
     string m_argTypes;
 
 public:
@@ -2634,7 +2638,8 @@ public:
     AstNodeCCall(AstType t, AstNodeCCall* oldp, AstCFunc* funcp)
         : AstNodeStmt{t, oldp->fileline(), true}
         , m_funcp{funcp} {
-        m_hiername = oldp->hiername();
+        m_hiernameToProt = oldp->hiernameToProt();
+        m_hiernameToUnprot = oldp->hiernameToUnprot();
         m_argTypes = oldp->argTypes();
         if (oldp->argsp()) addNOp2p(oldp->argsp()->unlinkFrBackWithNext());
     }
@@ -2654,8 +2659,10 @@ public:
     virtual bool isPure() const override;
     virtual bool isOutputter() const override { return !isPure(); }
     AstCFunc* funcp() const { return m_funcp; }
-    string hiername() const { return m_hiername; }
+    string hiernameToProt() const { return m_hiernameToProt; }
-    void hiername(const string& hn) { m_hiername = hn; }
+    void hiernameToProt(const string& hn) { m_hiernameToProt = hn; }
+    string hiernameToUnprot() const { return m_hiernameToUnprot; }
+    void hiernameToUnprot(const string& hn) { m_hiernameToUnprot = hn; }
     string hiernameProtect() const;
     void argTypes(const string& str) { m_argTypes = str; }
     string argTypes() const { return m_argTypes; }

src/V3AstNodes.cpp

@@ -54,7 +54,7 @@ void AstNodeVarRef::cloneRelink() {
 }
 
 string AstNodeVarRef::hiernameProtect() const {
-    return VIdProtect::protectWordsIf(hiername(), protect());
+    return hiernameToUnprot() + VIdProtect::protectWordsIf(hiernameToProt(), protect());
 }
 
 int AstNodeSel::bitConst() const {
@@ -107,7 +107,7 @@ const char* AstNodeCCall::broken() const {
 }
 bool AstNodeCCall::isPure() const { return funcp()->pure(); }
 string AstNodeCCall::hiernameProtect() const {
-    return VIdProtect::protectWordsIf(hiername(), protect());
+    return hiernameToUnprot() + VIdProtect::protectWordsIf(hiernameToProt(), protect());
 }
 
 void AstNodeCond::numberOperate(V3Number& out, const V3Number& lhs, const V3Number& rhs,

src/V3AstNodes.h

@@ -2347,7 +2347,7 @@ public:
     ASTNODE_NODE_FUNCS(VarRef)
     virtual void dump(std::ostream& str) const override;
     virtual V3Hash sameHash() const override {
-        return V3Hash(V3Hash(varp()->name()), V3Hash(hiername()));
+        return V3Hash(V3Hash(varp()->name()), V3Hash(hiernameToProt()));
     }
     virtual bool same(const AstNode* samep) const override {
         return same(static_cast<const AstVarRef*>(samep));
@@ -2356,16 +2356,18 @@ public:
         if (varScopep()) {
             return (varScopep() == samep->varScopep() && access() == samep->access());
         } else {
-            return (hiername() == samep->hiername() && varp()->name() == samep->varp()->name()
+            return (hiernameToProt() == samep->hiernameToProt()
-                    && access() == samep->access());
+                    && hiernameToUnprot() == samep->hiernameToUnprot()
+                    && varp()->name() == samep->varp()->name() && access() == samep->access());
         }
     }
     inline bool sameNoLvalue(AstVarRef* samep) const {
         if (varScopep()) {
             return (varScopep() == samep->varScopep());
         } else {
-            return (hiername() == samep->hiername()
+            return (hiernameToProt() == samep->hiernameToProt()
-                    && (hiername() != "" || samep->hiername() != "")
+                    && hiernameToUnprot() == samep->hiernameToUnprot()
+                    && (!hiernameToProt().empty() || !samep->hiernameToProt().empty())
                     && varp()->name() == samep->varp()->name());
         }
     }
@@ -2406,7 +2408,8 @@ public:
     virtual V3Hash sameHash() const override { return V3Hash(V3Hash(varp()), V3Hash(dotted())); }
     virtual bool same(const AstNode* samep) const override {
         const AstVarXRef* asamep = static_cast<const AstVarXRef*>(samep);
-        return (hiername() == asamep->hiername() && varp() == asamep->varp()
+        return (hiernameToProt() == asamep->hiernameToProt()
+                && hiernameToUnprot() == asamep->hiernameToUnprot() && varp() == asamep->varp()
                 && name() == asamep->name() && dotted() == asamep->dotted());
     }
 };

src/V3Dead.cpp

@@ -122,6 +122,7 @@ private:
 
     // VISITORS
     virtual void visit(AstNodeModule* nodep) override {
+        if (m_modp) m_modp->user1Inc();  // e.g. Class under Package
         VL_RESTORER(m_modp);
         {
             m_modp = nodep;

src/V3Descope.cpp

@@ -76,7 +76,8 @@ private:
     // Sets 'hierThisr' true if the object is local to this scope
     // (and could be made into a function-local later in V3Localize),
     // false if the object is in another scope.
-    string descopedName(const AstScope* scopep, bool& hierThisr, const AstVar* varp = nullptr) {
+    string descopedName(bool& hierThisr, string& hierUnprot, const AstScope* scopep,
+                        const AstVar* varp) {
         UASSERT(scopep, "Var/Func not scoped");
         hierThisr = (scopep == m_scopep);
 
@@ -118,6 +119,9 @@ private:
         } else if (relativeRefOk && scopep == m_scopep) {
             m_needThis = true;
             return "this->";
+        } else if (VN_IS(scopep->modp(), Class)) {
+            hierUnprot = v3Global.opt.modPrefix() + "_";  // Prefix before protected part
+            return scopep->modp()->name() + "::";
         } else if (relativeRefOk && scopep->aboveScopep() && scopep->aboveScopep() == m_scopep) {
             // Reference to scope of cell directly under this module, can just "cell->"
             string name = scopep->name();
@@ -249,12 +253,17 @@ private:
     virtual void visit(AstNodeVarRef* nodep) override {
         iterateChildren(nodep);
         // Convert the hierch name
+        UINFO(9, "  ref-in " << nodep << endl);
         UASSERT_OBJ(m_scopep, nodep, "Node not under scope");
         bool hierThis;
-        nodep->hiername(descopedName(nodep->varScopep()->scopep(), hierThis /*ref*/,
+        string hierUnprot;
-                                     nodep->varScopep()->varp()));
+        nodep->hiernameToProt(descopedName(hierThis /*ref*/, hierUnprot /*ref*/,
+                                           nodep->varScopep()->scopep(),
+                                           nodep->varScopep()->varp()));
+        nodep->hiernameToUnprot(hierUnprot);
         nodep->hierThis(hierThis);
         nodep->varScopep(nullptr);
+        UINFO(9, "  refout " << nodep << endl);
     }
     virtual void visit(AstNodeCCall* nodep) override {
         // UINFO(9, "       " << nodep << endl);
@@ -263,7 +272,10 @@ private:
         UASSERT_OBJ(m_scopep, nodep, "Node not under scope");
         UASSERT_OBJ(nodep->funcp()->scopep(), nodep, "CFunc not under scope");
         bool hierThis;
-        nodep->hiername(descopedName(nodep->funcp()->scopep(), hierThis /*ref*/));
+        string hierUnprot;
+        nodep->hiernameToProt(
+            descopedName(hierThis /*ref*/, hierUnprot /*ref*/, nodep->funcp()->scopep(), nullptr));
+        nodep->hiernameToUnprot(hierUnprot);
         // Can't do this, as we may have more calls later
         // nodep->funcp()->scopep(nullptr);
     }

src/V3EmitV.cpp

@@ -613,7 +613,8 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor {
         if (nodep->varScopep()) {
             putfs(nodep, nodep->varScopep()->prettyName());
         } else {
-            putfs(nodep, nodep->hiername());
+            putfs(nodep, nodep->hiernameToUnprot());
+            puts(nodep->hiernameToProt());
             puts(nodep->varp()->prettyName());
         }
     }

src/V3LinkDot.cpp

@@ -839,6 +839,7 @@ class LinkDotFindVisitor final : public AstNVisitor {
         UASSERT_OBJ(m_curSymp, nodep, "Class not under module/package/$unit");
         UINFO(8, "   " << nodep << endl);
         VL_RESTORER(m_scope);
+        VL_RESTORER(m_classOrPackagep);
         VL_RESTORER(m_modSymp);
         VL_RESTORER(m_curSymp);
         VL_RESTORER(m_paramNum);
@@ -849,6 +850,7 @@ class LinkDotFindVisitor final : public AstNVisitor {
             UINFO(4, "     Link Class: " << nodep << endl);
             VSymEnt* upperSymp = m_curSymp;
             m_scope = m_scope + "." + nodep->name();
+            m_classOrPackagep = nodep;
             m_curSymp = m_modSymp
                 = m_statep->insertBlock(upperSymp, nodep->name(), nodep, m_classOrPackagep);
             m_statep->insertMap(m_curSymp, m_scope);

src/V3Localize.cpp

@@ -76,7 +76,8 @@ private:
         // cppcheck-suppress unreadVariable  // cppcheck 1.90 bug
         VarFlags flags(nodep->varp());
         if (flags.m_done) {
-            nodep->hiername("");  // Remove this->
+            nodep->hiernameToProt("");  // Remove this->
+            nodep->hiernameToUnprot("");  // Remove this->
             nodep->hierThis(true);
         }
     }

src/V3Scope.cpp

@@ -68,7 +68,7 @@ private:
         for (const auto& itr : m_varRefScopes) {
             AstVarRef* nodep = itr.first;
             AstScope* scopep = itr.second;
-            if (nodep->classOrPackagep() && !nodep->varp()->isClassMember()) {
+            if (nodep->classOrPackagep()) {
                 const auto it2 = m_packageScopes.find(nodep->classOrPackagep());
                 UASSERT_OBJ(it2 != m_packageScopes.end(), nodep, "Can't locate package scope");
                 scopep = it2->second;
@@ -109,9 +109,7 @@ private:
             (m_aboveCellp ? static_cast<AstNode*>(m_aboveCellp) : static_cast<AstNode*>(nodep))
                 ->fileline(),
             nodep, scopename, m_aboveScopep, m_aboveCellp);
-        if (VN_IS(nodep, Package)) {
+        if (VN_IS(nodep, Package)) m_packageScopes.insert(make_pair(nodep, m_scopep));
-            m_packageScopes.insert(make_pair(VN_CAST(nodep, Package), m_scopep));
-        }
 
         // Now for each child cell, iterate the module this cell points to
         for (AstNode* cellnextp = nodep->stmtsp(); cellnextp; cellnextp = cellnextp->nextp()) {
@@ -152,8 +150,10 @@ private:
         VL_RESTORER(m_scopep);
         VL_RESTORER(m_aboveCellp);
         VL_RESTORER(m_aboveScopep);
+        VL_RESTORER(m_modp);
         {
             m_aboveScopep = m_scopep;
+            m_modp = nodep;
 
             string scopename;
             if (!m_aboveScopep) {
@@ -169,6 +169,8 @@ private:
                                             : static_cast<AstNode*>(nodep));
             m_scopep
                 = new AstScope(abovep->fileline(), m_modp, scopename, m_aboveScopep, m_aboveCellp);
+            m_packageScopes.insert(make_pair(nodep, m_scopep));
+
             // Create scope for the current usage of this cell
             AstNode::user1ClearTree();
             nodep->addMembersp(m_scopep);
@@ -283,7 +285,8 @@ private:
         } else {
             // We may have not made the variable yet, and we can't make it now as
             // the var's referenced package etc might not be created yet.
-            // So push to a list and post-correct
+            // So push to a list and post-correct.
+            // No check here for nodep->classOrPackagep(), will check when walk list.
             m_varRefScopes.insert(make_pair(nodep, m_scopep));
         }
     }

test_regress/t/t_class_extends_this3.pl

@@ -0,0 +1,21 @@
+#!/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(simulator => 1);
+
+compile(
+    );
+
+execute(
+    check_finished => 1,
+    );
+
+ok(1);
+1;

test_regress/t/t_class_extends_this3.v

@@ -0,0 +1,31 @@
+// 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
+
+typedef class Cls;
+
+class Base;
+   int value = 1;
+   function void testBase;
+      if (value != 1) $stop;
+   endfunction
+endclass
+
+class Cls extends Base;
+   function void testDerived;
+      if (value != 1) $stop;
+   endfunction
+endclass
+
+module t (/*AUTOARG*/);
+   initial begin
+      Cls c;
+      c = new;
+      c.testBase();
+      c.testDerived();
+      $write("*-* All Finished *-*\n");
+      $finish;
+   end
+endmodule

test_regress/t/t_class_local.v

@@ -4,27 +4,72 @@
 // any use, without warranty, 2020 by Wilson Snyder.
 // SPDX-License-Identifier: CC0-1.0
 
-module t (/*AUTOARG*/);
-
 class Cls;
    typedef enum {A = 10, B = 20, C = 30} en_t;
 
+   int m_pub = 1;
    local int m_loc = 2;
    protected int m_prot = B;
+   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;
+      if (m_pub != 1) $stop;
+      if (m_loc != 10) $stop;
+      if (m_prot != 20) $stop;
+      f_pub();  // Ok
+      f_loc();  // Ok
+      f_prot();  // Ok
+      s_pub();  // Ok
+      s_loc();  // Ok
+      s_prot();  // Ok
+      Cls::s_pub();  // Ok
+      Cls::s_loc();  // Ok
+      Cls::s_prot();  // Ok
+   endtask
 endclass
 
+class Ext extends Cls;
+   task check;
+      if (m_pub != 1) $stop;
+      if (m_prot != 20) $stop;
+      f_pub();  // Ok
+      f_prot();  // Ok
+      s_pub();  // Ok
+      s_prot();  // Ok
+      Cls::s_pub();  // Ok
+      Cls::s_prot();  // Ok
+   endtask
+endclass
+
+module t (/*AUTOARG*/);
    const Cls mod_c = new;
 
    initial begin
       Cls c;
+      Ext e;
       if (c.A != 10) $stop;
       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;
+      //
+      c.check();
+      e.check();
+      //
+      Cls::s_pub();  // Ok
+      c.s_pub();  // Ok
+      e.s_pub();  // Ok
+      //
       $write("*-* All Finished *-*\n");
       $finish;
    end