changeset 28430:5bfa8e018704 stable

store local init vars for anonymous functions in handle, not function object This change is step toward revamping function handles by storing variable init values for anonymous functions in function handle objects instead of in the corresponding functions. * call-stack.h, call-stack.cc (call_stack::push): New overload that accepts local variable map in addition to function object. * stack-frame.h (user_fcn_stack_frame::user_fcn_stack_frame): New constructor that accepts local variable map in addition to function object. (stack_frame::local_vars_map): New typedef. * ov-fcn-handle.h, ov-fcn-handle.cc (octave_fcn_handle::m_local_vars): New data member. (octave_fcn_handle::octave_fcn_handle): Update existing constructors and provide new one to construct handle from function object and local variable map. (octave_fcn_handle::call): If m_local_vars is defined, push stack frame with that info and execute function here. (octave_fcn_handle::workspace): Create workspace struct from m_local_vars instead of getting that info from the function object. (octave_fcn_handle::parse_anon_fcn_handle): Copy m_local_vars from new function handle object. (octave_fcn_handle::save_ascii, octave_fcn_handle::save_binary, octave_fcn_handle::save_hdf5): Use m_local_vars instead of getting info from function object. * ov-usr-fcn.h, ov-usr-fcn.cc (octave_user_function::local_vars_map): Delete typedef. (octave_user_function::m_local_var_init_vals): Delete data member and all uses. (octave_user_function::local_var_init_vals): Delete. * pt-eval.h, pt-eval.cc (tree_evaluator::push_stack_frame): New overload that accepts local variable map and user function. (tree_evaluator::init_local_fcn_vars): Delete function and all uses. * pt-fcn-handle.cc (tree_anon_fcn_handle::evaluate): Store local variables in function handle object instead of function object.
author John W. Eaton <jwe@octave.org>
date Mon, 30 Mar 2020 15:14:10 -0400
parents 8eb8ba8aff9a
children 0ffae065ca03
files libinterp/corefcn/call-stack.cc libinterp/corefcn/call-stack.h libinterp/corefcn/stack-frame.cc libinterp/corefcn/stack-frame.h libinterp/octave-value/ov-fcn-handle.cc libinterp/octave-value/ov-fcn-handle.h libinterp/octave-value/ov-usr-fcn.cc libinterp/octave-value/ov-usr-fcn.h libinterp/parse-tree/pt-eval.cc libinterp/parse-tree/pt-eval.h libinterp/parse-tree/pt-fcn-handle.cc
diffstat 11 files changed, 137 insertions(+), 76 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/call-stack.cc	Mon Mar 30 10:43:47 2020 -0400
+++ b/libinterp/corefcn/call-stack.cc	Mon Mar 30 15:14:10 2020 -0400
@@ -401,6 +401,25 @@
     m_cs.push_back (new_frame);
   }
 
+  void call_stack::push (octave_user_function *fcn,
+                         const stack_frame::local_vars_map& local_vars)
+  {
+    size_t prev_frame = m_curr_frame;
+    m_curr_frame = m_cs.size ();
+
+    // m_max_stack_depth should never be less than zero.
+    if (m_curr_frame > static_cast<size_t> (m_max_stack_depth))
+      error ("max_stack_depth exceeded");
+
+    std::shared_ptr<stack_frame> slink = get_static_link (prev_frame);
+
+    std::shared_ptr<stack_frame>
+      new_frame (stack_frame::create (m_evaluator, fcn, m_curr_frame, slink,
+                                      local_vars));
+
+    m_cs.push_back (new_frame);
+  }
+
   void call_stack::push (octave_user_script *script)
   {
     size_t prev_frame = m_curr_frame;
--- a/libinterp/corefcn/call-stack.h	Mon Mar 30 10:43:47 2020 -0400
+++ b/libinterp/corefcn/call-stack.h	Mon Mar 30 15:14:10 2020 -0400
@@ -160,6 +160,9 @@
     void push (octave_user_function *fcn,
                const std::shared_ptr<stack_frame>& closure_frames = std::shared_ptr<stack_frame> ());
 
+    void push (octave_user_function *fcn,
+               const stack_frame::local_vars_map& local_vars);
+
     void push (octave_user_script *script);
 
     void push (octave_function *fcn);
--- a/libinterp/corefcn/stack-frame.cc	Mon Mar 30 10:43:47 2020 -0400
+++ b/libinterp/corefcn/stack-frame.cc	Mon Mar 30 15:14:10 2020 -0400
@@ -406,6 +406,20 @@
         m_fcn (fcn), m_unwind_protect_frame (nullptr)
     { }
 
+    user_fcn_stack_frame (tree_evaluator& tw, octave_user_function *fcn,
+                          size_t index,
+                          const std::shared_ptr<stack_frame>& static_link,
+                          const local_vars_map& local_vars)
+      : base_value_stack_frame (tw, get_num_symbols (fcn), index, static_link,
+                                get_access_link (fcn, static_link)),
+        m_fcn (fcn), m_unwind_protect_frame (nullptr)
+    {
+      // Initialize local variable values.
+
+      for (const auto& nm_ov : local_vars)
+        assign (nm_ov.first, nm_ov.second);
+    }
+
     user_fcn_stack_frame (const user_fcn_stack_frame& elt) = default;
 
     user_fcn_stack_frame&
@@ -1040,6 +1054,14 @@
   }
 
   stack_frame * stack_frame::create (tree_evaluator& tw,
+                                     octave_user_function *fcn, size_t index,
+                                     const std::shared_ptr<stack_frame>& static_link,
+                                     const local_vars_map& local_vars)
+  {
+    return new user_fcn_stack_frame (tw, fcn, index, static_link, local_vars);
+  }
+
+  stack_frame * stack_frame::create (tree_evaluator& tw,
                                      const symbol_scope& scope, size_t index,
                                      const std::shared_ptr<stack_frame>& static_link)
   {
--- a/libinterp/corefcn/stack-frame.h	Mon Mar 30 10:43:47 2020 -0400
+++ b/libinterp/corefcn/stack-frame.h	Mon Mar 30 15:14:10 2020 -0400
@@ -40,8 +40,6 @@
 
 #include "error.h"
 #include "ov-fcn.h"
-#include "ov-fcn.h"
-#include "ov-fcn-handle.h"
 #include "ov-usr-fcn.h"
 #include "syminfo.h"
 #include "symscope.h"
@@ -111,6 +109,8 @@
   {
   public:
 
+    typedef std::map<std::string, octave_value> local_vars_map;
+
     // Markers indicating the type of a variable.  Values for local
     // variables are stored in the stack frame.  Values for
     // global variables are stored in the tree_evaluator object that
@@ -163,6 +163,12 @@
             const std::shared_ptr<stack_frame>& static_link,
             const std::shared_ptr<stack_frame>& access_link = std::shared_ptr<stack_frame> ());
 
+    // Anonymous user-defined function with init vars.
+    static stack_frame *
+    create (tree_evaluator& tw, octave_user_function *fcn, size_t index,
+            const std::shared_ptr<stack_frame>& static_link,
+            const local_vars_map& local_vars);
+
     // Scope.
     static stack_frame *
     create (tree_evaluator& tw, const symbol_scope& scope, size_t index,
@@ -216,7 +222,7 @@
             retval = parent_fcn_name + '>';
 
           if (fcn->is_anonymous_function ())
-            retval += octave_fcn_handle::anonymous;
+            retval += "@<anonymous>";
           else
             retval += fcn->name ();
         }
--- a/libinterp/octave-value/ov-fcn-handle.cc	Mon Mar 30 10:43:47 2020 -0400
+++ b/libinterp/octave-value/ov-fcn-handle.cc	Mon Mar 30 15:14:10 2020 -0400
@@ -63,6 +63,7 @@
 #include "pt-misc.h"
 #include "pt-pr-code.h"
 #include "pt-stmt.h"
+#include "stack-frame.h"
 #include "syminfo.h"
 #include "symscope.h"
 #include "unwind-prot.h"
@@ -85,7 +86,7 @@
 octave_fcn_handle::octave_fcn_handle (const octave::symbol_scope& scope,
                                       const std::string& n)
   : m_fcn (), m_obj (), m_name (n), m_scope (scope), m_is_nested (false),
-    m_closure_frames (), m_dispatch_class ()
+    m_closure_frames (), m_local_vars (nullptr), m_dispatch_class ()
 {
   if (! m_name.empty () && m_name[0] == '@')
     m_name = m_name.substr (1);
@@ -119,7 +120,7 @@
                                       const octave_value& f,
                                       const std::string& n)
   : m_fcn (f), m_obj (), m_name (n), m_scope (scope), m_is_nested (false),
-    m_closure_frames (), m_dispatch_class ()
+    m_closure_frames (), m_local_vars (nullptr), m_dispatch_class ()
 {
   octave_user_function *uf = m_fcn.user_function_value (true);
 
@@ -138,7 +139,7 @@
 octave_fcn_handle::octave_fcn_handle (const octave_value& f,
                                       const std::string& n)
   : m_fcn (f), m_obj (), m_name (n), m_scope (), m_is_nested (false),
-    m_closure_frames (), m_dispatch_class ()
+    m_closure_frames (), m_local_vars (nullptr), m_dispatch_class ()
 {
   octave_user_function *uf = m_fcn.user_function_value (true);
 
@@ -154,6 +155,27 @@
     m_is_nested = true;
 }
 
+octave_fcn_handle::octave_fcn_handle (const octave_value& f,
+                                      const octave::stack_frame::local_vars_map& local_vars)
+  : m_fcn (f), m_obj (), m_name (anonymous), m_scope (), m_is_nested (false),
+    m_closure_frames (), m_local_vars (new octave::stack_frame::local_vars_map (local_vars)),
+    m_dispatch_class ()
+{ }
+
+octave_fcn_handle::octave_fcn_handle (const octave_fcn_handle& fh)
+  : octave_base_value (fh), m_fcn (fh.m_fcn), m_obj (fh.m_obj),
+    m_name (fh.m_name), m_scope (fh.m_scope), m_is_nested (fh.m_is_nested),
+    m_closure_frames (fh.m_closure_frames),
+    m_local_vars (fh.m_local_vars
+                  ? new octave::stack_frame::local_vars_map (*(fh.m_local_vars)) : nullptr),
+    m_dispatch_class (fh.m_dispatch_class)
+{ }
+
+octave_fcn_handle::~octave_fcn_handle (void)
+{
+  delete m_local_vars;
+}
+
 octave_value_list
 octave_fcn_handle::subsref (const std::string& type,
                             const std::list<octave_value_list>& idx,
@@ -381,6 +403,23 @@
 
       return oct_usr_fcn->execute (tw, nargout, args);
     }
+  else if (m_local_vars)
+    {
+      if (! fcn_to_call.is_user_function ())
+        {
+          std::string tname = fcn_to_call.type_name ();
+          error ("internal error: local vars associated with '%s' object",
+                 tname.c_str ());
+        }
+
+      octave_user_function *oct_usr_fcn = fcn_to_call.user_function_value ();
+
+      tw.push_stack_frame (oct_usr_fcn, *m_local_vars);
+
+      octave::unwind_action act1 ([&tw] () { tw.pop_stack_frame (); });
+
+      return oct_usr_fcn->execute (tw, nargout, args);
+    }
   else
     {
       octave_function *oct_fcn = fcn_to_call.function_value ();
@@ -443,13 +482,11 @@
 {
   if (m_name == anonymous)
     {
-      octave_user_function *fu = m_fcn.user_function_value ();
-
       octave_scalar_map ws;
 
-      if (fu)
+      if (m_local_vars)
         {
-          for (const auto& nm_val : fu->local_var_init_vals ())
+          for (const auto& nm_val : *m_local_vars)
             ws.assign (nm_val.first, nm_val.second);
         }
 
@@ -578,23 +615,18 @@
       if (m_fcn.is_undefined ())
         return false;
 
-      octave_user_function *f = m_fcn.user_function_value ();
-
-      octave_user_function::local_vars_map local_vars
-        = f->local_var_init_vals ();
-
-      size_t varlen = local_vars.size ();
-
       os << m_name << "\n";
 
       print_raw (os, true);
       os << "\n";
 
+      size_t varlen = m_local_vars ? m_local_vars->size () : 0;
+
       if (varlen > 0)
         {
           os << "# length: " << varlen << "\n";
 
-          for (const auto& nm_val : local_vars)
+          for (const auto& nm_val : *m_local_vars)
             {
               if (! save_text_data (os, nm_val.second, nm_val.first, false, 0))
                 return ! os.fail ();
@@ -635,6 +667,9 @@
         {
           m_fcn = fh->m_fcn;
 
+          if (fh->m_local_vars)
+            m_local_vars = new octave::stack_frame::local_vars_map (*(fh->m_local_vars));
+
           octave_user_function *uf = m_fcn.user_function_value (true);
 
           if (uf)
@@ -754,12 +789,7 @@
       if (m_fcn.is_undefined ())
         return false;
 
-      octave_user_function *f = m_fcn.user_function_value ();
-
-      octave_user_function::local_vars_map local_vars
-        = f->local_var_init_vals ();
-
-      size_t varlen = local_vars.size ();
+      size_t varlen = m_local_vars ? m_local_vars->size () : 0;
 
       if (varlen > 0)
         nmbuf << m_name << ' ' << varlen;
@@ -780,7 +810,7 @@
 
       if (varlen > 0)
         {
-          for (const auto& nm_val : local_vars)
+          for (const auto& nm_val : *m_local_vars)
             {
               if (! save_binary_data (os, nm_val.second, nm_val.first,
                                       "", 0, save_as_floats))
@@ -1002,12 +1032,7 @@
 
       H5Dclose (data_hid);
 
-      octave_user_function *f = m_fcn.user_function_value ();
-
-      octave_user_function::local_vars_map local_vars
-        = f->local_var_init_vals ();
-
-      size_t varlen = local_vars.size ();
+      size_t varlen = m_local_vars ? m_local_vars->size () : 0;
 
       if (varlen > 0)
         {
@@ -1052,12 +1077,13 @@
               return false;
             }
 
-          for (const auto& nm_val : local_vars)
+          for (const auto& nm_val : *m_local_vars)
             {
               if (! add_hdf5_data (data_hid, nm_val.second, nm_val.first,
                                    "", false, save_as_floats))
                 break;
             }
+
           H5Gclose (data_hid);
         }
     }
--- a/libinterp/octave-value/ov-fcn-handle.h	Mon Mar 30 10:43:47 2020 -0400
+++ b/libinterp/octave-value/ov-fcn-handle.h	Mon Mar 30 15:14:10 2020 -0400
@@ -35,12 +35,12 @@
 #include "ov-base.h"
 #include "ov-fcn.h"
 #include "ov-typeinfo.h"
+#include "stack-frame.h"
 #include "symscope.h"
 
 namespace octave
 {
   class interpreter;
-  class stack_frame;
   class tree_evaluator;
 }
 
@@ -56,21 +56,22 @@
 
   octave_fcn_handle (void)
     : m_fcn (), m_obj (), m_name (), m_scope (), m_is_nested (false),
-      m_closure_frames (), m_dispatch_class ()
+      m_closure_frames (), m_local_vars (nullptr), m_dispatch_class ()
   { }
 
   octave_fcn_handle (const octave::symbol_scope& scope, const std::string& n);
 
   octave_fcn_handle (const octave::symbol_scope& scope,
-                     const octave_value& f,
-                     const std::string& n = anonymous);
+                     const octave_value& f, const std::string& n);
+
+  octave_fcn_handle (const octave_value& f, const std::string& n);
 
   octave_fcn_handle (const octave_value& f,
-                     const std::string& n = anonymous);
+                     const octave::stack_frame::local_vars_map& local_vars);
 
-  octave_fcn_handle (const octave_fcn_handle& fh) = default;
+  octave_fcn_handle (const octave_fcn_handle& fh);
 
-  ~octave_fcn_handle (void) = default;
+  ~octave_fcn_handle (void);
 
   octave_base_value * clone (void) const
   { return new octave_fcn_handle (*this); }
@@ -186,6 +187,9 @@
   // functions indirectly through handles.
   std::shared_ptr<octave::stack_frame> m_closure_frames;
 
+  // List of captured variable values for anonymous fucntions.
+  octave::stack_frame::local_vars_map *m_local_vars;
+
   // The name of the class in which this handle was created, if any.
   // Used to determine access permission when the referenced function is
   // called.
--- a/libinterp/octave-value/ov-usr-fcn.cc	Mon Mar 30 10:43:47 2020 -0400
+++ b/libinterp/octave-value/ov-usr-fcn.cc	Mon Mar 30 15:14:10 2020 -0400
@@ -213,11 +213,9 @@
 
 octave_user_function::octave_user_function
   (const octave::symbol_scope& scope, octave::tree_parameter_list *pl,
-   octave::tree_parameter_list *rl, octave::tree_statement_list *cl,
-   const local_vars_map& lviv)
+   octave::tree_parameter_list *rl, octave::tree_statement_list *cl)
   : octave_user_code ("", "", scope, cl, ""),
     param_list (pl), ret_list (rl),
-    m_local_var_init_vals (lviv),
     lead_comm (), trail_comm (),
     location_line (0), location_column (0),
     parent_name (), system_fcn_file (false),
--- a/libinterp/octave-value/ov-usr-fcn.h	Mon Mar 30 10:43:47 2020 -0400
+++ b/libinterp/octave-value/ov-usr-fcn.h	Mon Mar 30 15:14:10 2020 -0400
@@ -213,13 +213,10 @@
 {
 public:
 
-  typedef std::map<std::string, octave_value> local_vars_map;
-
   octave_user_function (const octave::symbol_scope& scope = octave::symbol_scope (),
                         octave::tree_parameter_list *pl = nullptr,
                         octave::tree_parameter_list *rl = nullptr,
-                        octave::tree_statement_list *cl = nullptr,
-                        const local_vars_map& lviv = local_vars_map ());
+                        octave::tree_statement_list *cl = nullptr);
 
   // No copying!
 
@@ -397,11 +394,6 @@
 
   octave::comment_list * trailing_comment (void) { return trail_comm; }
 
-  const local_vars_map& local_var_init_vals (void) const
-  {
-    return m_local_var_init_vals;
-  }
-
   // If is_special_expr is true, retrieve the sigular expression that forms the
   // body.  May be null (even if is_special_expr is true).
   octave::tree_expression * special_expr (void);
@@ -437,9 +429,6 @@
   // this function.
   octave::tree_parameter_list *ret_list;
 
-  // For anonymous function values inherited from parent scope.
-  local_vars_map m_local_var_init_vals;
-
   // The comments preceding the FUNCTION token.
   octave::comment_list *lead_comm;
 
--- a/libinterp/parse-tree/pt-eval.cc	Mon Mar 30 10:43:47 2020 -0400
+++ b/libinterp/parse-tree/pt-eval.cc	Mon Mar 30 15:14:10 2020 -0400
@@ -61,6 +61,7 @@
 #include "pt-anon-scopes.h"
 #include "pt-eval.h"
 #include "pt-tm-const.h"
+#include "stack-frame.h"
 #include "symtab.h"
 #include "unwind-prot.h"
 #include "utils.h"
@@ -1534,6 +1535,12 @@
     m_call_stack.push (fcn, closure_frames);
   }
 
+  void tree_evaluator::push_stack_frame (octave_user_function *fcn,
+                                         const stack_frame::local_vars_map& local_vars)
+  {
+    m_call_stack.push (fcn, local_vars);
+  }
+
   void tree_evaluator::push_stack_frame (octave_user_script *script)
   {
     m_call_stack.push (script);
@@ -2398,9 +2405,6 @@
                         nargout, user_function.takes_varargs (),
                         user_function.all_va_args (args));
 
-    if (user_function.is_anonymous_function ())
-      init_local_fcn_vars (user_function);
-
     tree_parameter_list *param_list = user_function.parameter_list ();
 
     if (param_list && ! param_list->varargs_only ())
@@ -3829,18 +3833,6 @@
       assign ("varargin", va_args.cell_value ());
   }
 
-  void tree_evaluator::init_local_fcn_vars (octave_user_function& user_fcn)
-  {
-    std::shared_ptr<stack_frame> frame
-      = m_call_stack.get_current_stack_frame ();
-
-    const octave_user_function::local_vars_map& lviv
-      = user_fcn.local_var_init_vals ();
-
-    for (const auto& nm_ov : lviv)
-      frame->assign (nm_ov.first, nm_ov.second);
-  }
-
   std::string
   tree_evaluator::check_autoload_file (const std::string& nm) const
   {
--- a/libinterp/parse-tree/pt-eval.h	Mon Mar 30 10:43:47 2020 -0400
+++ b/libinterp/parse-tree/pt-eval.h	Mon Mar 30 15:14:10 2020 -0400
@@ -42,6 +42,7 @@
 #include "ovl.h"
 #include "profiler.h"
 #include "pt-walk.h"
+#include "stack-frame.h"
 
 class octave_user_code;
 
@@ -378,6 +379,9 @@
     void push_stack_frame (octave_user_function *fcn,
                            const std::shared_ptr<stack_frame>& closure_frames = std::shared_ptr<stack_frame> ());
 
+    void push_stack_frame (octave_user_function *fcn,
+                           const stack_frame::local_vars_map& local_vars);
+
     void push_stack_frame (octave_user_script *script);
 
     void push_stack_frame (octave_function *fcn);
@@ -737,8 +741,6 @@
                              int nargout, bool takes_varargs,
                              const octave_value_list& va_args);
 
-    void init_local_fcn_vars (octave_user_function& user_fcn);
-
     std::string check_autoload_file (const std::string& nm) const;
 
     interpreter& m_interpreter;
--- a/libinterp/parse-tree/pt-fcn-handle.cc	Mon Mar 30 10:43:47 2020 -0400
+++ b/libinterp/parse-tree/pt-fcn-handle.cc	Mon Mar 30 15:14:10 2020 -0400
@@ -32,6 +32,7 @@
 #include "interpreter-private.h"
 #include "pt-anon-scopes.h"
 #include "pt-fcn-handle.h"
+#include "stack-frame.h"
 
 namespace octave
 {
@@ -130,7 +131,7 @@
 
     std::set<std::string> free_vars = anon_fcn_ctx.free_variables ();
 
-    octave_user_function::local_vars_map local_var_init_vals;
+    stack_frame::local_vars_map local_vars;
 
     call_stack& cs = tw.get_call_stack ();
 
@@ -141,12 +142,12 @@
         octave_value val = frame->varval (name);
 
         if (val.is_defined ())
-          local_var_init_vals[name] = val;
+          local_vars[name] = val;
       }
 
     octave_user_function *af
       = new octave_user_function (new_scope, param_list_dup, ret_list,
-                                  stmt_list, local_var_init_vals);
+                                  stmt_list);
 
     octave_function *curr_fcn = cs.current_function ();
 
@@ -176,8 +177,7 @@
 
     // octave_value fh (octave_fcn_binder::maybe_binder (ov_fcn, m_interpreter));
 
-    return octave_value (new octave_fcn_handle
-                         (ov_fcn, octave_fcn_handle::anonymous));
+    return octave_value (new octave_fcn_handle (ov_fcn, local_vars));
   }
 }