LHS of assignment must be variable
nikic opened this issue · 7 comments
Noticed while looking at example 4:
In PHP an expression like $a == $b = $c is parsed as $a == ($b = $c), because this is the only valid way to parse the code under the constraint that the LHS of an assignment must be a variable. The parser currently treats this as ($a == $b) = $c instead.
The same also applies to other expressions that accept only variables on the LHS, for example PHP interprets !$x instanceof Y as !($x instanceof Y), while the parser currently treats it as (!$x) instanceof Y.
Ahh, makes sense. Thanks for pointing that out! I was wondering why variables were called out separately from expressions in general.
For instanceof, I think I'm missing something because both the spec and yacc grammar seem to specify expression as LHS.
Looking back at my notes, my interpretation of the spec was that unary-expression takes precedence over instanceof-expression, which in turn takes precedence over multiplicative-expression, where both intanceof and multiplicative expressions are defined similarly, which led me to believe they're parsed similarly.
instanceof-expression:
unary-expression
instanceof-subject instanceof instanceof-type-designator
instanceof-subject:
expression
multiplicative-expression:
instanceof-expression
multiplicative-expression * instanceof-expression
multiplicative-expression / instanceof-expression
multiplicative-expression % instanceof-expression
Should instanceof-subject be variable, rather than expression? And if so, how exactly does that get specified in the yacc grammar?
You're right about instanceof, I got confused there. The LHS is a normal expression, it's the RHS that is a (restricted) variable.
However, the spec is incorrect about the precedence of instanceof: It should have higher precedence than ! (but lower than ~, so different unary operator are treated differently here).
👍
I also happened to come across the following example that is related to this while looking at an RFC:
!$x = f() should be !($x = f()) instead of (!$x) = f()
@TysonAndre The affected operators with this restriction are:
- All assignment operators (it looks like you got all of these)
- Increment and decrement operators
The instanceof operator is not affected by the restriction. It only appears to be related because the ! operator has the wrong precedence. This means that the PR probably makes instanceof have an incorrect precedence as well, and that part of the change should be reverted.
Also, before anyone goes and thinks about closing this issue, note that the previous PR is just a workaround. It alters the precedence of those operators, but that isn't the cause of the issue. For example, the parser will still parse statements such as 1 = 2 without error.
The instanceof operator is not affected by the restriction. It only appears to be related because the ! operator has the wrong precedence.
That sounds right, after a few checks. I agree that this should be kept open.
I was basing my changes off of the comment from #19 (comment) . But that earlier comment now seems incorrect, according to https://secure.php.net/manual/en/language.operators.precedence.php (Which puts ! as lower precedence than instanceof)
You're right about instanceof, I got confused there. The LHS is a normal expression, it's the RHS that is a (restricted) variable.
I missed/misread the followup comment they made.
-
I had thought that
~$x instanceof Twould be parsed as~($x instanceof T), but it's actually parsed as(~$x) instanceof T. There's no special case. My change probably introduced a bug for~$x instanceof T. -
As mattacosta said, the precedence of instanceof (compared to unary operators) should be fixed.
And more tests should be added.
php > $x = new stdClass();
php > var_export(~$x instanceof stdClass);
Warning: Uncaught Error: Unsupported operand types in php shell code:1
Also, I'd agree that it looks like 1 = 2; is a syntax error (e.g. for getDiagnostics()), not a semantics error. That may be easier to track in a separate ticket.
https://github.com/php/php-langspec/blob/master/spec/10-expressions.md#assignment-operators limits the valid expression types for lhs
php > var_export(ast\parse_code('<?php 2 = 3;', 50));
Parse error: syntax error, unexpected '=' in string code on line 1