# HG changeset patch # User John W. Eaton # Date 1236107454 18000 # Node ID ab87d08d9a1b3d566f983f54b66567b386ba564e # Parent 24dd61b36591a1624a62cc8c2162db4746fbc1f9 improve symbol inheritance for anonymous functions diff -r 24dd61b36591 -r ab87d08d9a1b src/ChangeLog --- a/src/ChangeLog Tue Mar 03 14:02:20 2009 -0500 +++ b/src/ChangeLog Tue Mar 03 14:10:54 2009 -0500 @@ -1,5 +1,16 @@ 2009-03-03 John W. Eaton + * symtab.h (symbol_table::do_inherit): Only inherit values for + symbols from the donor_scope that already exist in the table. + (symbol_table::symbol_record::symbol_record_rep::dup): Now const. + (symbol_table::symbol_record::operator=): Decrement rep->count and + maybe delete rep. + (symbol_table::fcn_info::operator=): Likewise. + + * pt-fcn-handle.cc: (tree_anon_fcn_handle::dup): Transform + tree_anon_fcn_handle objects to tree_constant objects containing + octave_fcn_handle objects. New tests. + * pt-assign.cc (tree_simple_assignment::rvalue1): Assign result of call to rhs->rvalue1() to an octave_value object, not an octave_value_list object. diff -r 24dd61b36591 -r ab87d08d9a1b src/pt-fcn-handle.cc --- a/src/pt-fcn-handle.cc Tue Mar 03 14:02:20 2009 -0500 +++ b/src/pt-fcn-handle.cc Tue Mar 03 14:10:54 2009 -0500 @@ -31,6 +31,7 @@ #include "ov-fcn-handle.h" #include "pt-fcn-handle.h" #include "pager.h" +#include "pt-const.h" #include "pt-walk.h" #include "variables.h" @@ -87,6 +88,9 @@ octave_value tree_anon_fcn_handle::rvalue1 (int) { + // FIXME -- should CMD_LIST be limited to a single expression? + // I think that is what Matlab does. + tree_parameter_list *param_list = parameter_list (); tree_parameter_list *ret_list = return_list (); tree_statement_list *cmd_list = body (); @@ -127,6 +131,21 @@ return fh; } +/* +%!function r = f2 (f, x) +%! r = f (x); +%!function f = f1 (k) +%! f = @(x) f2 (@(y) y-k, x); +%!test +%! assert ((f1 (3)) (10) == 7) +%! +%!shared f, g, h +%! h = @(x) sin (x); +%! g = @(f, x) h (x); +%! f = @() g (@(x) h, pi); +%!assert (f () == sin (pi)) +*/ + octave_value_list tree_anon_fcn_handle::rvalue (int nargout) { @@ -140,6 +159,7 @@ return retval; } +#if 0 tree_expression * tree_anon_fcn_handle::dup (symbol_table::scope_id parent_scope, symbol_table::context_id parent_context) @@ -164,6 +184,55 @@ return new_afh; } +#endif + +tree_expression * +tree_anon_fcn_handle::dup (symbol_table::scope_id, symbol_table::context_id) +{ + // Instead of simply duplicating, transform to a tree_constant + // object that contains an octave_fcn_handle object with the symbol + // table of the referenced function primed with values from the + // current scope and context. + + tree_parameter_list *param_list = parameter_list (); + tree_parameter_list *ret_list = return_list (); + tree_statement_list *cmd_list = body (); + symbol_table::scope_id this_scope = scope (); + + symbol_table::scope_id new_scope = symbol_table::dup_scope (this_scope); + + if (new_scope > 0) + symbol_table::inherit (new_scope, symbol_table::current_scope (), + symbol_table::current_context ()); + + octave_user_function *uf + = new octave_user_function (new_scope, + param_list ? param_list->dup (new_scope, 0) : 0, + ret_list ? ret_list->dup (new_scope, 0) : 0, + cmd_list ? cmd_list->dup (new_scope, 0) : 0); + + octave_function *curr_fcn = octave_call_stack::current (); + + if (curr_fcn) + { + uf->stash_parent_fcn_name (curr_fcn->name ()); + + symbol_table::scope_id parent_scope = curr_fcn->parent_fcn_scope (); + + if (parent_scope < 0) + parent_scope = curr_fcn->scope (); + + uf->stash_parent_fcn_scope (parent_scope); + } + + uf->mark_as_inline_function (); + + octave_value ov_fcn (uf); + + octave_value fh (new octave_fcn_handle (ov_fcn, "@")); + + return new tree_constant (fh, line (), column ()); +} void tree_anon_fcn_handle::accept (tree_walker& tw) diff -r 24dd61b36591 -r ab87d08d9a1b src/symtab.h --- a/src/symtab.h Tue Mar 03 14:02:20 2009 -0500 +++ b/src/symtab.h Tue Mar 03 14:10:54 2009 -0500 @@ -370,7 +370,7 @@ symbol_table::erase_persistent (name); } - symbol_record_rep *dup (void) + symbol_record_rep *dup (void) const { return new symbol_record_rep (name, varval (xcurrent_context), storage_class); @@ -412,6 +412,9 @@ { if (this != &sr) { + if (--rep->count == 0) + delete rep; + rep = sr.rep; rep->count++; } @@ -754,16 +757,19 @@ fcn_info (const std::string& nm = std::string ()) : rep (new fcn_info_rep (nm)) { } - fcn_info (const fcn_info& ov) : rep (ov.rep) + fcn_info (const fcn_info& fi) : rep (fi.rep) { rep->count++; } - fcn_info& operator = (const fcn_info& ov) + fcn_info& operator = (const fcn_info& fi) { - if (this != &ov) + if (this != &fi) { - rep = ov.rep; + if (--rep->count == 0) + delete rep; + + rep = fi.rep; rep->count++; } @@ -1991,50 +1997,26 @@ return p->second; } - void do_inherit (symbol_table& donor_symbol_table, context_id donor_context) + void do_inherit (symbol_table& donor_table, context_id donor_context) { - // Copy all variables from the donor scope in case they are needed - // in a subsequent anonymous function. For example, to allow - // - // function r = f2 (f, x) - // r = f (x); - // end - // - // function f = f1 (k) - // f = @(x) f2 (@(y) y-k, x); - // end - // - // f = f1 (3) ==> @(x) fcn2 (@(y) y - k, x) - // f (10) ==> 7 - // - // to work as expected. - // - // FIXME -- is there a better way to accomplish this? - - std::map donor_table = donor_symbol_table.table; - - for (table_iterator p = donor_table.begin (); p != donor_table.end (); p++) + for (table_iterator p = table.begin (); p != table.end (); p++) { - symbol_record& donor_sr = p->second; - - std::string nm = donor_sr.name (); - - symbol_record& sr = do_insert (nm); - - if (! (sr.is_automatic () || sr.is_formal () || nm == "__retval__")) + symbol_record& sr = p->second; + + if (! (sr.is_automatic () || sr.is_formal ())) { - octave_value val = donor_sr.varval (donor_context); - - if (val.is_defined ()) + std::string nm = sr.name (); + + if (nm != "__retval__") { - // Currently, inherit is always called when creating a - // new table, so it only makes sense to copy values into - // the base context (== 0), but maybe the context - // should be passed in as a parameter instead? - - sr.varref (0) = val; - - sr.mark_inherited (); + octave_value val = donor_table.do_varval (nm, donor_context); + + if (val.is_defined ()) + { + sr.varref (0) = val; + + sr.mark_inherited (); + } } } }