Mercurial > jwe > octave
changeset 26850:8bd9fd99c12a
lazily evaluate fcn handles; fix overload resolution (bug #29447, bug #31821, bug #48802)
* ov-fcn-handle.h, ov-fcn-handle.cc (octave_fcn_handle::has_overloads,
octave_fcn_handle::builtin_overloads, octave_fcn_handle::overloads):
Delete member variables and all uses.
(octave_fcn_handle::set_overload, octave_fcn_handle::set_overload,
octave_fcn_handle::builtin_type, octave_fcn_handle::is_overloaded):
Delete.
(octave_fcn_handle::m_scope): New member variable. Store scop where
handle is created along with function name to allow proper lookup and
overload resolution.
(octave_fcn_handle::m_generic_fcn): New member variable.
(octave_fcn_handle::function_value, octave_fcn_handle::user_function_value):
Store octave_value object containing discovered function in
m_generic_fcn so we can safely return a pointer to the underlying
function object.
(make_fcn_handle, octave_fcn_handle::subsref): Greatly simplify.
* pt-eval.cc (tree_evaluator::execute_user_function): Push closure
context for any handle created inside a nested function or parent of a
nested function.
* cellfun.cc (Fcellfun): Eliminate special checks for "overloaded"
function handles.
* graphics.cc (gh_manager::do_execute_callback): If callback is a
function handle, pass it directly to feval instead of extracting
function value from it first.
* cdef-class.cc (make_fcn_handle): Accept interpreter as argument.
Change all uses. Create octave_fcn_handle object with current scope.
author | John W. Eaton <jwe@octave.org> |
---|---|
date | Tue, 05 Mar 2019 22:30:09 +0000 |
parents | 4ff25d9b1eec |
children | 603f5d6ada56 |
files | libinterp/corefcn/cellfun.cc libinterp/corefcn/graphics.cc libinterp/octave-value/cdef-class.cc libinterp/octave-value/ov-fcn-handle.cc libinterp/octave-value/ov-fcn-handle.h libinterp/parse-tree/pt-eval.cc |
diffstat | 6 files changed, 142 insertions(+), 221 deletions(-) [+] |
line wrap: on
line diff
--- a/libinterp/corefcn/cellfun.cc Tue Mar 05 07:32:38 2019 +0000 +++ b/libinterp/corefcn/cellfun.cc Tue Mar 05 22:30:09 2019 +0000 @@ -461,6 +461,11 @@ { if (func.is_function_handle ()) { + // We can't check for overloads now. Is there something else we + // should be doing instead? + goto nevermind; + +#if 0 octave_fcn_handle *f = func.fcn_handle_value (); // Overloaded function handles need to check the type of the @@ -468,6 +473,7 @@ // optimized this way. if (f -> is_overloaded ()) goto nevermind; +#endif } std::string name = func.function_value () -> name (); @@ -1176,6 +1182,11 @@ { if (func.is_function_handle ()) { + // We can't check for overloads now. Is there something + // else we should be doing instead? + goto nevermind; + +#if 0 octave_fcn_handle *f = func.fcn_handle_value (); // Overloaded function handles need to check the type of the @@ -1183,7 +1194,9 @@ // optimized this way. if (f -> is_overloaded ()) goto nevermind; +#endif } + octave_value f = symtab.find_function (func.function_value () -> name ());
--- a/libinterp/corefcn/graphics.cc Tue Mar 05 07:32:38 2019 +0000 +++ b/libinterp/corefcn/graphics.cc Tue Mar 05 22:30:09 2019 +0000 @@ -11616,6 +11616,7 @@ if (cb_arg.is_defined () && ! cb_arg.isempty ()) { octave_value_list args; + octave_value ov_fcn; octave_function *fcn = nullptr; args(0) = h.as_octave_value (); @@ -11640,8 +11641,10 @@ // Copy CB because "function_value" method is non-const. octave_value cb = cb_arg; - if (cb.is_function () || cb.is_function_handle ()) + if (cb.is_function ()) fcn = cb.function_value (); + else if (cb.is_function_handle ()) + ov_fcn = cb; else if (cb.is_string ()) { int status; @@ -11669,7 +11672,7 @@ { Cell c = cb.cell_value (); - fcn = c(0).function_value (); + ov_fcn = c(0); for (int i = 1; i < c.numel () ; i++) args(1+i) = c(i); @@ -11681,10 +11684,13 @@ nm.c_str ()); } - if (fcn) + if (fcn || ov_fcn.is_defined ()) try { - octave::feval (fcn, args); + if (ov_fcn.is_defined ()) + octave::feval (ov_fcn, args); + else + octave::feval (fcn, args); } catch (octave::execution_exception&) {
--- a/libinterp/octave-value/cdef-class.cc Tue Mar 05 07:32:38 2019 +0000 +++ b/libinterp/octave-value/cdef-class.cc Tue Mar 05 22:30:09 2019 +0000 @@ -56,12 +56,19 @@ namespace octave { static octave_value - make_fcn_handle (const octave_value& fcn, const std::string& nm) + make_fcn_handle (interpreter& interp, const octave_value& fcn, + const std::string& nm) { octave_value retval; if (fcn.is_defined ()) - retval = octave_value (new octave_fcn_handle (fcn, nm)); + { + tree_evaluator& tw = interp.get_evaluator (); + + symbol_scope curr_scope = tw.get_current_scope (); + + retval = octave_value (new octave_fcn_handle (curr_scope, fcn, nm)); + } return retval; } @@ -946,10 +953,12 @@ if (mprefix == "get.") get_methods[mname.substr (4)] = - make_fcn_handle (mtd, full_class_name + '>' + mname); + make_fcn_handle (interp, mtd, + full_class_name + '>' + mname); else if (mprefix == "set.") set_methods[mname.substr (4)] = - make_fcn_handle (mtd, full_class_name + '>' + mname); + make_fcn_handle (interp, mtd, + full_class_name + '>' + mname); else { cdef_method meth = cdm.make_method (retval, mname, mtd);
--- a/libinterp/octave-value/ov-fcn-handle.cc Tue Mar 05 07:32:38 2019 +0000 +++ b/libinterp/octave-value/ov-fcn-handle.cc Tue Mar 05 22:30:09 2019 +0000 @@ -82,10 +82,30 @@ const std::string octave_fcn_handle::anonymous ("@<anonymous>"); +octave_fcn_handle::octave_fcn_handle (const octave::symbol_scope& scope, + const octave_value& f, + const std::string& n) + : fcn (f), nm (n), m_scope (scope), m_is_nested (false), + m_closure_frames (nullptr) +{ + octave_user_function *uf = fcn.user_function_value (true); + + if (uf && nm != anonymous) + { + octave::symbol_scope uf_scope = uf->scope (); + + if (uf_scope) + uf_scope.cache_name (nm); + } + + if (uf && uf->is_nested_function () && ! uf->is_subfunction ()) + m_is_nested = true; +} + octave_fcn_handle::octave_fcn_handle (const octave_value& f, const std::string& n) - : fcn (f), nm (n), has_overloads (false), overloads (), - m_is_nested (false), m_closure_frames (nullptr) + : fcn (f), nm (n), m_scope (), m_is_nested (false), + m_closure_frames (nullptr) { octave_user_function *uf = fcn.user_function_value (true); @@ -160,72 +180,17 @@ octave_value_list octave_fcn_handle::call (int nargout, const octave_value_list& args) { - octave::out_of_date_check (fcn, "", false); - - // Possibly overloaded function. octave_value fcn_to_call = fcn; - if (has_overloads) + if (! fcn_to_call.is_defined ()) { - // Compute dispatch type. - builtin_type_t btyp; - std::string dispatch_type = octave::get_dispatch_type (args, btyp); - - // Retrieve overload. - if (btyp != btyp_unknown) - { - octave::out_of_date_check (builtin_overloads[btyp], dispatch_type, false); - fcn_to_call = builtin_overloads[btyp]; - } - else - { - auto it = overloads.find (dispatch_type); - - if (it == overloads.end ()) - { - // Try parent classes too. - - octave::symbol_table& symtab - = octave::__get_symbol_table__ ("octave_fcn_handle::call"); - - const std::list<std::string> plist - = symtab.parent_classes (dispatch_type); - - auto pit = plist.begin (); + octave::symbol_table& symtab + = octave::__get_symbol_table__ ("octave_fcn_handle::call"); - while (pit != plist.end ()) - { - std::string pname = *pit; - - std::string fnm = fcn_name (); - - octave_value ftmp = symtab.find_method (fnm, pname); - - if (ftmp.is_defined ()) - { - set_overload (pname, ftmp); - - octave::out_of_date_check (ftmp, pname, false); - fcn_to_call = ftmp; + fcn_to_call = symtab.find_function (nm, args, m_scope); + } - break; - } - - pit++; - } - } - else - { - octave::out_of_date_check (it->second, dispatch_type, false); - fcn_to_call = it->second; - } - } - - if (! fcn_to_call.is_defined ()) - error ("%s: no method for class %s", - nm.c_str (), dispatch_type.c_str ()); - } - else if (! fcn_to_call.is_defined ()) + if (! fcn_to_call.is_defined ()) error ("%s: no longer valid function handle", nm.c_str ()); octave::stack_frame *closure_context = nullptr; @@ -248,6 +213,40 @@ return dv; } +octave_function * octave_fcn_handle::function_value (bool) +{ + if (fcn.is_defined ()) + return fcn.function_value (); + + octave::symbol_table& symtab + = octave::__get_symbol_table__ ("octave_fcn_handle::set_fcn"); + + // Cache this value so that the pointer will be valid as long as the + // function handle object is valid. + + m_generic_fcn = symtab.find_function (nm, octave_value_list (), m_scope); + + return (m_generic_fcn.is_defined () + ? m_generic_fcn.function_value () : nullptr); +} + +octave_user_function * octave_fcn_handle::user_function_value (bool) +{ + if (fcn.is_defined ()) + return fcn.user_function_value (); + + octave::symbol_table& symtab + = octave::__get_symbol_table__ ("octave_fcn_handle::set_fcn"); + + // Cache this value so that the pointer will be valid as long as the + // function handle object is valid. + + m_generic_fcn = symtab.find_user_function (nm); + + return (m_generic_fcn.is_defined () + ? m_generic_fcn.user_function_value () : nullptr); +} + // Save call stack frames for handles to nested functions. void @@ -328,22 +327,12 @@ bool octave_fcn_handle::is_equal_to (const octave_fcn_handle& h) const { - bool retval = fcn.is_copy_of (h.fcn) && (has_overloads == h.has_overloads); - retval = retval && (overloads.size () == h.overloads.size ()); - - if (retval && has_overloads) - { - for (int i = 0; i < btyp_num_types && retval; i++) - retval = builtin_overloads[i].is_copy_of (h.builtin_overloads[i]); - - auto iter = overloads.cbegin (); - auto hiter = h.overloads.cbegin (); - for (; iter != overloads.cend () && retval; iter++, hiter++) - retval = (iter->first == hiter->first) - && (iter->second.is_copy_of (hiter->second)); - } - - return retval; + if (fcn.is_defined () && h.fcn.is_defined ()) + return fcn.is_copy_of (h.fcn); + else if (fcn.is_undefined () && h.fcn.is_undefined ()) + return nm == h.nm; + else + return false; } bool @@ -1658,75 +1647,11 @@ } } - octave::call_stack& cs = interp.get_call_stack(); - - std::string dispatch_class; - bool is_method_or_ctor_executing - = (cs.is_class_method_executing (dispatch_class) - || cs.is_class_constructor_executing (dispatch_class)); - - octave::symbol_table& symtab = interp.get_symbol_table (); - - octave_value f; - - if (is_method_or_ctor_executing) - f = symtab.find_method (tnm, dispatch_class); - - if (f.is_undefined ()) - f = symtab.find_function (tnm, octave_value_list ()); - - octave_function *fptr = f.function_value (true); - - // Here we are just looking to see if FCN is a method or constructor - // for any class. - if (fptr && (fptr->is_subfunction () || fptr->is_private_function () - || fptr->is_class_constructor () - || fptr->is_classdef_constructor ())) - { - // Locally visible function. - retval = octave_value (new octave_fcn_handle (f, tnm)); - } - else - { - octave::load_path& lp = interp.get_load_path (); + octave::tree_evaluator& tw = interp.get_evaluator (); - // Globally visible (or no match yet). Query overloads. - std::list<std::string> classes = lp.overloads (tnm); - bool any_match = fptr != nullptr || classes.size () > 0; - if (! any_match) - { - // No match found, try updating load_path and query classes again. - lp.update (); - classes = lp.overloads (tnm); - any_match = classes.size () > 0; - } - - if (! any_match) - error ("@%s: no function and no method found", tnm.c_str ()); - - octave_fcn_handle *fh = new octave_fcn_handle (f, tnm); - retval = fh; + octave::symbol_scope curr_scope = tw.get_current_scope (); - for (auto& cls : classes) - { - std::string class_name = cls; - octave_value fmeth = symtab.find_method (tnm, class_name); - - bool is_builtin = false; - for (int i = 0; i < btyp_num_types; i++) - { - // FIXME: Too slow? Maybe binary lookup? - if (class_name == btyp_class_name[i]) - { - is_builtin = true; - fh->set_overload (static_cast<builtin_type_t> (i), fmeth); - } - } - - if (! is_builtin) - fh->set_overload (class_name, fmeth); - } - } + return new octave_fcn_handle (curr_scope, tnm); return retval; } @@ -1855,8 +1780,6 @@ m.setfield ("type", "private"); else if (fh->is_nested ()) m.setfield ("type", "nested"); - else if (fh->is_overloaded ()) - m.setfield ("type", "overloaded"); else m.setfield ("type", "simple"); }
--- a/libinterp/octave-value/ov-fcn-handle.h Tue Mar 05 07:32:38 2019 +0000 +++ b/libinterp/octave-value/ov-fcn-handle.h Tue Mar 05 22:30:09 2019 +0000 @@ -33,6 +33,7 @@ #include "ov-base.h" #include "ov-fcn.h" #include "ov-typeinfo.h" +#include "symscope.h" namespace octave { @@ -56,27 +57,26 @@ static const std::string anonymous; octave_fcn_handle (void) - : 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 (), - m_is_nested (false), m_closure_frames (nullptr) + : fcn (), nm (), m_scope (), 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::symbol_scope& scope, const std::string& n) + : fcn (), nm (n), m_scope (scope), m_is_nested (false), + m_closure_frames (nullptr) + { + if (! nm.empty () && nm[0] == '@') + nm = nm.substr (1); + } - octave_fcn_handle (const octave_fcn_handle& fh) - : octave_base_value (fh), fcn (fh.fcn), nm (fh.nm), - 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]; + octave_fcn_handle (const octave::symbol_scope& scope, + const octave_value& f, + const std::string& n = anonymous); - overloads = fh.overloads; - } + octave_fcn_handle (const octave_value& f, + const std::string& n = anonymous); + + octave_fcn_handle (const octave_fcn_handle& fh) = default; ~octave_fcn_handle (void); @@ -100,23 +100,14 @@ bool is_function_handle (void) const { return true; } - builtin_type_t builtin_type (void) const { return btyp_func_handle; } - - 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 (); - } - - octave_user_function * user_function_value (bool = false) - { - return fcn.user_function_value (); - } + // FIXME: These must go away. They don't do the right thing for + // scoping or overloads. + octave_function * function_value (bool = false); + octave_user_function * user_function_value (bool = false); octave_fcn_handle * fcn_handle_value (bool = false) { return this; } @@ -128,23 +119,6 @@ octave_value workspace (void) const; - void set_overload (builtin_type_t btyp, const octave_value& ov_fcn) - { - if (btyp != btyp_unknown) - { - has_overloads = true; - builtin_overloads[btyp] = ov_fcn; - } - - } - - void set_overload (const std::string& dispatch_type, - const octave_value& ov_fcn) - { - has_overloads = true; - overloads[dispatch_type] = ov_fcn; - } - bool is_equal_to (const octave_fcn_handle&) const; octave_value convert_to_str_internal (bool pad, bool force, char type) const; @@ -177,20 +151,22 @@ protected: - // The function we are handling. + // The function we are handling (this should be valid for handles to + // anonymous functions and some other special cases). Otherwise, we + // perform dynamic lookup based on the name of the function we are + // handling and the scope where the funtion handle object was created. octave_value fcn; - // The name of the handle, including the "@". + // The function we would find without considering argument types. We + // cache this value so that the function_value and user_function_value + // methods may continue to work. + octave_value m_generic_fcn; + + // The name of the handle, not including the "@". std::string nm; - // Whether the function is overloaded at all. - bool has_overloads; - - // Overloads for builtin types. We use array to make lookup faster. - octave_value builtin_overloads[btyp_num_types]; - - // Overloads for other classes. - str_ov_map overloads; + // The scope where this object was defined. + octave::symbol_scope m_scope; // TRUE means this is a handle to a nested function. bool m_is_nested;
--- a/libinterp/parse-tree/pt-eval.cc Tue Mar 05 07:32:38 2019 +0000 +++ b/libinterp/parse-tree/pt-eval.cc Tue Mar 05 22:30:09 2019 +0000 @@ -1804,9 +1804,6 @@ // Save old and set current symbol table context, for // eval_undefined_error(). - // 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); @@ -1930,11 +1927,8 @@ { 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); - } + if (fh) + fh->push_closure_context (*this); } } }