From b2bbe7d8e2b196d341cd8c3e1d3b7600b81c1a41 Mon Sep 17 00:00:00 2001 From: Lukas Mai Date: Sun, 19 Nov 2023 18:37:39 +0100 Subject: [PATCH] elide right operand if left operand transfers control This commit contains the following changes: - The "possible precedence issue" warning now mentions (in parentheses) the name of the control flow operator that triggered the warning: $ ./perl -cwe 'return $a or $b' Possible precedence issue with control flow operator (return) at -e line 1. - CORE::dump now counts as a control flow operator: $ ./perl -cwe 'CORE::dump or f()' Possible precedence issue with control flow operator (dump) at -e line 1. - Any binary operator whose left operand is a control flow operator is optimized out along with any right operand it may have, even if no warning is triggered: $ ./perl -Ilib -MO=Deparse -we '(die $a) + $b' BEGIN { $^W = 1; } die $a; -e syntax OK (No warnings because explicit parentheses are used in the preceding example.) $ ./perl -Ilib -MO=Deparse -we 'exit $a ? 0 : 1' Possible precedence issue with control flow operator (exit) at -e line 1. BEGIN { $^W = 1; } exit $a; -e syntax OK --- lib/B/Deparse.t | 35 +++++++++++++++++++++++++++++- op.c | 46 +++++++++++++++++++++++----------------- pod/perldiag.pod | 2 +- t/lib/warnings/op | 54 +++++++++++++++++++++++------------------------ 4 files changed, 89 insertions(+), 48 deletions(-) diff --git a/lib/B/Deparse.t b/lib/B/Deparse.t index 68d9f667383e..3d61c18eb907 100644 --- a/lib/B/Deparse.t +++ b/lib/B/Deparse.t @@ -1857,7 +1857,7 @@ package foo; CORE::do({}); CORE::do({}); #### -# [perl #77096] functions that do not follow the llafr +# [perl #77096] functions that do not follow the looks-like-a-function rule () = (return 1) + time; () = (return ($1 + $2) * $3) + time; () = (return ($a xor $b)) + time; @@ -1877,6 +1877,26 @@ CORE::do({}); () = (-r $_) + 3; () = (-w $_) + 3; () = (-x $_) + 3; +>>>> +() = (return 1); +() = (return ($1 + $2) * $3); +() = (return ($a xor $b)); +() = (do 'file') + time; +() = (do ($1 + $2) * $3) + time; +() = (do ($1 xor $2)) + time; +() = (goto 1); +() = (require 'foo') + 3; +() = (require foo) + 3; +() = (CORE::dump 1); +() = (last 1); +() = (next 1); +() = (redo 1); +() = (-R $_) + 3; +() = (-W $_) + 3; +() = (-X $_) + 3; +() = (-r $_) + 3; +() = (-w $_) + 3; +() = (-x $_) + 3; #### # require(foo()) and do(foo()) require (foo()); @@ -1900,6 +1920,13 @@ $_ = ($a xor not +($1 || 2) ** 2); () = warn; () = warn() + 1; () = setpgrp() + 1; +>>>> +() = (eof) + 1; +() = (return); +() = (return, 1); +() = warn; +() = warn() + 1; +() = setpgrp() + 1; #### # loopexes have assignment prec () = (CORE::dump a) | 'b'; @@ -1907,6 +1934,12 @@ $_ = ($a xor not +($1 || 2) ** 2); () = (last a) | 'b'; () = (next a) | 'b'; () = (redo a) | 'b'; +>>>> +() = (CORE::dump a); +() = (goto a); +() = (last a); +() = (next a); +() = (redo a); #### # [perl #63558] open local(*FH) open local *FH; diff --git a/op.c b/op.c index abe428ec4316..4f3a75182cde 100644 --- a/op.c +++ b/op.c @@ -4302,8 +4302,8 @@ Perl_invert(pTHX_ OP *o) /* Warn about possible precedence issues if op is a control flow operator that does not terminate normally (return, exit, next, etc). */ -static void -S_check_terminates(pTHX_ OP *op) +static bool +S_is_control_transfer(pTHX_ OP *op) { assert(op != NULL); @@ -4312,15 +4312,10 @@ S_check_terminates(pTHX_ OP *op) $b)". */ switch (op->op_type) { + case OP_DUMP: case OP_NEXT: case OP_LAST: case OP_REDO: - /* XXX: Perhaps we should emit a stronger warning for these. - Even with the high-precedence operator they don't seem to do - anything sensible. - - But until we do, fall through here. - */ case OP_EXIT: case OP_RETURN: case OP_DIE: @@ -4337,12 +4332,12 @@ S_check_terminates(pTHX_ OP *op) */ if (!op->op_folded && !(op->op_flags & OPf_PARENS)) Perl_ck_warner(aTHX_ packWARN(WARN_SYNTAX), - "Possible precedence issue with control flow operator"); - /* XXX: Should we optimze this to "return $a;" (i.e. remove - the "or $b" part)? - */ - break; + "Possible precedence issue with control flow operator (%s)", OP_DESC(op)); + + return true; } + + return false; } OP * @@ -4354,7 +4349,7 @@ Perl_cmpchain_start(pTHX_ I32 type, OP *left, OP *right) if (!left) left = newOP(OP_NULL, 0); else - S_check_terminates(aTHX_ left); + (void)S_is_control_transfer(aTHX_ left); if (!right) right = newOP(OP_NULL, 0); scalar(left); @@ -5962,8 +5957,11 @@ Perl_newBINOP(pTHX_ I32 type, I32 flags, OP *first, OP *last) if (!first) first = newOP(OP_NULL, 0); - else - S_check_terminates(aTHX_ first); + else if (S_is_control_transfer(aTHX_ first)) { + op_free(last); + first->op_folded = 1; + return first; + } NewOp(1101, binop, 1, BINOP); @@ -8759,13 +8757,17 @@ S_new_logop(pTHX_ I32 type, I32 flags, OP** firstp, OP** otherp) if (type == OP_XOR) /* Not short circuit, but here by precedence. */ return newBINOP(type, flags, scalar(first), scalar(other)); - S_check_terminates(aTHX_ first); - assert((PL_opargs[type] & OA_CLASS_MASK) == OA_LOGOP || type == OP_CUSTOM); scalarboolean(first); + if (S_is_control_transfer(aTHX_ first)) { + op_free(other); + first->op_folded = 1; + return first; + } + /* search for a constant op that could let us fold the test */ if ((cstop = search_const(first))) { if (cstop->op_private & OPpCONST_STRICT) @@ -8931,8 +8933,14 @@ Perl_newCONDOP(pTHX_ I32 flags, OP *first, OP *trueop, OP *falseop) if (!trueop) return newLOGOP(OP_OR, 0, first, falseop); - S_check_terminates(aTHX_ first); scalarboolean(first); + if (S_is_control_transfer(aTHX_ first)) { + op_free(trueop); + op_free(falseop); + first->op_folded = 1; + return first; + } + if ((cstop = search_const(first))) { /* Left or right arm of the conditional? */ const bool left = SvTRUE(cSVOPx(cstop)->op_sv); diff --git a/pod/perldiag.pod b/pod/perldiag.pod index 870e923b12a8..35de588c4fc8 100644 --- a/pod/perldiag.pod +++ b/pod/perldiag.pod @@ -5392,7 +5392,7 @@ Perl guesses a reasonable buffer size, but puts a sentinel byte at the end of the buffer just in case. This sentinel byte got clobbered, and Perl assumes that memory is now corrupted. See L. -=item Possible precedence issue with control flow operator +=item Possible precedence issue with control flow operator (%s) (W syntax) There is a possible problem with the mixing of a control flow operator (e.g. C) and a low-precedence operator like diff --git a/t/lib/warnings/op b/t/lib/warnings/op index 6a2c181b7c2e..3c4bad621751 100644 --- a/t/lib/warnings/op +++ b/t/lib/warnings/op @@ -1841,33 +1841,33 @@ sub do_warn_25 { redo $a xor $b while(1); } sub do_warn_26 { $b if return $a; } sub do_warn_27 { $b if die $a; } EXPECT -Possible precedence issue with control flow operator at - line 3. -Possible precedence issue with control flow operator at - line 4. -Possible precedence issue with control flow operator at - line 5. -Possible precedence issue with control flow operator at - line 6. -Possible precedence issue with control flow operator at - line 7. -Possible precedence issue with control flow operator at - line 8. -Possible precedence issue with control flow operator at - line 9. -Possible precedence issue with control flow operator at - line 10. -Possible precedence issue with control flow operator at - line 11. -Possible precedence issue with control flow operator at - line 15. -Possible precedence issue with control flow operator at - line 16. -Possible precedence issue with control flow operator at - line 17. -Possible precedence issue with control flow operator at - line 18. -Possible precedence issue with control flow operator at - line 20. -Possible precedence issue with control flow operator at - line 21. -Possible precedence issue with control flow operator at - line 22. -Possible precedence issue with control flow operator at - line 23. -Possible precedence issue with control flow operator at - line 24. -Possible precedence issue with control flow operator at - line 25. -Possible precedence issue with control flow operator at - line 26. -Possible precedence issue with control flow operator at - line 27. -Possible precedence issue with control flow operator at - line 28. -Possible precedence issue with control flow operator at - line 29. -Possible precedence issue with control flow operator at - line 30. -Possible precedence issue with control flow operator at - line 31. -Possible precedence issue with control flow operator at - line 33. -Possible precedence issue with control flow operator at - line 34. +Possible precedence issue with control flow operator (return) at - line 3. +Possible precedence issue with control flow operator (return) at - line 4. +Possible precedence issue with control flow operator (return) at - line 5. +Possible precedence issue with control flow operator (die) at - line 6. +Possible precedence issue with control flow operator (die) at - line 7. +Possible precedence issue with control flow operator (die) at - line 8. +Possible precedence issue with control flow operator (exit) at - line 9. +Possible precedence issue with control flow operator (exit) at - line 10. +Possible precedence issue with control flow operator (exit) at - line 11. +Possible precedence issue with control flow operator (exit) at - line 15. +Possible precedence issue with control flow operator (exit) at - line 16. +Possible precedence issue with control flow operator (exit) at - line 17. +Possible precedence issue with control flow operator (exit) at - line 18. +Possible precedence issue with control flow operator (goto) at - line 20. +Possible precedence issue with control flow operator (goto) at - line 21. +Possible precedence issue with control flow operator (goto) at - line 22. +Possible precedence issue with control flow operator (next) at - line 23. +Possible precedence issue with control flow operator (next) at - line 24. +Possible precedence issue with control flow operator (next) at - line 25. +Possible precedence issue with control flow operator (last) at - line 26. +Possible precedence issue with control flow operator (last) at - line 27. +Possible precedence issue with control flow operator (last) at - line 28. +Possible precedence issue with control flow operator (redo) at - line 29. +Possible precedence issue with control flow operator (redo) at - line 30. +Possible precedence issue with control flow operator (redo) at - line 31. +Possible precedence issue with control flow operator (return) at - line 33. +Possible precedence issue with control flow operator (die) at - line 34. ######## # op.c # (same as above, except these should not warn)