[Toybox] [PATCH] Fix the operator precedence in expr.

Andy Chu andychup at gmail.com
Wed Mar 16 18:20:26 PDT 2016


> I applied your two patches and I've done one round of cleanup on top of
> them so far. Then I started replacing "TT.tok = toys.optargs++" with
> directly incrementing TT.tok and I introduced a bug with parentheses
> management and got myself confused and went to do something else for a
> while and haven't gotten back to it yet.

Yeah it's a little fiddly, but elegant when you get used to the
algorithm's logic.  To fix the misleading error message on expr \( \),
try this.

+   if (!strcmp(TT.tok, ")")) {      // an atom can't start with )
+     error_exit("Unexpected )");
-   if (!strcmp(TT.tok, "(")) { // parenthesized expression
+   } else if (!strcmp(TT.tok, "(")) { // parenthesized expression

The entry into eval_expr() expects an "atom", which is either
something like "1" or "a" or a parenthesized expression like ( 1 + 2
).

Note that expr \* is valid -- it treats it like a literal string.  So
expr \) was ALSO valid.

expr \( \) \) was valid  -- it is a string inside parentheses.  Thus
expr \( \)  was being parsed as OPEN PAREN, STRING, and then ERROR,
expected closing \).  So it was technically correct, though very
confusing  This tiny patch will disallow things like expr \) and expr
\( \) \).

I actually had this check in my original version, but removed it
because of minimalism, but didn't realize that confusing case.

>> I wanted to bring up the one TODO in the patches... syntax_error()
>> defaults to printing "syntax error", but the caller passes a more
>> detailed error message, which can be enabled with "if (1)".
>
> This morning's cleanup pass replaced the calls to syntax_error() with
> calls to error_exit(), making it always print the more detailed error
> message.

OK awesome!  Makes sense.

>> That would make it clearer that + is both a unary and binary operator,
>> while - is only a binary operator.
>
> So my help text is wrong and not _all_ operators are infix.
>
> Sigh...

No your help is correct!  I didn't implement what GNU expr does (nor
did existing code).  It "puns" + as a unary escaping operator (in
addition to infix addition).

I wrote about this in my message about find / expr / test.

It's because GNU has "match" / "substr" operators, which we don't
have.  This is the problem:

$ expr foo = foo
1

$ expr match = match
expr: syntax error  # it's expecting arguments to the operator "match"

$ expr + match = + match  # escape match operator as string
1

I think the command is OK as is -- it's POSIX conformant... It's
possible we could run into something in Aboriginal LInux that uses the
match / substr operators, in which case we could add them and the
annoying + unary escape.

Andy

 1458177626.0


More information about the Toybox mailing list