On Wed, Jul 29, 2020 at 2:21 AM tyson andre <tysonandre...@hotmail.com>
wrote:
> Hi internals,
>
> For #[, my main objection is the various ways this can change the lexing
> in a way that is impractical to (efficiently) backfill,
> and that the proposed patch doesn't address the fact that the syntax may
> change the syntax of php 7 code in unexpected ways.
>
> This syntax would help phpcs with easy examples **in the short term**,
> when attributes are on a single line,
> but would make more complicated refactorings buggy and error-prone unless
> phpcbf was run in the same major version.
> When attributes span multiple lines, the lexing is completely different
> for `#[]`.
> For example, the below syntax would yield false in php 7, but a generator
> in php 8.
> (There's precedent at least - the lexing of heredocs changed in php 7.3 or
> 7.4)
>
That's an interesting point. Emulating #[] lexing on older versions will
definitely be a challenge for PHP-Parser. I don't think we should make
concerns of external tooling hold us back too much, but the phpcs argument
really doesn't hold water.
Regards,
Nikita
> ```
> // Aside: This code snippet seems to have an assert failure in the lexer
> with the patch in the RFC
> // Zend/zend_compile.c:1794: zendlex: Assertion
> `!(executor_globals.exception) || ret == T_ERROR' failed.
> function generator() {
> yield #[MyCustomAttribute('
> false;
> // ']function() {};
> }
> ```
>
> And another example which would cause problems for phpcs in php 7 - the
> comment syntax can cause code to be treated as inline html instead of php
> tokens.
>
> ```
> <?php
> // This example echoes the rest of the source code in php 7
> // and echoes "Test" in php 8.
> #[DeprecationReason('reason: <https://some-website/reason?>')]
> function main() {}
> const APP_SECRET = 'app-secret';
> echo "Test\n";
> ```
>
> I'd posted another example in https://externals.io/message/111101#111133
>
> One way I'd thought of to avoid this ambiguity would be to assert in PHP
> 8.0 with an E_COMPILE_ERROR (or E_COMPILE_WARNING) that:
> 1. All tokens of the #[...] annotation are on the same line
> 2. No non-whitespace token follows the ] on the same line.
> It may be permissible to allow other `//` comments or multiple
> attributes after it on the same line, though.
> 3. No `?>` substrings within the rest of the line after #[, maybe
>
> Although I'm not sure if others are actually concerned about this
> ambiguity and these are really artificial examples for code.
>
> As for `<<`, I'm assuming people may have meant `yield <<JIT>> function()
> {};`,
> but `yield` already has a precedence and didn't have issues - otherwise
> `yield+2;` could be adding 2 to the result of a yield.
> (It's unambiguously `yield(+2);` right now).
> I forget how it'd be parsed, but it wouldn't be ambiguous.
>
> As for `@@`, all of its known issues seem to have been resolved,
> and there is still the potential for future issues, but I still prefer it
> over the `#[` implementation in that patch.
>
> P.S. I'd like to note that
> 1. A lot more discussion has occurred since the initial vote.
> 2. Since `@@` was obviously passing at the time, fewer voters would put
> much thought into detailed tradeoffs of `#[`
> 3. I haven't seen those specific drawbacks to `#[` of potentially
> significantly changing lexing (not just losing tokens) mentioned,
> but the shorter attributes syntax RFC did seem to mention params were
> commented out.
>
> One idea I'd have on voting would be to have the exact same 3-way vote,
> again, and pick the attribute syntax with the same ranked choice procedure.
> I assume proponents of `#[` would have similar objections if an RFC with a
> two-way vote for `<<>>` and `@@` was started first.
>
> Regards,
> - Tyson
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>
