Mercurial > jwe > octave
changeset 26828:6e9034836239
allow handles to nested functions to work (bug #39257)
* stack-frame.h (stack_frame::set_closure_links): New function.
(usr_fcn_stack_frame::usr_fcn_stack_frame): New optional argument,
ACCESS_LINK, to allow access link to be set directly when creating
stack frames for handles to nested functions.
* call-stack.h, call-stack.cc (call_stack::push): New argument,
CLOSURE_FRAMES, in method to push user function objects on the call
stack to allow call stack context for handles to nested functions to
be set.
* symscope.h (symbol_scope::symbol_scope_rep::is_parent,
symbol_scope::is_parent): New functions.
* ov-fcn-handle.h, ov-fcn-handle.cc
(octave_fcn_handle::m_closure_frames, octave_fcn_handle::m_is_nested):
New data members.
(octave_fcn_handle::is_nested): New function.
(octave_fcn_handle::~octave_fcn_handle): Delete saved closure frames.
(octave_fcn_handle::octave_fcn_handle): Mark handles to nested
functions as nested instead of throwing error.
(octave_fcn_handle::push_closure_context,
octave_fcn_handle::workspace): New functions.
(Ffunctions): Handle nested functions.
(octave_fcn_handle::call): Pass pointer to first element of
closure_frames list to feval when executing a function.
* ov-fcn.h, ov-fcn.cc (octave_function::call): New overload with
pointer to closure_context stack frames as argument. Provide default
implementation.
* ov-usr-fcn.h, ov-usr-fcn.cc (octave_user_function::parent_function):
New member variable.
(octave_user_function::is_parent_function): New function.
(octave_user_function::call): Primary definition is now overload that
includes closure context. The other form now forwards to the new
version.
* pt-eval.cc (tree_evaluator::execute_user_function): New arg,
CLOSURE_FRAMES. Pass them to the call stack along with the user function.
Push current stack frame to any function handles returned from a
nested function or the parent of a nested function.
* test/nest/nst1.m, test/nest/nst2.m, test/nest/nst3.m: New files.
* test/nest/nest.tst: New tests.
| author | John W. Eaton <jwe@octave.org> |
|---|---|
| date | Sun, 03 Mar 2019 10:19:36 +0000 |
| parents | d9770844392e |
| children | 20881d195d20 |
| files | libinterp/corefcn/call-stack.cc libinterp/corefcn/call-stack.h libinterp/corefcn/stack-frame.h libinterp/corefcn/symscope.h libinterp/octave-value/ov-fcn-handle.cc libinterp/octave-value/ov-fcn-handle.h libinterp/octave-value/ov-fcn.cc libinterp/octave-value/ov-fcn.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 test/nest/nest.tst test/nest/nst1.m test/nest/nst2.m test/nest/nst3.m |
| diffstat | 16 files changed, 353 insertions(+), 53 deletions(-) [+] |
line wrap: on
line diff
--- a/libinterp/corefcn/call-stack.cc Sun Mar 03 07:08:52 2019 +0000 +++ b/libinterp/corefcn/call-stack.cc Sun Mar 03 10:19:36 2019 +0000 @@ -437,7 +437,8 @@ m_cs.push_back (new scope_stack_frame (*this, prev_frame, scope, slink)); } - void call_stack::push (octave_user_function *fcn, unwind_protect *up_frame) + void call_stack::push (octave_user_function *fcn, unwind_protect *up_frame, + stack_frame *closure_frames) { size_t prev_frame = m_curr_frame; m_curr_frame = m_cs.size (); @@ -449,7 +450,8 @@ stack_frame *slink = get_static_link (prev_frame); m_cs.push_back (new user_fcn_stack_frame (*this, fcn, up_frame, - prev_frame, slink)); + prev_frame, slink, + closure_frames)); } void call_stack::push (octave_user_script *script, unwind_protect *up_frame)
--- a/libinterp/corefcn/call-stack.h Sun Mar 03 07:08:52 2019 +0000 +++ b/libinterp/corefcn/call-stack.h Sun Mar 03 10:19:36 2019 +0000 @@ -165,7 +165,8 @@ void push (const symbol_scope& scope); - void push (octave_user_function *fcn, unwind_protect *up_frame); + void push (octave_user_function *fcn, unwind_protect *up_frame, + stack_frame *closure_frames = nullptr); void push (octave_user_script *script, unwind_protect *up_frame);
--- a/libinterp/corefcn/stack-frame.h Sun Mar 03 07:08:52 2019 +0000 +++ b/libinterp/corefcn/stack-frame.h Sun Mar 03 10:19:36 2019 +0000 @@ -309,6 +309,12 @@ stack_frame * access_link (void) const {return m_access_link; } + void set_closure_links (stack_frame *dup_frame) + { + m_static_link = dup_frame; + m_access_link = dup_frame; + } + virtual size_t size (void) const; virtual void resize (size_t); @@ -888,9 +894,12 @@ user_fcn_stack_frame (call_stack& cs, octave_user_function *fcn, unwind_protect *up_frame, size_t prev, - stack_frame *static_link) + stack_frame *static_link, + stack_frame *access_link = nullptr) : base_value_stack_frame (cs, get_num_symbols (fcn), prev, static_link, - get_access_link (fcn, static_link)), + (access_link + ? access_link + : get_access_link (fcn, static_link))), m_fcn (fcn), m_unwind_protect_frame (up_frame) { }
--- a/libinterp/corefcn/symscope.h Sun Mar 03 07:08:52 2019 +0000 +++ b/libinterp/corefcn/symscope.h Sun Mar 03 10:19:36 2019 +0000 @@ -96,6 +96,8 @@ void set_nesting_depth (size_t depth) { m_nesting_depth = depth; } + bool is_parent (void) const { return ! m_children.empty (); } + bool is_static (void) const { return m_is_static; } void mark_static (void) { m_is_static = true; } @@ -366,6 +368,11 @@ return m_rep ? m_rep->is_nested () : false; } + bool is_parent (void) const + { + return m_rep ? m_rep->is_parent () : false; + } + void set_nesting_depth (size_t depth) { if (m_rep)
--- a/libinterp/octave-value/ov-fcn-handle.cc Sun Mar 03 07:08:52 2019 +0000 +++ b/libinterp/octave-value/ov-fcn-handle.cc Sun Mar 03 10:19:36 2019 +0000 @@ -30,6 +30,7 @@ #include <list> #include <ostream> #include <sstream> +#include <string> #include <vector> #include "file-ops.h" @@ -62,6 +63,7 @@ #include "pt-misc.h" #include "pt-pr-code.h" #include "pt-stmt.h" +#include "syminfo.h" #include "symscope.h" #include "unwind-prot.h" #include "variables.h" @@ -82,7 +84,8 @@ octave_fcn_handle::octave_fcn_handle (const octave_value& f, const std::string& n) - : fcn (f), nm (n), has_overloads (false) + : fcn (f), nm (n), has_overloads (false), overloads (), + m_is_nested (false), m_closure_frames (nullptr) { octave_user_function *uf = fcn.user_function_value (true); @@ -95,7 +98,24 @@ } if (uf && uf->is_nested_function () && ! uf->is_subfunction ()) - error ("handles to nested functions are not yet supported"); + m_is_nested = true; +} + +octave_fcn_handle::~octave_fcn_handle (void) +{ + if (m_closure_frames) + { + while (m_closure_frames->size () > 0) + { + octave::stack_frame *elt = m_closure_frames->back (); + + delete elt; + + m_closure_frames->pop_back (); + } + + delete m_closure_frames; + } } octave_value_list @@ -140,15 +160,13 @@ octave_value_list octave_fcn_handle::call (int nargout, const octave_value_list& args) { - octave_value_list retval; + octave::out_of_date_check (fcn, "", false); - octave::out_of_date_check (fcn, "", false); + // Possibly overloaded function. + octave_value fcn_to_call = fcn; if (has_overloads) { - // Possibly overloaded function. - octave_value ov_fcn; - // Compute dispatch type. builtin_type_t btyp; std::string dispatch_type = octave::get_dispatch_type (args, btyp); @@ -157,7 +175,7 @@ if (btyp != btyp_unknown) { octave::out_of_date_check (builtin_overloads[btyp], dispatch_type, false); - ov_fcn = builtin_overloads[btyp]; + fcn_to_call = builtin_overloads[btyp]; } else { @@ -188,7 +206,7 @@ set_overload (pname, ftmp); octave::out_of_date_check (ftmp, pname, false); - ov_fcn = ftmp; + fcn_to_call = ftmp; break; } @@ -199,28 +217,28 @@ else { octave::out_of_date_check (it->second, dispatch_type, false); - ov_fcn = it->second; + fcn_to_call = it->second; } } - if (ov_fcn.is_defined ()) - retval = octave::feval (ov_fcn, args, nargout); - else if (fcn.is_defined ()) - retval = octave::feval (fcn, args, nargout); - else + if (! fcn_to_call.is_defined ()) error ("%s: no method for class %s", nm.c_str (), dispatch_type.c_str ()); } - else - { - // Non-overloaded function (anonymous, subfunction, private function). - if (fcn.is_defined ()) - retval = octave::feval (fcn, args, nargout); - else - error ("%s: no longer valid function handle", nm.c_str ()); - } + else if (! fcn_to_call.is_defined ()) + error ("%s: no longer valid function handle", nm.c_str ()); + + octave::stack_frame *closure_context = nullptr; - return retval; + if (m_closure_frames && m_closure_frames->size () > 0) + closure_context = m_closure_frames->front (); + + octave::tree_evaluator& tw + = octave::__get_evaluator__ ("octave_fcn_handle::call"); + + octave_function *of = fcn_to_call.function_value (); + + return of->call (tw, nargout, args, closure_context); } dim_vector @@ -230,6 +248,83 @@ return dv; } +// Save call stack frames for handles to nested functions. + +void +octave_fcn_handle::push_closure_context (octave::tree_evaluator& tw) +{ + if (! m_closure_frames) + m_closure_frames = new std::list<octave::stack_frame *> (); + + octave::call_stack& main_cs = tw.get_call_stack (); + + octave::stack_frame& curr_frame = main_cs.get_current_stack_frame (); + + octave::stack_frame *dup_frame = curr_frame.dup (); + + if (! m_closure_frames->empty ()) + { + octave::stack_frame *top_frame = m_closure_frames->back (); + + // Arrange for static and access links in the top stack frame (the + // last one saved before this one) to point to the new duplicated + // frame. This way we will look up through the duplicated frames + // when evaluating the function. + + top_frame->set_closure_links (dup_frame); + } + + m_closure_frames->push_back (dup_frame); +} + +octave_value +octave_fcn_handle::workspace (void) const +{ + if (nm == anonymous) + { + octave_user_function *fu = fcn.user_function_value (); + + octave_scalar_map ws; + + if (fu) + { + for (const auto& nm_val : fu->local_var_init_vals ()) + ws.assign (nm_val.first, nm_val.second); + } + + return ws; + } + else if (m_closure_frames) + { + octave_idx_type num_frames = m_closure_frames->size (); + + Cell ws_frames (num_frames, 1); + + octave_idx_type i = 0; + + for (auto elt : *m_closure_frames) + { + octave::symbol_info_list symbols = elt->all_variables (); + + octave_scalar_map ws; + + for (auto sym_name : symbols.names ()) + { + octave_value val = symbols.varval (sym_name); + + if (val.is_defined ()) + ws.assign (sym_name, val); + } + + ws_frames(i++) = ws; + } + + return ws_frames; + } + + return Cell (); +} + bool octave_fcn_handle::is_equal_to (const octave_fcn_handle& h) const { @@ -1704,6 +1799,9 @@ The function is a subfunction within an m-file. @end table +@item nested +The function is nested. + @item file The m-file that will be called to perform the function. This field is empty for anonymous and built-in functions. @@ -1755,6 +1853,8 @@ } else if (fcn->is_private_function ()) m.setfield ("type", "private"); + else if (fh->is_nested ()) + m.setfield ("type", "nested"); else if (fh->is_overloaded ()) m.setfield ("type", "overloaded"); else @@ -1767,18 +1867,14 @@ { m.setfield ("file", nm); - octave_user_function *fu = fh->user_function_value (); - - octave_scalar_map ws; - for (const auto& nm_val : fu->local_var_init_vals ()) - ws.assign (nm_val.first, nm_val.second); - - m.setfield ("workspace", ws); + m.setfield ("workspace", fh->workspace ()); } else if (fcn->is_user_function () || fcn->is_user_script ()) { octave_function *fu = fh->function_value (); m.setfield ("file", fu->fcn_file_name ()); + + m.setfield ("workspace", fh->workspace ()); } else m.setfield ("file", "");
--- a/libinterp/octave-value/ov-fcn-handle.h Sun Mar 03 07:08:52 2019 +0000 +++ b/libinterp/octave-value/ov-fcn-handle.h Sun Mar 03 10:19:36 2019 +0000 @@ -27,6 +27,7 @@ #include "octave-config.h" #include <iosfwd> +#include <list> #include <string> #include "ov-base.h" @@ -36,6 +37,8 @@ namespace octave { class interpreter; + class stack_frame; + class tree_evaluator; } // Function handles. @@ -53,16 +56,21 @@ static const std::string anonymous; octave_fcn_handle (void) - : fcn (), nm (), has_overloads (false), overloads () { } + : fcn (), nm (), has_overloads (false), overloads (), + m_is_nested (false), m_closure_frames (nullptr) + { } octave_fcn_handle (const std::string& n) - : fcn (), nm (n), has_overloads (false), overloads () { } + : fcn (), nm (n), has_overloads (false), overloads (), + m_is_nested (false), m_closure_frames (nullptr) + { } octave_fcn_handle (const octave_value& f, const std::string& n = anonymous); octave_fcn_handle (const octave_fcn_handle& fh) : octave_base_value (fh), fcn (fh.fcn), nm (fh.nm), - has_overloads (fh.has_overloads), overloads () + has_overloads (fh.has_overloads), overloads (), + m_is_nested (fh.m_is_nested), m_closure_frames (fh.m_closure_frames) { for (int i = 0; i < btyp_num_types; i++) builtin_overloads[i] = fh.builtin_overloads[i]; @@ -70,7 +78,7 @@ overloads = fh.overloads; } - ~octave_fcn_handle (void) = default; + ~octave_fcn_handle (void); octave_base_value * clone (void) const { return new octave_fcn_handle (*this); } @@ -96,13 +104,19 @@ bool is_overloaded (void) const { return has_overloads; } + bool is_nested (void) const { return m_is_nested; } + dim_vector dims (void) const; octave_function * function_value (bool = false) - { return fcn.function_value (); } + { + return fcn.function_value (); + } octave_user_function * user_function_value (bool = false) - { return fcn.user_function_value (); } + { + return fcn.user_function_value (); + } octave_fcn_handle * fcn_handle_value (bool = false) { return this; } @@ -110,6 +124,10 @@ std::string fcn_name (void) const { return nm; } + void push_closure_context (octave::tree_evaluator& tw); + + octave_value workspace (void) const; + void set_overload (builtin_type_t btyp, const octave_value& ov_fcn) { if (btyp != btyp_unknown) @@ -174,6 +192,14 @@ // Overloads for other classes. str_ov_map overloads; + // TRUE means this is a handle to a nested function. + bool m_is_nested; + + // Saved stack frames for handles to nested functions. This allows us + // to access non-locals and other context info when calling nested + // functions indirectly through handles. + std::list<octave::stack_frame *> *m_closure_frames; + bool parse_anon_fcn_handle (const std::string& fcn_text); virtual octave_value_list call (int nargout, const octave_value_list& args);
--- a/libinterp/octave-value/ov-fcn.cc Sun Mar 03 07:08:52 2019 +0000 +++ b/libinterp/octave-value/ov-fcn.cc Sun Mar 03 10:19:36 2019 +0000 @@ -33,12 +33,21 @@ octave_function::clone (void) const { panic_impossible (); - return nullptr; } octave_base_value * octave_function::empty_clone (void) const { panic_impossible (); - return nullptr; } + +octave_value_list +octave_function::call (octave::tree_evaluator& tw, int nargout, + const octave_value_list& args, + octave::stack_frame *closure_context) +{ + if (closure_context) + panic_impossible (); + + return call (tw, nargout, args); +}
--- a/libinterp/octave-value/ov-fcn.h Sun Mar 03 07:08:52 2019 +0000 +++ b/libinterp/octave-value/ov-fcn.h Sun Mar 03 10:19:36 2019 +0000 @@ -37,6 +37,7 @@ namespace octave { + class stack_frame; class tree_evaluator; class tree_walker; } @@ -220,7 +221,12 @@ call (octave::tree_evaluator& tw, int nargout = 0, const octave_value_list& args = octave_value_list ()) = 0; -protected: + virtual octave_value_list + call (octave::tree_evaluator& tw, int nargout, + const octave_value_list& args, + octave::stack_frame *closure_context); + + protected: octave_function (const std::string& nm, const std::string& ds = "")
--- a/libinterp/octave-value/ov-usr-fcn.cc Sun Mar 03 07:08:52 2019 +0000 +++ b/libinterp/octave-value/ov-usr-fcn.cc Sun Mar 03 10:19:36 2019 +0000 @@ -451,9 +451,10 @@ octave_value_list octave_user_function::call (octave::tree_evaluator& tw, int nargout, - const octave_value_list& args) + const octave_value_list& args, + octave::stack_frame *closure_frames) { - return tw.execute_user_function (*this, nargout, args); + return tw.execute_user_function (*this, nargout, args, closure_frames); } void
--- a/libinterp/octave-value/ov-usr-fcn.h Sun Mar 03 07:08:52 2019 +0000 +++ b/libinterp/octave-value/ov-usr-fcn.h Sun Mar 03 10:19:36 2019 +0000 @@ -327,6 +327,8 @@ bool is_nested_function (void) const { return nested_function; } + bool is_parent_function (void) const { return m_scope.is_parent (); } + void mark_as_legacy_constructor (void) { class_constructor = legacy; } bool is_legacy_constructor (const std::string& cname = "") const @@ -361,7 +363,14 @@ octave_value_list call (octave::tree_evaluator& tw, int nargout = 0, - const octave_value_list& args = octave_value_list ()); + const octave_value_list& args = octave_value_list ()) + { + return call (tw, nargout, args, nullptr); + } + + octave_value_list + call (octave::tree_evaluator& tw, int nargout, + const octave_value_list& args, octave::stack_frame *); octave::tree_parameter_list * parameter_list (void) { return param_list; } @@ -446,9 +455,12 @@ // TRUE means this is an anonymous function. bool anonymous_function; - // TRUE means this is a nested function. (either a child or parent) + // TRUE means this is a nested function. bool nested_function; + // TRUE means this function contains a nested function. + bool parent_function; + // Enum describing whether this function is the constructor for class object. class_method_type class_constructor;
--- a/libinterp/parse-tree/pt-eval.cc Sun Mar 03 07:08:52 2019 +0000 +++ b/libinterp/parse-tree/pt-eval.cc Sun Mar 03 10:19:36 2019 +0000 @@ -1763,7 +1763,8 @@ octave_value_list tree_evaluator::execute_user_function (octave_user_function& user_function, int nargout, - const octave_value_list& xargs) + const octave_value_list& xargs, + stack_frame *closure_frames) { octave_value_list retval; @@ -1803,7 +1804,10 @@ // Save old and set current symbol table context, for // eval_undefined_error(). - m_call_stack.push (&user_function, &frame); + // std::cerr << "eval: " << user_function.name () + // << " with closure_frames: " << closure_frames << std::endl; + + m_call_stack.push (&user_function, &frame, closure_frames); frame.protect_var (Vtrack_line_num); // update source line numbers, even if debugging @@ -1913,6 +1917,28 @@ varargout); } + if (user_function.is_nested_function () + || user_function.is_parent_function ()) + { + // Copy current stack frame to handles to nested functions. + + for (octave_idx_type i = 0; i < retval.length (); i++) + { + octave_value val = retval(i); + + if (val.is_function_handle ()) + { + octave_fcn_handle *fh = val.fcn_handle_value (); + + if (fh && fh->is_nested ()) + { + // std::cerr << "pushing closure context" << std::endl; + fh->push_closure_context (*this); + } + } + } + } + return retval; }
--- a/libinterp/parse-tree/pt-eval.h Sun Mar 03 07:08:52 2019 +0000 +++ b/libinterp/parse-tree/pt-eval.h Sun Mar 03 10:19:36 2019 +0000 @@ -200,8 +200,9 @@ void visit_octave_user_function (octave_user_function&); octave_value_list - execute_user_function (octave_user_function& user_function, int nargout, - const octave_value_list& args); + execute_user_function (octave_user_function& user_function, + int nargout, const octave_value_list& args, + stack_frame *closure_frames = nullptr); void visit_octave_user_function_header (octave_user_function&);
--- a/test/nest/nest.tst Sun Mar 03 07:08:52 2019 +0000 +++ b/test/nest/nest.tst Sun Mar 03 10:19:36 2019 +0000 @@ -70,3 +70,69 @@ %!error <D' undefined near line 7> scope2 %!error <can not add variable "y" to a static workspace> nest_eval ("y = 5;", "") %!error <can not add variable "y" to a static workspace> nest_eval ("y;", "") + +## Test the way that non-local variables referenced by nested functions +## work with function handles. + +## FH1 and FH2 were created separately so will have distinct +## closure contexts.handles, FH3 is a copy of FH2 so they will +## share the same context. + +%!test <39257> +%! fh1 = nst1 (13); +%! fh2 = nst1 (13); +%! fh3 = fh2; +%! +%! assert (fh1 (), 13); +%! assert (fh2 (), 13); +%! assert (fh3 (), 13); +%! +%! assert (fh1 (42), 42); +%! assert (fh2 (), 13); +%! assert (fh3 (), 13); +%! +%! assert (fh2 (pi), pi); +%! assert (fh1 (), 42); +%! assert (fh3 (), pi); + +## Similar to the test above, but with persistent variables. These are +## stored in the function, not the closure context, so are shared among +## all handles whether they are created separately or copied. + +%!test +%! fh1 = nst2 (13); +%! fh2 = nst2 (13); +%! fh3 = fh2; +%! +%! assert (fh1 (), 13); +%! assert (fh2 (), 13); +%! assert (fh3 (), 13); +%! +%! assert (fh1 (42), 42); +%! assert (fh2 (), 42); +%! assert (fh3 (), 42); +%! +%! assert (fh2 (pi), pi); +%! assert (fh1 (), pi); +%! assert (fh3 (), pi); + +## And again with global variables. + +%!test +%! fh1 = nst3 (13); +%! fh2 = nst3 (13); +%! fh3 = fh2; +%! +%! assert (fh1 (), 13); +%! assert (fh2 (), 13); +%! assert (fh3 (), 13); +%! +%! assert (fh1 (42), 42); +%! assert (fh2 (), 42); +%! assert (fh3 (), 42); +%! +%! assert (fh2 (pi), pi); +%! assert (fh1 (), pi); +%! assert (fh3 (), pi); +%! +%! clear -global g; # cleanup after tests
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/nest/nst1.m Sun Mar 03 10:19:36 2019 +0000 @@ -0,0 +1,12 @@ +function fh = nst1 (xval) + if (nargin > 0) + x = xval; + end + fh = @nst; + function r = nst (xval) + if (nargin > 0) + x = xval; + end + r = x; + end +end