Mercurial > octave
changeset 23521:551fa3879615
improve compatibility and simplify evaluation of index expressions
* pt-eval.cc (tree_evaluator::visit_index_expression): Improve
compatibility and simplify.
* pt-arg-list.cc (tree_argument_list::has_magic_end):
Don't check elements that are themselves index expressions.
author | John W. Eaton <jwe@octave.org> |
---|---|
date | Tue, 23 May 2017 12:24:44 -0400 |
parents | 9f925aed7d1b |
children | d2e300f7700c |
files | libinterp/parse-tree/pt-arg-list.cc libinterp/parse-tree/pt-eval.cc |
diffstat | 2 files changed, 186 insertions(+), 112 deletions(-) [+] |
line wrap: on
line diff
--- a/libinterp/parse-tree/pt-arg-list.cc Mon May 22 13:45:19 2017 -0400 +++ b/libinterp/parse-tree/pt-arg-list.cc Tue May 23 12:24:44 2017 -0400 @@ -64,7 +64,7 @@ { for (const tree_expression *elt : *this) { - if (elt && elt->has_magic_end ()) + if (elt && ! elt->is_index_expression () && elt->has_magic_end ()) return true; }
--- a/libinterp/parse-tree/pt-eval.cc Mon May 22 13:45:19 2017 -0400 +++ b/libinterp/parse-tree/pt-eval.cc Tue May 23 12:24:44 2017 -0400 @@ -1084,6 +1084,77 @@ namespace octave { + // Unlike Matlab, which does not allow the result of a function call + // or array indexing expression to be further indexed, Octave attempts + // to handle arbitrary index expressions. For example, Octave allows + // expressions like + // + // svd (rand (10))(1:5) + // + // Although octave_value objects may contain function objects, no + // indexing operation or function call is supposed to return them + // directly. Instead, the language is supposed to only allow function + // objects to be stored as function handles (named or anonymous) or as + // inline functions. The only place a function object should appear + // directly is if the symbol stored in a tree_identifier object + // resolves to a function. This means that the only place we need to + // look for functions is in the first element of the index + // expression. + // + // Steps: + // + // * Obtain the initial value from the expression component of the + // tree_index_expression object. If it is a tree_identifier object + // indexed by '(args)' and the identifier is not a variable, then + // peform a function call. Use the (optional) arguments to perform + // the function lookup so we choose the correct function or class + // method to call. Otherwise, evaluate the first expression + // without any additional arguments. + // + // * Iterate over the remaining elements of the index expression and + // call the octave_value::subsref method. If indexing a class or + // classdef object, build up a list of indices for a call to the + // subsref method for the object. Otherwise, use the result of + // each temporary evaluation for the next index element. + // + // * If not indexing a class or classdef object and any partial + // expression evaluation produces a class or classdef object, then + // build up a complete argument list from that point on for a final + // subsref call for that object. + // + // Multiple partial evaluations may be required. For example, + // given a class or classdef object X, then for the expression + // + // x.a{end}(2:end).b + // + // we must evaluate x.a to obtain the size for the first {end} + // expression, then we must evaluate x.a{end} to obtain the size + // for the second (2:end) expression. Finally, the complete + // expression may be evaluated. + // + // If X is a cell array in the above expression, and none of the + // intermediate evaluations produces a class or classdef object, + // then the evaluation is performed as the following series of + // steps + // + // tmp = x.a + // tmp = tmp{end} + // tmp = tmp(2:end) + // result = tmp.b + // + // If any of the partial evaluations produces a class or classdef + // object, then the subsref method for that object is called as + // described above. For example, suppose x.a produces a classdef + // object. Then the evaluation is performed as the following + // series of steps + // + // base_expr = tmp = x.a + // tmp = base_expr{end} + // base_expr{end}(2:end).b + // + // In the last two steps, the partial value computed in the + // previous step is used to determine the value of END. + void tree_evaluator::visit_index_expression (tree_index_expression& idx_expr) { @@ -1091,32 +1162,38 @@ int nargout = m_nargout_stack.top (); - octave_value first_expr_val; - - octave_value_list first_args; - - bool have_args = false; - - tree_expression *expr = idx_expr.expression (); + std::string type = idx_expr.type_tags (); std::list<tree_argument_list *> args = idx_expr.arg_lists (); - std::string type = idx_expr.type_tags (); std::list<string_vector> arg_nm = idx_expr.arg_names (); std::list<tree_expression *> dyn_field = idx_expr.dyn_fields (); - if (expr->is_identifier () && type[0] == '(') + assert (! args.empty ()); + + std::list<tree_argument_list *>::iterator p_args = args.begin (); + std::list<string_vector>::iterator p_arg_nm = arg_nm.begin (); + std::list<tree_expression *>::iterator p_dyn_field = dyn_field.begin (); + + int n = args.size (); + int beg = 0; + + octave_value base_expr_val; + + tree_expression *expr = idx_expr.expression (); + + if (expr->is_identifier () && type[beg] == '(') { tree_identifier *id = dynamic_cast<tree_identifier *> (expr); - if (! (id->is_variable () || args.empty ())) + if (! id->is_variable ()) { - tree_argument_list *al = *(args.begin ()); - - size_t n = (al ? al->length () : 0); - - if (n > 0) + octave_value_list first_args; + + tree_argument_list *al = *p_args; + + if (al && al->length () > 0) { - // Function calls inside an argument list can't have ignored - // output arguments. + // Function calls inside an argument list can't have + // ignored output arguments. unwind_protect frame; @@ -1125,97 +1202,107 @@ frame.add_method (m_lvalue_list_stack, &value_stack<const std::list<octave_lvalue>*>::pop); - string_vector anm = *(arg_nm.begin ()); - have_args = true; - first_args = al -> convert_to_const_vector (this); + string_vector anm = *p_arg_nm; + first_args = al->convert_to_const_vector (this); first_args.stash_name_tags (anm); - - first_expr_val = id->do_lookup (first_args); + } + + octave_function *fcn = nullptr; + + octave_value val = id->do_lookup (first_args); + + if (val.is_function ()) + fcn = val.function_value (true); + + if (fcn) + { + try + { + retval = fcn->call (nargout, first_args); + } + catch (octave::index_exception& e) + { + final_index_error (e, expr); + } + + beg++; + p_args++; + p_arg_nm++; + p_dyn_field++; + + if (n > beg) + { + // More indices to follow. Silently ignore + // extra output values. + + if (retval.length () == 0) + error ("indexing undefined value"); + else + base_expr_val = retval(0); + } + else + { + // No more indices, so we are done. + + m_value_stack.push (retval); + return; + } } } } - if (first_expr_val.is_undefined ()) - first_expr_val = evaluate (expr); - - octave_value tmp = first_expr_val; - octave_idx_type tmpi = 0; + if (base_expr_val.is_undefined ()) + base_expr_val = evaluate (expr); + + bool indexing_object = base_expr_val.is_object (); std::list<octave_value_list> idx; - int n = args.size (); - - std::list<tree_argument_list *>::iterator p_args = args.begin (); - std::list<string_vector>::iterator p_arg_nm = arg_nm.begin (); - std::list<tree_expression *>::iterator p_dyn_field = dyn_field.begin (); - - for (int i = 0; i < n; i++) + octave_value partial_expr_val = base_expr_val; + + for (int i = beg; i < n; i++) { - if (i > 0) + if (i > beg) { tree_argument_list *al = *p_args; - // In Matlab, () can only be followed by '.'. In Octave, we - // do not enforce this for rvalue expressions, but we'll - // split the evaluation at this point. This will, - // hopefully, allow Octave's looser rules apply smoothly for - // Matlab overloaded subsref codes. - - // We might have an expression like - // - // x{end}.a(end) - // - // and we are looking at the argument list that contains the - // second (or third, etc.) "end" token, so we must evaluate - // everything up to the point of that argument list so we - // can pass the appropriate value to the built-in end - // function. - - // An expression like - // - // s.a (f (1:end)) - // - // can mean a lot of different things depending on the types - // of s, a, and f. Let's just say it's complicated and that - // the following code is definitely not correct in all - // cases. That it is already so complex makes me think that - // there must be a better way. - - bool split = ((type[i-1] == '(' && type[i] != '.') - || (al && al->has_magic_end () - && ! tmp.is_classdef_object ())); - - if (split) + if (! indexing_object || (al && al->has_magic_end ())) { + // Evaluate what we have so far to find the value to + // pass to the END function. + try { + // Silently ignore extra output values. + octave_value_list tmp_list - =tmp.subsref (type.substr (tmpi, i-tmpi), idx, nargout); - - tmp = (tmp_list.length () ? tmp_list(0) : octave_value ()); - tmpi = i; - idx.clear (); - - if (tmp.is_cs_list ()) - err_indexed_cs_list (); - - if (tmp.is_function ()) + = base_expr_val.subsref (type.substr (beg, i-beg), idx, nargout); + + partial_expr_val = tmp_list.length () ? tmp_list(0) : octave_value (); + + if (! indexing_object) { - octave_function *fcn = tmp.function_value (true); - - if (fcn && ! fcn->accepts_postfix_index (type[i])) + base_expr_val = partial_expr_val; + + if (partial_expr_val.is_cs_list ()) + err_indexed_cs_list (); + + retval = partial_expr_val; + + beg = i; + idx.clear (); + + if (partial_expr_val.is_object ()) { - tmp_list = fcn->call (1); - - tmp = (tmp_list.length () - ? tmp_list(0) : octave_value ()); - - if (tmp.is_cs_list ()) - err_indexed_cs_list (); + // Found an object, so now we'll build up + // complete index list for one big subsref + // call from this point on. + + indexing_object = true; } } } - catch (octave::index_exception& e) // problems with index range, type etc. + catch (octave::index_exception& e) { final_index_error (e, expr); } @@ -1225,17 +1312,11 @@ switch (type[i]) { case '(': - if (have_args) - { - idx.push_back (first_args); - have_args = false; - } - else - idx.push_back (make_value_list (*p_args, *p_arg_nm, &tmp)); + idx.push_back (make_value_list (*p_args, *p_arg_nm, &partial_expr_val)); break; case '{': - idx.push_back (make_value_list (*p_args, *p_arg_nm, &tmp)); + idx.push_back (make_value_list (*p_args, *p_arg_nm, &partial_expr_val)); break; case '.': @@ -1252,23 +1333,16 @@ p_dyn_field++; } - try - { - retval = tmp.subsref (type.substr (tmpi, n - tmpi), idx, nargout); - } - catch (octave::index_exception& e) // range problems, bad index type, etc. + if (! idx.empty ()) { - final_index_error (e, expr); - } - - octave_value val = (retval.length () ? retval(0) : octave_value ()); - - if (val.is_function ()) - { - octave_function *fcn = val.function_value (true); - - if (fcn) - retval = fcn->call (nargout); + try + { + retval = base_expr_val.subsref (type.substr (beg, n-beg), idx, nargout); + } + catch (octave::index_exception& e) // range problems, bad index type, etc. + { + final_index_error (e, expr); + } } m_value_stack.push (retval);