Repository navigation
PHP 8.0+ does no longer accept a comment in the whitespace between ampersand and argument in function signature #10083
Description
Description
The following code:
<?php
function foo(
array
#if __PHP__
&
#endif
$bodies
)
{
}Resulted in this output:
Parse error: syntax error, unexpected variable "$bodies" in /in/aq2rT on line 8
Process exited with code 255.
But I expected this output instead:
<Empty, should compile without syntax error as in PHP 7.4
PHP Version
PHP 8.1 and above, works fine in PHP 7.4
Operating System
No response
Activity
iluuu1994 commentedon Dec 11, 2022
This is expected and caused by some parser/tokenizer hackery for intersection types that was necessary to avoid ambiguity. See 069a9fa.
olleharstedt commentedon Dec 11, 2022
Won't fix = impossible to fix?
iluuu1994 commentedon Dec 11, 2022
Yes 🙂
olleharstedt commentedon Dec 11, 2022
😢
iluuu1994 commentedon Dec 11, 2022
I think in theory we could adjust the tokens so that variables are assumed after
& /* comment */instead of intersection types, but one case will always not work.olleharstedt commentedon Dec 11, 2022
Are comments part of the token stream? That's the problem? Because whitespace and newline works fine.
iluuu1994 commentedon Dec 11, 2022
No. The problem is that the grammar is ambiguous. Our LR(1) parser only gets one token of lookahead, but code like
function foo(Foo |& Bar $param) {(|denoting the current position) requires multiple tokens of lookahead to understand if this is a param by reference (Foo &$param) or an intersection type. We avoid this ambiguity by creating different tokens for&depending on what follows, but this has limitations in that comments will influence tokenization.php-src/Zend/zend_language_scanner.l
Lines 1872 to 1879 in b96b88b
olleharstedt commentedon Dec 11, 2022
Ah ok. But expanding
<ST_IN_SCRIPTING>"&"[ \t\r\n]*("$"|"...") {to also eat comments could fix it? If that's possible during lexing. But maybe not good for lexing performance. Or possible only eat the single-line comments # and //.5 remaining items
olleharstedt commentedon Dec 11, 2022
Yep, can do, thanks!
Danack commentedon Dec 11, 2022
To be clear, this appears to have been found during someone doing code-golf. It doesn't appear to affect an in production codebase.
olleharstedt commentedon Dec 11, 2022
Possibly this. But does not match newlines inside /* */ comments. :/
iluuu1994 commentedon Dec 12, 2022
Attempted fix #10084 showed this doesn't seem feasible for now.
cmb69 commentedon Dec 12, 2022
I agree that we shouldn't try to fix this with more regexps; in the long run, we might consider using a GLP parser. Still, it may make sense to document the current restriction.
iluuu1994 commentedon Dec 18, 2022
Reopening as it looks like there's a viable solution after all. See GH-10125.
derickr commentedon Feb 8, 2023
This seems to break installs with
re2c0.15.3, which the date/time parser still uses. The source code also defines the minimum as0.13.4which is also broken. It looks like re2c1.0.3(which is what my release VM insisted on) does work.I can't find in the GIT repo or mailing list history why the date/time parser (which we commit) couldn't deal with a later version. I think we should change the minimum in PHP to 1.0.3 (like the release images do) and I will also start using that for timelib's parser.
I've made a PR to add that check: #10542
Allow comments between intersection types and by-ref params