Hi Benjamin, > We are looking for further feedback from the community.
Thanks, the updated RFC looks much better. Some more feedback on why the edge cases are a concern to me, and why the lack of an ending delimiter is similar to parsing problems already faced. I'd agree that restarting a two-week discussion period seems excessive, and a shorter one is fine with me. ---- The references section should probably also link to the other 2 RFC discussion threads https://externals.io/message/111416 and the RFC discussion thread https://externals.io/message/111218 ---- Developers who are new to PHP (e.g. learning by example) or those who spend most of their time programming in other languages such as Rust or Java or Golang might forget/not know that `#` is a supported line comment syntax, or that attributes were introduced in 8.0 (not an earlier release). They might see that code using these edge cases passes syntax checks in PHP 7.4 and 8.0, and assume it'd work the same way in both versions, leading to published code incorrectly marked as compatible with PHP 7.4. - PHP doesn't warn about calling a user-defined function too many parameters, making this harder to detect. ---- The `#[]` syntax makes it possible and convenient to use attributes syntax, but causes various problems for end users of code using attributes syntax. I'd expect the drawbacks of bugs caused by edge cases such as those in https://wiki.php.net/rfc/shorter_attribute_syntax_change#discussion_of_forwards_compatibility_procons to outweigh the benefits of being able to release attributes immediately. Putting attributes on the same line as parameters or closures or anonymous classes seems natural to do to me, and I'd expect releases would do it eventually, whether it be immediately in 8.0, years from now, etc. Suppose that package `A/A` supports PHP `7.4|^8.0`. It depends on a package `B/B` 2.0 which supports PHP 7.4. `B/B` depends on `C/C` `^4.0|^5.0`, where C/C 4.x supports PHP 7 and C/C 5.0 drops support for PHP 7. C/C 5.x uses attribute syntax on the same line as other code, including several edge cases with one-line attributes that parse differently in php 7.0. ``` public function doDbMaintenance( #[MyUnused] bool $log = false, // unused in db backend bool $doPotentiallyDestructiveUpdate = false, mixed $extraFlags = null, ) {...} // B/B calls doDbMaintenance($log = true) ``` Then suppose that a maintainer or third-party packager (e.g. for a Linux distro) of `A/A` runs composer install with PHP 8.0 before building a package such as a phar, rpm, or tarball (or `vendor/` checked into git) from `A/A` 1.0. They would pull in B/B 2.0 and C/C 5.0, creating a release of A/A that was buggy but did not emit any parse, compilation, or runtime warnings if end users ran that release with php 7.4. Then the maintainers of `A/A` and `C/C` 5.0 would get bug reports for PHP 7.4 for mysterious behaviors with no warnings, which would be hard to reproduce and debug, despite all involved packages correctly specifying their dependencies. End users would also be inconvenienced by bugs that had no obvious indication or subtler symptoms that may take a long time to get reported. This could be worked around by 1. Setting `{"config": {"platform": {"php": "7.4.9"}}}` in composer.json of A/A, or by documenting that `composer install` should always be run with PHP 7.4, but I wouldn't expect new composer users to be aware of the ability to do that until they run into issues, and that doesn't help if A/A is a dependency of another package. 2. C/C 5.0 could exit() when bootstrapping when it's run in php 7, but that seems excessive and not currently done by libraries in my experience. ---- If a developer needs to backport code or patches for php 8.0+ to php 7.4 or older (e.g. to support legacy applications or OSes that make updating php impractical), the lack of a syntax error would make this backporting error prone if the developer didn't remember this incompatibility or learn about tools created to catch issues. (or didn't know about *up-to-date, bug-free* tools to downgrade php syntax) ---- For https://wiki.php.net/rfc/shorter_attribute_syntax_change#machine_scanning_for_end_of_attribute_declaration I'd agree that the lack of an ending delimiter makes token-based extraction more complicated but still practical, but it is a complexity that projects such as phpcs **would already face in similar syntaxes.** The problems with whitespace and comments and doc comments between tokens can be fixed by filtering out whitespace and comments before/while looking to the end of a token. Projects such as nikic/php-parser or microsoft/tolerant-php-parser would be what I'd recommend for new code/scripts, but this is impractical for large existing codebases with third party plugins such as phpcs. A stack-based approach is used for `@[` or `#[`. A stack-based approach would also be used for `@@Attribute /*comment */ (` by skipping T_WHITESPACE, T_COMMENT, and T_DOC_COMMENT and ending parsing of that attribute if the next token isn't `(`. Skipping whitespace and comments that way already needs to be done to **correctly** find the end of the function call `myGlobalFunction /* comment */ (args);` (or `new MyClass /* comment */ (args)`). For both function calls and attributes, I'd assume style guides would forbid whitespace and comments before `(`. Thanks, - Tyson -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
