changeset 8906:ab87d08d9a1b

improve symbol inheritance for anonymous functions
author John W. Eaton <jwe@octave.org>
date Tue, 03 Mar 2009 14:10:54 -0500
parents 24dd61b36591
children 5a956c026b6c
files src/ChangeLog src/pt-fcn-handle.cc src/symtab.h
diffstat 3 files changed, 107 insertions(+), 45 deletions(-) [+]
line wrap: on
line diff
--- 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  <jwe@octave.org>
 
+	* 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.
--- 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, "@<anonymous>"));
+
+  return new tree_constant (fh, line (), column ());
+}
 
 void
 tree_anon_fcn_handle::accept (tree_walker& tw)
--- 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<std::string, symbol_record> 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 ();
+		  }
 	      }
 	  }
       }