From 5f56a0388271d2de6cf31af1041bdcb3d11029fc Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Thu, 15 Jun 2023 10:07:12 +0200 Subject: shell/math: fix parsing of ?: and explain why it's parsed that way This fixes arith-precedence1.tests. This breaks arith-ternary2.tests again (we now evaluate variables on not-taken branches). We need a better logic here anyway: not only bare variables should not evaluate when not-taken: 1 ? eval_me : do_not_eval but any (arbitrarily complex) expressions shouldn't evaluate as well! 1 ? var_is_set=1 : ((var_is_not_set=2,var2*=4)) function old new delta evaluate_string 1097 1148 +51 Signed-off-by: Denys Vlasenko --- .../ash_test/ash-arith/arith-ternary_nested1.right | 1 + .../ash_test/ash-arith/arith-ternary_nested1.tests | 2 + shell/ash_test/ash-arith/arith.right | 2 +- shell/hush_test/hush-arith/arith-precedence1.right | 4 ++ shell/hush_test/hush-arith/arith-precedence1.tests | 15 +++++++ shell/hush_test/hush-arith/arith-ternary2.right | 3 -- shell/hush_test/hush-arith/arith-ternary2.tests | 7 ---- .../hush-arith/arith-ternary_nested1.right | 1 + .../hush-arith/arith-ternary_nested1.tests | 2 + shell/hush_test/hush-arith/arith.right | 2 +- shell/math.c | 48 +++++++++++++++------- 11 files changed, 61 insertions(+), 26 deletions(-) create mode 100644 shell/ash_test/ash-arith/arith-ternary_nested1.right create mode 100755 shell/ash_test/ash-arith/arith-ternary_nested1.tests create mode 100644 shell/hush_test/hush-arith/arith-precedence1.right create mode 100755 shell/hush_test/hush-arith/arith-precedence1.tests delete mode 100644 shell/hush_test/hush-arith/arith-ternary2.right delete mode 100755 shell/hush_test/hush-arith/arith-ternary2.tests create mode 100644 shell/hush_test/hush-arith/arith-ternary_nested1.right create mode 100755 shell/hush_test/hush-arith/arith-ternary_nested1.tests diff --git a/shell/ash_test/ash-arith/arith-ternary_nested1.right b/shell/ash_test/ash-arith/arith-ternary_nested1.right new file mode 100644 index 000000000..d80319695 --- /dev/null +++ b/shell/ash_test/ash-arith/arith-ternary_nested1.right @@ -0,0 +1 @@ +3:3 diff --git a/shell/ash_test/ash-arith/arith-ternary_nested1.tests b/shell/ash_test/ash-arith/arith-ternary_nested1.tests new file mode 100755 index 000000000..469584bea --- /dev/null +++ b/shell/ash_test/ash-arith/arith-ternary_nested1.tests @@ -0,0 +1,2 @@ +exec 2>&1 +echo 3:$((1?(2?(3):4):5)) diff --git a/shell/ash_test/ash-arith/arith.right b/shell/ash_test/ash-arith/arith.right index 61fcab55e..8bc78b8d1 100644 --- a/shell/ash_test/ash-arith/arith.right +++ b/shell/ash_test/ash-arith/arith.right @@ -92,7 +92,7 @@ ghi ./arith.tests: line 190: arithmetic syntax error 16 16 ./arith.tests: line 195: arithmetic syntax error -./arith.tests: line 196: malformed ?: operator +./arith.tests: line 196: arithmetic syntax error ./arith.tests: line 197: arithmetic syntax error 9 9 ./arith.tests: line 204: arithmetic syntax error diff --git a/shell/hush_test/hush-arith/arith-precedence1.right b/shell/hush_test/hush-arith/arith-precedence1.right new file mode 100644 index 000000000..3f9320a13 --- /dev/null +++ b/shell/hush_test/hush-arith/arith-precedence1.right @@ -0,0 +1,4 @@ +4:4 +4:4 +4:4 +4:4 diff --git a/shell/hush_test/hush-arith/arith-precedence1.tests b/shell/hush_test/hush-arith/arith-precedence1.tests new file mode 100755 index 000000000..bfef05292 --- /dev/null +++ b/shell/hush_test/hush-arith/arith-precedence1.tests @@ -0,0 +1,15 @@ +exec 2>&1 +# bash documentation says that precedence order is: +# ... +# expr ? expr1 : expr2 +# = *= /= %= += -= <<= >>= &= ^= |= +# exprA , exprB +# but in practice, the rules for expr1 and expr2 are different: +# assignments and commas in expr1 have higher precedence than :?, +# but in expr2 they haven't: +# "v ? 1,2 : 3,4" is parsed as "(v ? (1,2) : 3),4" +# "v ? a=2 : b=4" is parsed as "(v ? (a=1) : b)=4" (thus, this is a syntax error) +echo 4:$((0 ? 1,2 : 3,4)) +echo 4:$((1 ? 1,2 : 3,4)) +echo 4:"$((0 ? 1,2 : 3,4))" +echo 4:"$((1 ? 1,2 : 3,4))" diff --git a/shell/hush_test/hush-arith/arith-ternary2.right b/shell/hush_test/hush-arith/arith-ternary2.right deleted file mode 100644 index a549b1b5c..000000000 --- a/shell/hush_test/hush-arith/arith-ternary2.right +++ /dev/null @@ -1,3 +0,0 @@ -6:6 -a=b=+err+ -b=6 diff --git a/shell/hush_test/hush-arith/arith-ternary2.tests b/shell/hush_test/hush-arith/arith-ternary2.tests deleted file mode 100755 index cb3163932..000000000 --- a/shell/hush_test/hush-arith/arith-ternary2.tests +++ /dev/null @@ -1,7 +0,0 @@ -exec 2>&1 -a='b=+err+' -b=5 -# The not-taken branch should not parse variables -echo 6:$((0 ? a : ++b)) -echo "a=$a" -echo "b=$b" diff --git a/shell/hush_test/hush-arith/arith-ternary_nested1.right b/shell/hush_test/hush-arith/arith-ternary_nested1.right new file mode 100644 index 000000000..d80319695 --- /dev/null +++ b/shell/hush_test/hush-arith/arith-ternary_nested1.right @@ -0,0 +1 @@ +3:3 diff --git a/shell/hush_test/hush-arith/arith-ternary_nested1.tests b/shell/hush_test/hush-arith/arith-ternary_nested1.tests new file mode 100755 index 000000000..469584bea --- /dev/null +++ b/shell/hush_test/hush-arith/arith-ternary_nested1.tests @@ -0,0 +1,2 @@ +exec 2>&1 +echo 3:$((1?(2?(3):4):5)) diff --git a/shell/hush_test/hush-arith/arith.right b/shell/hush_test/hush-arith/arith.right index a8612295e..df8154f97 100644 --- a/shell/hush_test/hush-arith/arith.right +++ b/shell/hush_test/hush-arith/arith.right @@ -94,7 +94,7 @@ ghi hush: arithmetic syntax error 16 16 hush: arithmetic syntax error -hush: malformed ?: operator +hush: arithmetic syntax error hush: arithmetic syntax error 9 9 hush: arithmetic syntax error diff --git a/shell/math.c b/shell/math.c index 748c3b3ad..f6aa02ac2 100644 --- a/shell/math.c +++ b/shell/math.c @@ -157,17 +157,17 @@ typedef unsigned char operator; #define fix_assignment_prec(prec) do { if (prec == 3) prec = 2; } while (0) /* Ternary conditional operator is right associative too */ -// FIXME: -// bash documentation says that precedence order is: -// ... -// expr ? expr1 : expr2 -// = *= /= %= += -= <<= >>= &= ^= |= -// exprA , exprB -// but in practice, the rules for expr1 and expr2 are different: -// assignments and commas in expr1 have higher precedence than ?:, -// but in expr2 they haven't: -// "v ? 1,2 : 3,4" is parsed as "(v ? (1,2) : 3),4" -// "v ? a=2 : b=4" is parsed as "(v ? (a=1) : b)=4" (thus, this is a syntax error) +/* + * bash documentation says that precedence order is: + * ... + * expr ? expr1 : expr2 + * = *= /= %= += -= <<= >>= &= ^= |= + * exprA , exprB + * What it omits is that expr1 is parsed as if parenthesized + * (this matches the rules of ?: in C language): + * "v ? 1,2 : 3,4" is parsed as "(v ? (1,2) : 3),4" + * "v ? a=2 : b=4" is parsed as "(v ? (a=1) : b)=4" (thus, this is a syntax error) + */ #define TOK_CONDITIONAL tok_decl(4,0) #define TOK_CONDITIONAL_SEP tok_decl(4,1) @@ -629,6 +629,7 @@ evaluate_string(arith_state_t *math_state, const char *expr) /* Stack of operator tokens */ operator *const opstack = alloca(expr_len * sizeof(opstack[0])); operator *opstackptr = opstack; + operator insert_op = 0xff; /* Start with a left paren */ dbg("(%d) op:TOK_LPAREN", (int)(opstackptr - opstack)); @@ -751,11 +752,24 @@ evaluate_string(arith_state_t *math_state, const char *expr) goto err; } } + /* NB: expr now points past the operator */ tok_found: op = p[1]; /* fetch TOK_foo value */ - tok_found1: - /* NB: expr now points past the operator */ + /* Special rule for "? EXPR :" + * "EXPR in the middle of ? : is parsed as if parenthesized" + * (this quirk originates in C grammar, I think). + */ + if (op == TOK_CONDITIONAL) { + insert_op = TOK_LPAREN; + dbg("insert_op=%02x", insert_op); + } + if (op == TOK_CONDITIONAL_SEP) { + insert_op = op; + op = TOK_RPAREN; + dbg("insert_op=%02x op=%02x", insert_op, op); + } + tok_found1: /* post grammar: a++ reduce to num */ if (lasttok == TOK_POST_INC || lasttok == TOK_POST_DEC) lasttok = TOK_NUM; @@ -865,9 +879,15 @@ dbg(" numstack:%d val:%lld '%s'", (int)(numstackptr - numstack), numstackptr[ /* else: LPAREN or UNARY: push it on opstack */ push_op: /* Push this operator to opstack */ - dbg("(%d) op:%02x", (int)(opstackptr - opstack), op); + dbg("(%d) op:%02x insert_op:%02x", (int)(opstackptr - opstack), op, insert_op); *opstackptr++ = lasttok = op; next: ; + if (insert_op != 0xff) { + op = insert_op; + insert_op = 0xff; + dbg("inserting %02x", op); + goto tok_found1; + } } /* while (1) */ err: -- cgit v1.2.3-55-g6feb