Skip to content

Should $a instanceof $b instanceof $c be permitted in implementation/specification? #226

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TysonAndre opened this issue Dec 5, 2018 · 3 comments

Comments

@TysonAndre
Copy link
Contributor

TysonAndre commented Dec 5, 2018

$x instanceof $y instanceof $z is a nonsense expression, but is successfully parsed in PHP 5.6-7.3 (see https://3v4l.org/QWYZJ)

Either of the following would make sense:

  1. Mention that this is non-associative in php-langspec (to reflect php.net) (It seems like that's probably a consequence of the current CFG, so maybe nothing needs to be done)

    Additionally, try to make that a syntax error in php 8.x or 7.4, to reflect the documentation on php.net: https://secure.php.net/manual/en/language.operators.precedence.php

  2. Update the specification to allow it, and update php.net (it seems like it's currently parsed as ($x instanceof $y) instanceof $z according to ast\parse_code()) - I'm opposed to that, and it seems like the spec in this repo forbids that

I'm not quite sure what repo to file this in, or the process to request the change to php implementation's syntax, or if this has been mentioned elsewhere.

@nikic
Copy link
Member

nikic commented Dec 5, 2018

Nice catch! I don't think that allowing this was intentional. The operator is marked non-associative in the PHP grammar, but because it's not an ordinary binary operator (with expressions on both sides), that does not end up actually mattering.

If we want to change this, then an issue should also be filed on bugs.php.net, as changes generally always go from the implementation to the spec, no the other way around. I also suspect that this is going to be ugly to fix.

php-pulls pushed a commit that referenced this issue Mar 15, 2019
May not have been originally intended this way, but it's how the
language works right now, so specify it as such (#226).
@nikic
Copy link
Member

nikic commented Mar 15, 2019

For now, I've marked instanceof as left-associative in the spec, in order to reflect current reality: 15aa452

@mattacosta
Copy link

mattacosta commented Apr 19, 2019

I think a better solution is to just note that the Zend implementation is non-conforming. This has the benefit of keeping the parse error as acceptable, while also letting other parsers choose if they want to implement the deviation for compatibility purposes.

Since there are already multiple implementations that do not have this bug, it makes more sense to go through the RFC process at this point rather than unilaterally make them in violation of the specification too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants