changeset 16605:2f8fb02a6c31

erase subfunctions when primary function scope is deleted (bug #38691) * symtab.h (symbol_table::fcn_info::fcn_info_rep::clear_map): Rename from clear_unlocked. New arg, force. (symbol_table::fcn_info::fcn_info_rep::clear_autoload_function, (symbol_table::fcn_info::fcn_info::clear_autoload_function, symbol_table::fcn_info::fcn_info_rep::clear_user_function, symbol_table::fcn_info::fcn_info::clear_user_function, symbol_table::fcn_info::fcn_info_rep::clear, symbol_table::fcn_info::clear, symbol_table::clear_all, symbol_table::clear_functions.): New arg, force. (symbol_table::erase_scope): Call erase_subfunctions_in_scope. (symbol_table::do_pop_context, symbol_table::do_clear_global_pattern): Style fixes. * symtab.cc (symbol_table::cleanup): Simplify. * test/bug-38691/module.mk, test/bug-38691/bug-38691.tst, test/bug-38691/dir1/func1.m, test/bug-38691/dir2/func1.m, test/bug-38691/dir2/func2.m, test/bug-38691/dir2/func3.m: New files. * test/Makefile.am: Include bug-38691/module.mk.
author John W. Eaton <jwe@octave.org>
date Fri, 03 May 2013 01:16:41 -0400
parents 5b6d8bfdea25
children 6698d0c42fbf
files libinterp/interpfcn/symtab.cc libinterp/interpfcn/symtab.h libinterp/octave-value/ov-usr-fcn.cc test/Makefile.am test/bug-38691/bug-38691.tst test/bug-38691/dir1/func1.m test/bug-38691/dir2/func1.m test/bug-38691/dir2/func2.m test/bug-38691/dir2/func3.m test/bug-38691/module.mk
diffstat 10 files changed, 103 insertions(+), 54 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/interpfcn/symtab.cc	Fri May 03 00:32:50 2013 -0400
+++ b/libinterp/interpfcn/symtab.cc	Fri May 03 01:16:41 2013 -0400
@@ -1458,27 +1458,12 @@
 
 void symbol_table::cleanup (void)
 {
-  // Clear variables in top scope.
-  all_instances[xtop_scope]->clear_variables ();
-
-  // Clear function table. This is a hard clear, ignoring mlocked functions.
-  fcn_table.clear ();
-
-  // Clear variables in global scope.
-  // FIXME: are there any?
-  all_instances[xglobal_scope]->clear_variables ();
-
-  // Clear global variables.
-  global_table.clear ();
+  clear_all (true);
 
   // Delete all possibly remaining scopes.
   for (all_instances_iterator iter = all_instances.begin ();
        iter != all_instances.end (); iter++)
     {
-      scope_id scope = iter->first;
-      if (scope != xglobal_scope && scope != xtop_scope)
-        scope_id_cache::free (scope);
-
       // First zero the table entry to avoid possible duplicate delete.
       symbol_table *inst = iter->second;
       iter->second = 0;
@@ -1487,6 +1472,12 @@
       // deleting other scopes.
       delete inst;
     }
+
+  global_table.clear ();
+  fcn_table.clear ();
+  class_precedence_table.clear ();
+  parent_map.clear ();
+  all_instances.clear ();
 }
 
 void
--- a/libinterp/interpfcn/symtab.h	Fri May 03 00:32:50 2013 -0400
+++ b/libinterp/interpfcn/symtab.h	Fri May 03 01:16:41 2013 -0400
@@ -844,37 +844,33 @@
 
       template <class T>
       void
-      clear_unlocked (std::map<T, octave_value>& map)
+      clear_map (std::map<T, octave_value>& map, bool force = false)
       {
         typename std::map<T, octave_value>::iterator p = map.begin ();
 
         while (p != map.end ())
           {
-            if (p->second.islocked ())
+            if (force || ! p->second.islocked ())
+              map.erase (p++);
+            else
               p++;
-            else
-              map.erase (p++);
           }
       }
 
-      void clear_autoload_function (void)
+      void clear_autoload_function (bool force = false)
       {
-        if (! autoload_function.islocked ())
+        if (force || ! autoload_function.islocked ())
           autoload_function = octave_value ();
       }
 
       // We also clear command line functions here, as these are both
       // "user defined"
-      void clear_user_function (void)
+      void clear_user_function (bool force = false)
       {
-        if (! function_on_path.islocked ())
-          {
-            function_on_path.erase_subfunctions ();
-
-            function_on_path = octave_value ();
-          }
-
-        if (! cmdline_function.islocked ())
+        if (force || ! function_on_path.islocked ())
+          function_on_path = octave_value ();
+
+        if (force || ! cmdline_function.islocked ())
           cmdline_function = octave_value ();
       }
 
@@ -884,14 +880,15 @@
           clear_user_function ();
       }
 
-      void clear (void)
+      void clear (bool force = false)
       {
-        clear_unlocked (subfunctions);
-        clear_unlocked (private_functions);
-        clear_unlocked (class_constructors);
-        clear_unlocked (class_methods);
-        clear_autoload_function ();
-        clear_user_function ();
+        clear_map (subfunctions, force);
+        clear_map (private_functions, force);
+        clear_map (class_constructors, force);
+        clear_map (class_methods, force);
+
+        clear_autoload_function (force);
+        clear_user_function (force);
       }
 
       void add_dispatch (const std::string& type, const std::string& fname)
@@ -1079,11 +1076,17 @@
       rep->install_built_in_function (f);
     }
 
-    void clear (void) { rep->clear (); }
-
-    void clear_user_function (void) { rep->clear_user_function (); }
-
-    void clear_autoload_function (void) { rep->clear_autoload_function (); }
+    void clear (bool force = false) { rep->clear (force); }
+
+    void clear_user_function (bool force = false)
+    {
+      rep->clear_user_function (force);
+    }
+
+    void clear_autoload_function (bool force = false)
+    {
+      rep->clear_autoload_function (force);
+    }
 
     void clear_mex_function (void) { rep->clear_mex_function (); }
 
@@ -1182,6 +1185,8 @@
   {
     assert (scope != xglobal_scope);
 
+    erase_subfunctions_in_scope (scope);
+
     all_instances_iterator p = all_instances.find (scope);
 
     if (p != all_instances.end ())
@@ -1532,6 +1537,9 @@
       }
   }
 
+  // Install subfunction FCN named NAME.  SCOPE is the scope of the
+  // primary function corresponding to this subfunction.
+
   static void install_subfunction (const std::string& name,
                                    const octave_value& fcn,
                                    scope_id scope)
@@ -1612,13 +1620,13 @@
     clear_variable (name);
   }
 
-  static void clear_all (void)
+  static void clear_all (bool force = false)
   {
     clear_variables ();
 
     clear_global_pattern ("*");
 
-    clear_functions ();
+    clear_functions (force);
   }
 
   static void clear_variables (scope_id scope)
@@ -1643,10 +1651,10 @@
       inst->do_clear_objects ();
   }
 
-  static void clear_functions (void)
+  static void clear_functions (bool force = false)
   {
     for (fcn_table_iterator p = fcn_table.begin (); p != fcn_table.end (); p++)
-      p->second.clear ();
+      p->second.clear (force);
   }
 
   static void clear_function (const std::string& name)
@@ -2609,10 +2617,12 @@
 
   void do_pop_context (void)
   {
-    for (table_iterator p = table.begin (); p != table.end (); )
+    table_iterator p = table.begin ();
+
+    while (p != table.end ())
       {
         if (p->second.pop_context (my_scope) == 0)
-            table.erase (p++);
+          table.erase (p++);
         else
           p++;
       }
@@ -2674,13 +2684,12 @@
           sr.unmark_global ();
       }
 
-
-    for (global_table_iterator q = global_table.begin ();
-         q != global_table.end ();)
+    global_table_iterator q = global_table.begin ();
+
+    while (q != global_table.end ())
       {
         if (pattern.match (q->first))
-          global_table.erase (q++); //Gotta be careful to not
-                                    //invalidate iterators
+          global_table.erase (q++);
         else
           q++;
       }
--- a/libinterp/octave-value/ov-usr-fcn.cc	Fri May 03 00:32:50 2013 -0400
+++ b/libinterp/octave-value/ov-usr-fcn.cc	Fri May 03 01:16:41 2013 -0400
@@ -230,6 +230,7 @@
   delete jit_info;
 #endif
 
+  // FIXME -- this is really playing with fire.
   symbol_table::erase_scope (local_scope);
 }
 
--- a/test/Makefile.am	Fri May 03 00:32:50 2013 -0400
+++ b/test/Makefile.am	Fri May 03 01:16:41 2013 -0400
@@ -54,6 +54,7 @@
 include bug-35448/module.mk
 include bug-36025/module.mk
 include bug-38236/module.mk
+include bug-38691/module.mk
 include classes/module.mk
 include class-concat/module.mk
 include ctor-vs-method/module.mk
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-38691/bug-38691.tst	Fri May 03 01:16:41 2013 -0400
@@ -0,0 +1,13 @@
+%!test
+%! addpath dir1
+%! [d1_r, d1_f1, d1_f2, d1_f3] = func1 (0);
+%! addpath dir2
+%! [d2_r, d2_f1, d2_f2, d2_f3] = func1 (0);
+%! assert (d1_r, 0);
+%! assert (d2_r, 1);
+%! assert (d1_f1, "dir1/func1");
+%! assert (d1_f2, "dir1/func2");
+%! assert (d1_f3, "dir1/func3");
+%! assert (d2_f1, "dir2/func1");
+%! assert (d2_f2, "dir2/func2");
+%! assert (d2_f3, "dir2/func3");
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-38691/dir1/func1.m	Fri May 03 01:16:41 2013 -0400
@@ -0,0 +1,14 @@
+function [r, f1, f2, f3] = func1 (x)
+  f1 = "dir1/func1";
+  [r, f2, f3] = feval ("func2", x);
+endfunction
+
+function [r, f2, f3] = func2 (x)
+  f2 = "dir1/func2";
+  [r, f3] = feval ("func3", x);
+endfunction
+
+function [r, f3] = func3 (x)
+  f3 = "dir1/func3";
+  r = x;
+endfunction
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-38691/dir2/func1.m	Fri May 03 01:16:41 2013 -0400
@@ -0,0 +1,4 @@
+function [r, f1, f2, f3] = func1 (x)
+  f1 = "dir2/func1";
+  [r, f2, f3] = feval ("func2", x);
+endfunction
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-38691/dir2/func2.m	Fri May 03 01:16:41 2013 -0400
@@ -0,0 +1,4 @@
+function [r, f2, f3] = func2 (x)
+  f2 = "dir2/func2";
+  [r, f3] = feval ("func3", x);
+endfunction
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-38691/dir2/func3.m	Fri May 03 01:16:41 2013 -0400
@@ -0,0 +1,4 @@
+function [r, f3] = func3 (x)
+  f3 = "dir2/func3";
+  r = 1;
+endfunction
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-38691/module.mk	Fri May 03 01:16:41 2013 -0400
@@ -0,0 +1,8 @@
+bug_38691_FCN_FILES = \
+  bug-38691/dir1/func1.m \
+  bug-38691/dir2/func1.m \
+  bug-38691/dir2/func2.m \
+  bug-38691/dir2/func3.m \
+  bug-38691/bug-38691.tst
+
+FCN_FILES += $(bug_38691_FCN_FILES)