Mercurial > octave
changeset 31394:7781b1e77406
use separate class for braindead shortcircuit evaluation
Change adapted from patch #10238 by Petter Tomner <tomner@kth.se>.
* parse.h, oct-parse.yy
(base_parser::maybe_convert_to_braindead_shortcircuit): New function.
(if_cmd_list1, elseif_clause, loop_command): Use it to convert
expressions that could be evaluated using Matlab-style short-circuit
rules to be tree_braindead_shortcircuit_binary_expression objects
instead of simple tree_binary_expression objects.
* pt-exp.h (tree_expression::mark_braindead_shortcircuit): Delete.
* pt-binop.h (tree_binary_expression::mark_braindead_shortcircuit):
Delete.
* pt-binop.h, pt-binop.cc
(tree_braindead_shortcircuit_binary_expression): New class to evaluate
Matlab-style short-circuit expressions.
* pt-binop.h, pt-binop.cc (tree_binary_expression::preserve_operands):
New function.
(tree_binary_expression::m_preserve_operands): New private data member.
(tree_binary_expression::m_eligible_for_braindead_shortcircuit):
Delete data member.
(tree_binary_expression::is_eligible_for_braindead_shortcircuit):
Delete function.
(tree_binary_expression::mark_braindead_shortcircuit): Delete function.
(tree_binary_expression::~tree_binary_expression): Don't delete m_lhs
or m_rhs if m_preserve_operands is true.
(tree_binary_expression::evaluate): Don't check for or process
Matlab-style short-circuit expressions here.
| author | John W. Eaton <jwe@octave.org> |
|---|---|
| date | Thu, 03 Nov 2022 17:50:24 -0400 |
| parents | 50f57a2be778 |
| children | 068342cc93b8 |
| files | libinterp/parse-tree/oct-parse.yy libinterp/parse-tree/parse.h libinterp/parse-tree/pt-binop.cc libinterp/parse-tree/pt-binop.h libinterp/parse-tree/pt-exp.h |
| diffstat | 5 files changed, 140 insertions(+), 73 deletions(-) [+] |
line wrap: on
line diff
--- a/libinterp/parse-tree/oct-parse.yy Thu Nov 03 14:47:18 2022 -0400 +++ b/libinterp/parse-tree/oct-parse.yy Thu Nov 03 17:50:24 2022 -0400 @@ -1100,7 +1100,7 @@ { OCTAVE_YYUSE ($3); - $1->mark_braindead_shortcircuit (); + parser.maybe_convert_to_braindead_shortcircuit ($1); $$ = parser.start_if_command ($1, $4); } @@ -1112,7 +1112,7 @@ { OCTAVE_YYUSE ($3, $6); - $4->mark_braindead_shortcircuit (); + parser.maybe_convert_to_braindead_shortcircuit ($4); $$ = parser.make_elseif_clause ($1, $4, $7, $2); } @@ -1182,7 +1182,7 @@ { OCTAVE_YYUSE ($5); - $3->mark_braindead_shortcircuit (); + parser.maybe_convert_to_braindead_shortcircuit ($3); if (! ($$ = parser.make_while_command ($1, $3, $6, $7, $2))) { @@ -3140,6 +3140,37 @@ return maybe_compound_binary_expression (op1, op2, l, c, t); } + void + base_parser::maybe_convert_to_braindead_shortcircuit (tree_expression*& expr) + { + if (expr->is_binary_expression ()) + { + tree_binary_expression *binexp + = dynamic_cast<tree_binary_expression *> (expr); + + tree_expression *lhs = binexp->lhs (); + tree_expression *rhs = binexp->rhs (); + + maybe_convert_to_braindead_shortcircuit (lhs); + maybe_convert_to_braindead_shortcircuit (rhs); + + octave_value::binary_op op_type = binexp->op_type (); + if (op_type == octave_value::op_el_and + || op_type == octave_value::op_el_or) + { + binexp->preserve_operands (); + + int line = expr->line (); + int column = expr->column (); + + delete expr; + + expr = new tree_braindead_shortcircuit_binary_expression + (lhs, rhs, line, column, op_type); + } + } + } + // Build a boolean expression. tree_expression *
--- a/libinterp/parse-tree/parse.h Thu Nov 03 14:47:18 2022 -0400 +++ b/libinterp/parse-tree/parse.h Thu Nov 03 17:50:24 2022 -0400 @@ -273,6 +273,10 @@ make_binary_op (int op, tree_expression *op1, token *tok_val, tree_expression *op2); + // Maybe convert EXPR to a braindead_shortcircuit expression. + OCTINTERP_API void + maybe_convert_to_braindead_shortcircuit (tree_expression*& expr); + // Build a boolean expression. OCTINTERP_API tree_expression * make_boolean_op (int op, tree_expression *op1, token *tok_val,
--- a/libinterp/parse-tree/pt-binop.cc Thu Nov 03 14:47:18 2022 -0400 +++ b/libinterp/parse-tree/pt-binop.cc Thu Nov 03 17:50:24 2022 -0400 @@ -69,49 +69,6 @@ octave_value tree_binary_expression::evaluate (tree_evaluator& tw, int) { - octave_value val; - - if (is_eligible_for_braindead_shortcircuit ()) - { - if (m_lhs) - { - octave_value a = m_lhs->evaluate (tw); - - if (a.ndims () == 2 && a.rows () == 1 && a.columns () == 1) - { - bool result = false; - - bool a_true = a.is_true (); - - if (a_true) - { - if (m_etype == octave_value::op_el_or) - { - matlab_style_short_circuit_warning ("|"); - return octave_value (true); - } - } - else - { - if (m_etype == octave_value::op_el_and) - { - matlab_style_short_circuit_warning ("&"); - return octave_value (false); - } - } - - if (m_rhs) - { - octave_value b = m_rhs->evaluate (tw); - - result = b.is_true (); - } - - return octave_value (result); - } - } - } - if (m_lhs) { octave_value a = m_lhs->evaluate (tw); @@ -135,12 +92,73 @@ type_info& ti = interp.get_type_info (); - val = binary_op (ti, m_etype, a, b); + return binary_op (ti, m_etype, a, b); } } } - return val; + return octave_value (); + } + + tree_expression * + tree_braindead_shortcircuit_binary_expression::dup (symbol_scope& scope) const + { + tree_braindead_shortcircuit_binary_expression *new_be + = new tree_braindead_shortcircuit_binary_expression + (m_lhs ? m_lhs->dup (scope) : nullptr, + m_rhs ? m_rhs->dup (scope) : nullptr, + line (), column (), op_type ()); + + new_be->copy_base (*this); + + return new_be; + } + + octave_value + tree_braindead_shortcircuit_binary_expression::evaluate (tree_evaluator& tw, + int) + { + if (m_lhs) + { + octave_value a = m_lhs->evaluate (tw); + + if (a.ndims () == 2 && a.rows () == 1 && a.columns () == 1) + { + bool result = false; + + bool a_true = a.is_true (); + + octave_value::binary_op oper_type = op_type (); + + if (a_true) + { + if (oper_type == octave_value::op_el_or) + { + matlab_style_short_circuit_warning ("|"); + return octave_value (true); + } + } + else + { + if (oper_type == octave_value::op_el_and) + { + matlab_style_short_circuit_warning ("&"); + return octave_value (false); + } + } + + if (m_rhs) + { + octave_value b = m_rhs->evaluate (tw); + + result = b.is_true (); + } + + return octave_value (result); + } + } + + return octave_value (); } // Boolean expressions.
--- a/libinterp/parse-tree/pt-binop.h Thu Nov 03 14:47:18 2022 -0400 +++ b/libinterp/parse-tree/pt-binop.h Thu Nov 03 17:50:24 2022 -0400 @@ -51,7 +51,7 @@ octave_value::binary_op t = octave_value::unknown_binary_op) : tree_expression (l, c), m_lhs (nullptr), m_rhs (nullptr), m_etype (t), - m_eligible_for_braindead_shortcircuit (false) + m_preserve_operands (false) { } tree_binary_expression (tree_expression *a, tree_expression *b, @@ -59,7 +59,7 @@ octave_value::binary_op t = octave_value::unknown_binary_op) : tree_expression (l, c), m_lhs (a), m_rhs (b), m_etype (t), - m_eligible_for_braindead_shortcircuit (false) + m_preserve_operands (false) { } // No copying! @@ -70,21 +70,14 @@ ~tree_binary_expression (void) { - delete m_lhs; - delete m_rhs; + if (! m_preserve_operands) + { + delete m_lhs; + delete m_rhs; + } } - void mark_braindead_shortcircuit (void) - { - if (m_etype == octave_value::op_el_and - || m_etype == octave_value::op_el_or) - { - m_eligible_for_braindead_shortcircuit = true; - - m_lhs->mark_braindead_shortcircuit (); - m_rhs->mark_braindead_shortcircuit (); - } - } + void preserve_operands (void) { m_preserve_operands = true; } bool is_binary_expression (void) const { return true; } @@ -97,11 +90,6 @@ tree_expression * lhs (void) { return m_lhs; } tree_expression * rhs (void) { return m_rhs; } - bool is_eligible_for_braindead_shortcircuit (void) const - { - return m_eligible_for_braindead_shortcircuit; - } - tree_expression * dup (symbol_scope& scope) const; octave_value evaluate (tree_evaluator&, int nargout = 1); @@ -131,9 +119,37 @@ // The type of the expression. octave_value::binary_op m_etype; - // TRUE if this is an | or & expression in the condition of an IF - // or WHILE statement. - bool m_eligible_for_braindead_shortcircuit; + // If TRUE, don't delete m_lhs and m_rhs in destructor; + bool m_preserve_operands; + }; + + class tree_braindead_shortcircuit_binary_expression + : public tree_binary_expression + { + public: + + tree_braindead_shortcircuit_binary_expression (tree_expression *a, + tree_expression *b, + int l, int c, + octave_value::binary_op t) + : tree_binary_expression (a, b, l, c, t) + { } + + // No copying! + + tree_braindead_shortcircuit_binary_expression + (const tree_braindead_shortcircuit_binary_expression&) = delete; + + tree_braindead_shortcircuit_binary_expression& + operator = (const tree_braindead_shortcircuit_binary_expression&) = delete; + + ~tree_braindead_shortcircuit_binary_expression (void) = default; + + tree_expression * dup (symbol_scope& scope) const; + + octave_value evaluate (tree_evaluator&, int nargout = 1); + + using tree_binary_expression::evaluate_n; }; // Boolean expressions.
--- a/libinterp/parse-tree/pt-exp.h Thu Nov 03 14:47:18 2022 -0400 +++ b/libinterp/parse-tree/pt-exp.h Thu Nov 03 17:50:24 2022 -0400 @@ -107,8 +107,6 @@ virtual std::string original_text (void) const; - virtual void mark_braindead_shortcircuit (void) { } - void mark_as_for_cmd_expr (void) { m_for_cmd_expr = true; } bool is_for_cmd_expr (void) const { return m_for_cmd_expr; }