use weak_ptr for symbol_scope_rep and symbol_record_rep pointers (bug #53092) * symrec.h, symrec.cc (symbol_record_rep::m_fwd_scope): Manage with std::weak_ptr. Change all uses. Wrap symbol_record_rep and symbol_scope_rep pointers in std::shared_ptr<>. * symscope.h, symscope.cc (symbol_scope_rep::m_parent): Manage with std::weak_ptr. Change all uses. Wrap symbol_record_rep and symbol_scope_rep pointers in std::shared_ptr<>. (symbol_scope_rep::m_parent_fcn): Delete. (symbol_scope_rep::set_parent): Simplify. (symbol_scope_rep): Derive from std::enable_shared_from_this. (symbol_scope_rep::look_nonlocal): Use shared_from_this instead of raw pointer to this. (symbol_scope_rep::get_rep_ptr, symbol_scope_rep::parent_scope_rep, symbol_scope_rep::parent_fcn, symbol_scope::parent_fcn): Delete. Delete <, <=, >=, and > operators. * call-stack.cc (call_stack::goto_frame, call_stack::backtrace): Don't display or return pointer to symbol_scope_rep. diff --git a/libinterp/corefcn/call-stack.cc b/libinterp/corefcn/call-stack.cc --- a/libinterp/corefcn/call-stack.cc +++ b/libinterp/corefcn/call-stack.cc @@ -394,7 +394,6 @@ namespace octave << " at line " << elt.m_line << " column " << elt.m_column << " [" << elt.fcn_file_name () << "] " - << " (scope = " << elt.m_scope.get_rep_ptr () << "[context = " << elt.m_context << "])" << std::endl; } @@ -578,14 +577,12 @@ namespace octave Cell& name = retval.contents (1); Cell& line = retval.contents (2); Cell& column = retval.contents (3); - Cell& scope = retval.contents (4); Cell& context = retval.contents (5); octave_idx_type k = 0; for (const auto& frm : frames) { - scope(k) = frm.m_scope.get_rep_ptr (); context(k) = frm.m_context; file(k) = frm.fcn_file_name (); name(k) = frm.fcn_name (print_subfn); diff --git a/libinterp/corefcn/symrec.cc b/libinterp/corefcn/symrec.cc --- a/libinterp/corefcn/symrec.cc +++ b/libinterp/corefcn/symrec.cc @@ -49,7 +49,8 @@ namespace octave // only in symbol_record::symbol_record_rep::varref and // symbol_record::symbol_record_rep::varval. - return m_fwd_scope ? m_fwd_scope->current_context () : 0; + auto t_fwd_scope = m_fwd_scope.lock (); + return t_fwd_scope ? t_fwd_scope->current_context () : 0; } void @@ -64,8 +65,8 @@ namespace octave mark_persistent (); } - symbol_record::symbol_record_rep * - symbol_record::symbol_record_rep::dup (symbol_scope_rep *new_scope) const + std::shared_ptr<symbol_record::symbol_record_rep> + symbol_record::symbol_record_rep::dup (const std::shared_ptr<symbol_scope_rep>& new_scope) const { // FIXME: is this the right thing do to? if (auto t_fwd_rep = m_fwd_rep.lock ()) @@ -73,8 +74,9 @@ namespace octave static const context_id FIXME_CONTEXT = 0; - return new symbol_record_rep (m_name, varval (FIXME_CONTEXT), - m_storage_class); + return std::shared_ptr<symbol_record_rep> + (new symbol_record_rep (m_name, varval (FIXME_CONTEXT), + m_storage_class)); } octave_value diff --git a/libinterp/corefcn/symrec.h b/libinterp/corefcn/symrec.h --- a/libinterp/corefcn/symrec.h +++ b/libinterp/corefcn/symrec.h @@ -80,7 +80,7 @@ namespace octave symbol_record_rep (const std::string& nm, const octave_value& v, unsigned int sc) - : m_storage_class (sc), m_name (nm), m_fwd_scope (nullptr), + : m_storage_class (sc), m_name (nm), m_fwd_scope (), m_fwd_rep (), m_value_stack () { m_value_stack.push_back (v); @@ -478,7 +478,7 @@ namespace octave void init_persistent (void); - void bind_fwd_rep (symbol_scope_rep *fwd_scope, + void bind_fwd_rep (const std::shared_ptr<symbol_scope_rep>& fwd_scope, const std::shared_ptr<symbol_record_rep>& fwd_rep) { // If this object is already bound to another scope (for @@ -504,11 +504,12 @@ namespace octave // global in a script remain linked as globals in the // enclosing scope. - m_fwd_scope = nullptr; + m_fwd_scope = std::weak_ptr<symbol_scope_rep> (); m_fwd_rep.reset (); } - symbol_record_rep * dup (symbol_scope_rep *new_scope) const; + std::shared_ptr<symbol_record_rep> + dup (const std::shared_ptr<symbol_scope_rep>& new_scope) const; octave_value dump (context_id context) const; @@ -522,7 +523,7 @@ namespace octave std::string m_name; - symbol_scope_rep *m_fwd_scope; + std::weak_ptr<symbol_scope_rep> m_fwd_scope; std::weak_ptr<symbol_record_rep> m_fwd_rep; @@ -543,7 +544,7 @@ namespace octave ~symbol_record (void) = default; - symbol_record dup (symbol_scope_rep *sid) const + symbol_record dup (const std::shared_ptr<symbol_scope_rep>& sid) const { return symbol_record (m_rep->dup (sid)); } @@ -652,7 +653,8 @@ namespace octave unsigned int storage_class (void) const { return m_rep->storage_class (); } - void bind_fwd_rep (symbol_scope_rep *fwd_scope, const symbol_record& sr) + void bind_fwd_rep (const std::shared_ptr<symbol_scope_rep>& fwd_scope, + const symbol_record& sr) { m_rep->bind_fwd_rep (fwd_scope, sr.m_rep); } @@ -669,7 +671,9 @@ namespace octave std::shared_ptr<symbol_record_rep> m_rep; // NEW_REP must be dynamically allocated or nullptr. - symbol_record (symbol_record_rep *new_rep) : m_rep (new_rep) { } + symbol_record (const std::shared_ptr<symbol_record_rep>& new_rep) + : m_rep (new_rep) + { } octave_value find_function (const std::string& name, const octave_value_list& args) const; diff --git a/libinterp/corefcn/symscope.cc b/libinterp/corefcn/symscope.cc --- a/libinterp/corefcn/symscope.cc +++ b/libinterp/corefcn/symscope.cc @@ -93,7 +93,9 @@ namespace octave { symbol_record ret (name); - if (m_is_nested && m_parent && m_parent->look_nonlocal (name, ret)) + auto t_parent = m_parent.lock (); + + if (m_is_nested && t_parent && t_parent->look_nonlocal (name, ret)) return m_symbols[name] = ret; else { @@ -141,8 +143,10 @@ namespace octave if (p != m_subfunctions.end ()) return p->second; - if (m_parent) - return m_parent->find_subfunction (name); + auto t_parent = m_parent.lock (); + + if (t_parent) + return t_parent->find_subfunction (name); return octave_value (); } @@ -160,31 +164,17 @@ namespace octave } void - symbol_scope_rep::set_parent (symbol_scope_rep *p) + symbol_scope_rep::set_parent (const std::shared_ptr<symbol_scope_rep>& parent) { - m_parent = p; - - if (m_parent) - { - // If m_parent is the top-level scope, there will be no parent - // function. - - octave_function *current_fcn = function (); - - if (current_fcn && current_fcn->is_anonymous_function ()) - { - octave_function *parent_fcn = m_parent->function (); - - if (parent_fcn) - m_parent_fcn = octave_value (parent_fcn, true); - } - } + m_parent = std::weak_ptr<symbol_scope_rep> (parent); } void symbol_scope_rep::update_nest (void) { - if (m_parent) + auto t_parent = m_parent.lock (); + + if (t_parent) { // fix bad symbol_records for (auto& nm_sr : m_symbols) @@ -192,7 +182,7 @@ namespace octave symbol_record& ours = nm_sr.second; if (! ours.is_formal () - && m_is_nested && m_parent->look_nonlocal (nm_sr.first, ours)) + && m_is_nested && t_parent->look_nonlocal (nm_sr.first, ours)) { if (ours.is_global () || ours.is_persistent ()) error ("global and persistent may only be used in the topmost level in which a nested variable is used"); @@ -219,12 +209,14 @@ namespace octave table_iterator p = m_symbols.find (name); if (p == m_symbols.end ()) { - if (m_is_nested && m_parent) - return m_parent->look_nonlocal (name, result); + auto t_parent = m_parent.lock (); + + if (m_is_nested && t_parent) + return t_parent->look_nonlocal (name, result); } else if (! p->second.is_automatic ()) { - result.bind_fwd_rep (this, p->second); + result.bind_fwd_rep (shared_from_this (), p->second); return true; } @@ -232,7 +224,8 @@ namespace octave } void - symbol_scope_rep::bind_script_symbols (symbol_scope_rep *curr_scope) + symbol_scope_rep::bind_script_symbols + (const std::shared_ptr<symbol_scope_rep>& curr_scope) { for (auto& nm_sr : m_symbols) nm_sr.second.bind_fwd_rep (curr_scope, diff --git a/libinterp/corefcn/symscope.h b/libinterp/corefcn/symscope.h --- a/libinterp/corefcn/symscope.h +++ b/libinterp/corefcn/symscope.h @@ -50,6 +50,7 @@ namespace octave class symbol_scope; class symbol_scope_rep + : public std::enable_shared_from_this<symbol_scope_rep> { public: @@ -64,9 +65,8 @@ namespace octave subfunctions_iterator; symbol_scope_rep (const std::string& name = "") - : m_name (name), m_symbols (), m_subfunctions (), - m_fcn (nullptr), m_parent (nullptr), m_parent_fcn (), - m_children (), m_is_nested (false), + : m_name (name), m_symbols (), m_subfunctions (), m_fcn (nullptr), + m_parent (), m_children (), m_is_nested (false), m_is_static (false), m_context (0) { } @@ -91,19 +91,20 @@ namespace octave void mark_static (void) { m_is_static = true; } - symbol_scope_rep * parent_scope_rep (void) const { return m_parent; } - - octave_value parent_fcn (void) const { return m_parent_fcn; } + std::shared_ptr<symbol_scope_rep> parent_scope_rep (void) const + { + return m_parent.lock (); + } - symbol_scope_rep * dup (void) const + std::shared_ptr<symbol_scope_rep> dup (void) const { - symbol_scope_rep *new_sid = new symbol_scope_rep (m_name); + std::shared_ptr<symbol_scope_rep> new_sid + = std::shared_ptr<symbol_scope_rep> (new symbol_scope_rep (m_name)); for (const auto& nm_sr : m_symbols) new_sid->insert_symbol_record (nm_sr.second.dup (new_sid)); new_sid->m_parent = m_parent; - new_sid->m_parent_fcn = m_parent_fcn; return new_sid; } @@ -128,7 +129,8 @@ namespace octave return p->second; } - void inherit_internal (symbol_scope_rep& donor_scope_rep) + void inherit_internal + (const std::shared_ptr<symbol_scope_rep>& donor_scope_rep) { for (auto& nm_sr : m_symbols) { @@ -140,7 +142,7 @@ namespace octave if (nm != "__retval__") { - octave_value val = donor_scope_rep.varval (nm); + octave_value val = donor_scope_rep->varval (nm); if (val.is_defined ()) { @@ -153,14 +155,16 @@ namespace octave } } - void inherit (symbol_scope_rep *donor_scope_rep) + void inherit (const std::shared_ptr<symbol_scope_rep>& donor_scope_rep) { - while (donor_scope_rep) + std::shared_ptr<symbol_scope_rep> dsr = donor_scope_rep; + + while (dsr) { - inherit_internal (*donor_scope_rep); + inherit_internal (dsr); - if (donor_scope_rep->is_nested ()) - donor_scope_rep = parent_scope_rep (); + if (dsr->is_nested ()) + dsr = parent_scope_rep (); else break; } @@ -513,13 +517,13 @@ namespace octave void set_function (octave_user_function *fcn) { m_fcn = fcn; } - void set_parent (symbol_scope_rep *p); + void set_parent (const std::shared_ptr<symbol_scope_rep>& parent); void update_nest (void); bool look_nonlocal (const std::string& name, symbol_record& result); - void bind_script_symbols (symbol_scope_rep *curr_scope); + void bind_script_symbols (const std::shared_ptr<symbol_scope_rep>& curr_scope); void unbind_script_symbols (void); @@ -545,8 +549,7 @@ namespace octave octave_user_function *m_fcn; // Parent of nested function (may be null). - symbol_scope_rep *m_parent; - octave_value m_parent_fcn; + std::weak_ptr<symbol_scope_rep> m_parent; // Child nested functions. std::vector<symbol_scope> m_children; @@ -571,7 +574,9 @@ namespace octave // NEW_REP must be dynamically allocated or nullptr. If it is // nullptr, the scope is invalid. - symbol_scope (symbol_scope_rep *new_rep = nullptr) : m_rep (new_rep) { } + symbol_scope (const std::shared_ptr<symbol_scope_rep> new_rep = nullptr) + : m_rep (new_rep) + { } symbol_scope (const symbol_scope&) = default; @@ -611,15 +616,17 @@ namespace octave m_rep->mark_static (); } - symbol_scope_rep * parent_scope (void) const + std::shared_ptr<symbol_scope_rep> parent_scope (void) const { return m_rep ? m_rep->parent_scope_rep () : nullptr; } - octave_value parent_fcn (void) const + octave_value parent_fcn (void) const; +#if 0 { return m_rep ? m_rep->parent_fcn () : octave_value (); } +#endif symbol_scope dup (void) const { @@ -906,12 +913,6 @@ namespace octave m_rep->set_parent (p.get_rep ()); } - void set_parent (symbol_scope_rep *p) - { - if (m_rep) - m_rep->set_parent (p); - } - void update_nest (void) { if (m_rep) @@ -935,9 +936,9 @@ namespace octave m_rep->unbind_script_symbols (); } - symbol_scope_rep * get_rep (void) const + std::shared_ptr<symbol_scope_rep> get_rep (void) const { - return m_rep.get (); + return m_rep; } friend bool operator == (const symbol_scope& a, const symbol_scope& b) @@ -950,28 +951,6 @@ namespace octave return a.m_rep != b.m_rep; } - friend bool operator < (const symbol_scope& a, const symbol_scope& b) - { - return a.m_rep < b.m_rep; - } - - friend bool operator <= (const symbol_scope& a, const symbol_scope& b) - { - return a.m_rep <= b.m_rep; - } - - friend bool operator >= (const symbol_scope& a, const symbol_scope& b) - { - return a.m_rep >= b.m_rep; - } - - friend bool operator > (const symbol_scope& a, const symbol_scope& b) - { - return a.m_rep > b.m_rep; - } - - symbol_scope_rep * get_rep_ptr (void) const { return m_rep.get (); } - private: std::shared_ptr<symbol_scope_rep> m_rep; @@ -980,11 +959,6 @@ namespace octave { return m_rep ? m_rep->dump_symbols_map () : octave_value (); } - - symbol_scope_rep * parent_scope_rep (void) const - { - return m_rep ? m_rep->parent_scope_rep () : nullptr; - } }; }

--- a/libinterp/corefcn/symscope.h	Thu Feb 08 01:08:36 2018 -0500
+++ b/libinterp/corefcn/symscope.h	Thu Feb 08 04:06:03 2018 -0500
@@ -50,6 +50,7 @@
   class symbol_scope;
 
   class symbol_scope_rep
+    : public std::enable_shared_from_this<symbol_scope_rep>
   {
   public:
 
@@ -64,9 +65,8 @@
     subfunctions_iterator;
 
     symbol_scope_rep (const std::string& name = "")
-      : m_name (name), m_symbols (), m_subfunctions (),
-        m_fcn (nullptr), m_parent (nullptr), m_parent_fcn (),
-        m_children (), m_is_nested (false),
+      : m_name (name), m_symbols (), m_subfunctions (), m_fcn (nullptr),
+        m_parent (), m_children (), m_is_nested (false),
         m_is_static (false), m_context (0)
     { }
 
@@ -91,19 +91,20 @@
 
     void mark_static (void) { m_is_static = true; }
 
-    symbol_scope_rep * parent_scope_rep (void) const { return m_parent; }
-
-    octave_value parent_fcn (void) const { return m_parent_fcn; }
+    std::shared_ptr<symbol_scope_rep> parent_scope_rep (void) const
+    {
+      return m_parent.lock ();
+    }
 
-    symbol_scope_rep * dup (void) const
+    std::shared_ptr<symbol_scope_rep> dup (void) const
     {
-      symbol_scope_rep *new_sid = new symbol_scope_rep (m_name);
+      std::shared_ptr<symbol_scope_rep> new_sid
+        = std::shared_ptr<symbol_scope_rep> (new symbol_scope_rep (m_name));
 
       for (const auto& nm_sr : m_symbols)
         new_sid->insert_symbol_record (nm_sr.second.dup (new_sid));
 
       new_sid->m_parent = m_parent;
-      new_sid->m_parent_fcn = m_parent_fcn;
 
       return new_sid;
     }
@@ -128,7 +129,8 @@
         return p->second;
     }
 
-    void inherit_internal (symbol_scope_rep& donor_scope_rep)
+    void inherit_internal
+      (const std::shared_ptr<symbol_scope_rep>& donor_scope_rep)
     {
       for (auto& nm_sr : m_symbols)
         {
@@ -140,7 +142,7 @@
 
               if (nm != "__retval__")
                 {
-                  octave_value val = donor_scope_rep.varval (nm);
+                  octave_value val = donor_scope_rep->varval (nm);
 
                   if (val.is_defined ())
                     {
@@ -153,14 +155,16 @@
         }
     }
 
-    void inherit (symbol_scope_rep *donor_scope_rep)
+    void inherit (const std::shared_ptr<symbol_scope_rep>& donor_scope_rep)
     {
-      while (donor_scope_rep)
+      std::shared_ptr<symbol_scope_rep> dsr = donor_scope_rep;
+
+      while (dsr)
         {
-          inherit_internal (*donor_scope_rep);
+          inherit_internal (dsr);
 
-          if (donor_scope_rep->is_nested ())
-            donor_scope_rep = parent_scope_rep ();
+          if (dsr->is_nested ())
+            dsr = parent_scope_rep ();
           else
             break;
         }
@@ -513,13 +517,13 @@
 
     void set_function (octave_user_function *fcn) { m_fcn = fcn; }
 
-    void set_parent (symbol_scope_rep *p);
+    void set_parent (const std::shared_ptr<symbol_scope_rep>& parent);
 
     void update_nest (void);
 
     bool look_nonlocal (const std::string& name, symbol_record& result);
 
-    void bind_script_symbols (symbol_scope_rep *curr_scope);
+    void bind_script_symbols (const std::shared_ptr<symbol_scope_rep>& curr_scope);
 
     void unbind_script_symbols (void);
 
@@ -545,8 +549,7 @@
     octave_user_function *m_fcn;
 
     // Parent of nested function (may be null).
-    symbol_scope_rep *m_parent;
-    octave_value m_parent_fcn;
+    std::weak_ptr<symbol_scope_rep> m_parent;
 
     // Child nested functions.
     std::vector<symbol_scope> m_children;
@@ -571,7 +574,9 @@
 
     // NEW_REP must be dynamically allocated or nullptr.  If it is
     // nullptr, the scope is invalid.
-    symbol_scope (symbol_scope_rep *new_rep = nullptr) : m_rep (new_rep) { }
+    symbol_scope (const std::shared_ptr<symbol_scope_rep> new_rep = nullptr)
+      : m_rep (new_rep)
+    { }
 
     symbol_scope (const symbol_scope&) = default;
 
@@ -611,16 +616,11 @@
         m_rep->mark_static ();
     }
 
-    symbol_scope_rep * parent_scope (void) const
+    std::shared_ptr<symbol_scope_rep> parent_scope (void) const
     {
       return m_rep ? m_rep->parent_scope_rep () : nullptr;
     }
 
-    octave_value parent_fcn (void) const
-    {
-      return m_rep ? m_rep->parent_fcn () : octave_value ();
-    }
-
     symbol_scope dup (void) const
     {
       return symbol_scope (m_rep ? m_rep->dup () : nullptr);
@@ -906,12 +906,6 @@
         m_rep->set_parent (p.get_rep ());
     }
 
-    void set_parent (symbol_scope_rep *p)
-    {
-      if (m_rep)
-        m_rep->set_parent (p);
-    }
-
     void update_nest (void)
     {
       if (m_rep)
@@ -935,9 +929,9 @@
         m_rep->unbind_script_symbols ();
     }
 
-    symbol_scope_rep * get_rep (void) const
+    std::shared_ptr<symbol_scope_rep> get_rep (void) const
     {
-      return m_rep.get ();
+      return m_rep;
     }
 
     friend bool operator == (const symbol_scope& a, const symbol_scope& b)
@@ -950,28 +944,6 @@
       return a.m_rep != b.m_rep;
     }
 
-    friend bool operator < (const symbol_scope& a, const symbol_scope& b)
-    {
-      return a.m_rep < b.m_rep;
-    }
-
-    friend bool operator <= (const symbol_scope& a, const symbol_scope& b)
-    {
-      return a.m_rep <= b.m_rep;
-    }
-
-    friend bool operator >= (const symbol_scope& a, const symbol_scope& b)
-    {
-      return a.m_rep >= b.m_rep;
-    }
-
-    friend bool operator > (const symbol_scope& a, const symbol_scope& b)
-    {
-      return a.m_rep > b.m_rep;
-    }
-
-    symbol_scope_rep * get_rep_ptr (void) const { return m_rep.get (); }
-
   private:
 
     std::shared_ptr<symbol_scope_rep> m_rep;
@@ -980,11 +952,6 @@
     {
       return m_rep ? m_rep->dump_symbols_map () : octave_value ();
     }
-
-    symbol_scope_rep * parent_scope_rep (void) const
-    {
-      return m_rep ? m_rep->parent_scope_rep () : nullptr;
-    }
   };
 }