# HG changeset patch # User John W. Eaton # Date 1367558201 14400 # Node ID 2f8fb02a6c311a05a9a4a23fe45d0322d880ab31 # Parent 5b6d8bfdea25ed428ba8540692cae3b396c07a79 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. diff -r 5b6d8bfdea25 -r 2f8fb02a6c31 libinterp/interpfcn/symtab.cc --- 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 diff -r 5b6d8bfdea25 -r 2f8fb02a6c31 libinterp/interpfcn/symtab.h --- 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 void - clear_unlocked (std::map& map) + clear_map (std::map& map, bool force = false) { typename std::map::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++; } diff -r 5b6d8bfdea25 -r 2f8fb02a6c31 libinterp/octave-value/ov-usr-fcn.cc --- 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); } diff -r 5b6d8bfdea25 -r 2f8fb02a6c31 test/Makefile.am --- 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 diff -r 5b6d8bfdea25 -r 2f8fb02a6c31 test/bug-38691/bug-38691.tst --- /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"); diff -r 5b6d8bfdea25 -r 2f8fb02a6c31 test/bug-38691/dir1/func1.m --- /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 diff -r 5b6d8bfdea25 -r 2f8fb02a6c31 test/bug-38691/dir2/func1.m --- /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 diff -r 5b6d8bfdea25 -r 2f8fb02a6c31 test/bug-38691/dir2/func2.m --- /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 diff -r 5b6d8bfdea25 -r 2f8fb02a6c31 test/bug-38691/dir2/func3.m --- /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 diff -r 5b6d8bfdea25 -r 2f8fb02a6c31 test/bug-38691/module.mk --- /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)