oils-for-unix / oils

Oils is our upgrade path from bash to a better language and runtime. It's also for Python and JavaScript users who avoid shell!
http://www.oilshell.org/
Other
2.85k stars 158 forks source link

ternary operator ? should be right associative #712

Closed andychu closed 4 years ago

andychu commented 4 years ago

FizzBuzz doesn't work for me in bash 4.3 or 4.4?

What's wrong with the output? If you're referring to the error messages, only stdout is counted in the golf and the stderr junk is intended. You can prepend it with : to stop it.

The prime numbers program works with let as let(){ IFS=,;return $((!($*)));} and eval_unsafe_arith.

The divisors program doesn't work even with workarounds for the lack of $_ and $[ as OSH doesn't support mixed case in brace expansion ({a..C}).

The FizzBuzz program doesn't work even with workarounds for the lack of $_; it seems to be because OSH doesn't support nested ternary:

osh$ echo $((1?2?3:4:5))
  echo $((1?2?3:4:5))
             ^
[ interactive ]:1: Parser expected Id.Arith_Colon, got Id.Arith_QMark
$ echo $((1?2?3:4:5))
3

bash, dash, ksh, mrsh, yash, and zsh seem to support it, but ash doesn't seem to either

Originally posted by @Crestwave in https://github.com/oilshell/oil/issues/161#issuecomment-614574694

akinomyoga commented 4 years ago

Hi, this now causes a new error in source ble.osh. Here is a reduced test case:

$ osh -c '((1?a=1:(b=1)))'
  ((1?a=1:(b=1)))
       ^
[ -c flag ]:1: Parser expected Id.Arith_Colon, got Id.Arith_Equal
akinomyoga commented 4 years ago

I'm sorry the bug is not caused by this change. I'm not sure why the problem did not appear in source ble.sh until recently, but it turned out that this bug has existed before.

Edit: OK, this problem started to appear because of a recent change to ble.sh.

andychu commented 4 years ago

OK cool, yes it should be doing the right thing now! It was parsing incorrectly before this.

I guess you need parens around (a=1) like (b=1)

akinomyoga commented 4 years ago

I guess you need parens around (a=1) like (b=1)

Thank you. Yes, I know that I can work around it by adding parens, and in fact, now I have parens there to test ble.sh in Oil. But, we don't usually need such parens in conditional operators in C and similar languages. Is there any reason not to support it in Oil?

I tested with shells and was surprised that zsh and busybox sh don't support it. But all the other shells that I tested (dash, mksh, ksh93, ksh2020, bash, posh, oksh and pdksh) support $((1?a=1:1)). I also tested with ksh88, but it turned out that ksh88 only accept a simple number in conditional operators, i.e., neither $((1?a=1:1)) nor $((1?(a=1):1)) works. The Bourne shell doesn't have arithmetic expressions.

andychu commented 4 years ago

Ah it is possible that there is a precedence bug between ? and = ... let me see.

andychu commented 4 years ago

Yeah according to the precedence table, ? is higher precedence than =. But it also has this footnote:

https://en.cppreference.com/w/c/language/operator_precedence

The expression in the middle of the conditional operator (between ? and :) is parsed as if parenthesized: its precedence relative to ?: is ignored.

Shell specifies that its arithmetic is identical to C arithmetic, so I guess this is a bug.

So I am not sure right now how to fix this using the TDOP parsing algorithm, but it can probably be done. It sounds like it's a special case in C, which was originally done with the shunting yard algorithm... not sure if that matters or not.