diff libinterp/corefcn/symrec.h @ 24977:2b040946dc69 stable

improve handling of clear for globals, forwarded symbols (bug #53027) * symrec.h (symbol_record::symbol_record_rep::clear): Don't return early after unbinding global. Don't check for forwarded symbol here. * symrec.h (symbol_record::is_forwarded, symbol_record::symbol_record_rep::is_forwarded): New functions. * symscope.h (symbol_scope_rep::refresh): Don't delete forwarded symbols. * test/bug-53027/bug-53027.tst, test/bug-53027/module.mk, test/bug-53027/ntest53027a.m, test/bug-53027/ntest53027b.m: New test files. * test/module.mk: Include test/bug-53027/module.mk.
author John W. Eaton <jwe@octave.org>
date Thu, 22 Mar 2018 17:34:41 -0400
parents 0cd310ac0d77
children c16ad80274c9
line wrap: on
line diff
--- a/libinterp/corefcn/symrec.h	Wed Mar 21 21:53:40 2018 -0500
+++ b/libinterp/corefcn/symrec.h	Thu Mar 22 17:34:41 2018 -0400
@@ -199,31 +199,19 @@
 
       void clear (context_id context)
       {
-        // There is no need to do anything with a fowarded
-        // symbol_record_rep here.
-        //
-        // For scripts, we are never executing in the script "scope".
-        //
-        // For globals, we are only interested in breaking the link to
-        // the global value and clearing the local value, not the
-        // global one.
-
-        // For persistent values, we clear the value then unmark so
-        // that we clear the first element of the value stack.
+        // For globals, break the link to the global value first, then
+        // clear the local value.
 
         if (is_global ())
-          {
-            unbind_fwd_rep ();
-            return;
-          }
-
-        if (auto t_fwd_rep = m_fwd_rep.lock ())
-          return;
+          unbind_fwd_rep ();
 
         if (! (is_hidden () || is_inherited ()))
           {
             assign (octave_value (), context);
 
+            // For persistent values, we clear the value then unmark so
+            // that we clear the first element of the value stack.
+
             if (is_persistent ())
               unmark_persistent ();
           }
@@ -282,6 +270,11 @@
         return m_storage_class & inherited;
       }
 
+      bool is_forwarded (void) const
+      {
+        return ! m_fwd_rep.expired ();
+      }
+
       bool is_global (void) const
       {
         if (auto t_fwd_rep = m_fwd_rep.lock ())
@@ -499,6 +492,12 @@
 
       void unbind_fwd_rep (void)
       {
+        if (auto t_fwd_rep = m_fwd_rep.lock ())
+          {
+            t_fwd_rep->unbind_fwd_rep ();
+            return;
+          }
+
         // When unbinding an object, only break the immediate link.
         // By doing that, we ensure that any variables that are made
         // global in a script remain linked as globals in the
@@ -629,6 +628,7 @@
     bool is_global (void) const { return m_rep->is_global (); }
     bool is_hidden (void) const { return m_rep->is_hidden (); }
     bool is_inherited (void) const { return m_rep->is_inherited (); }
+    bool is_forwarded (void) const { return m_rep->is_forwarded (); }
     bool is_persistent (void) const { return m_rep->is_persistent (); }
     bool is_added_static (void) const { return m_rep->is_added_static (); }