Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » Request for opinions: bug vs feature - change in tokenization of yield from »

Request for opinions: bug vs feature - change in tokenization of yield from

php.internals

Unnamed Person

2 years ago
Hi all, I recently discovered a change was made to the Tokenizer in PHP 8.3.0 which now allows for a comment to exist between the `yield` and `from` keyword from the `yield from` keyword. Before PHP 8.3, this was a parse error. This change was not documented (anywhere) nor publicized, which is why I only recently came across it and opened a bug report on GitHub about it [1]. This change is a breaking change for projects consuming the token stream, but more than that, it introduces an inconsistency in the Tokenizer as there is no other single token in PHP which can contain a comment (aside from comments tokens of course). Comments were even explicitly forbidden in the PHP 8.0 RFC which changed the tokenization of namespaced names [2]. Some more detailed explanation and examples can be found in the GH thread [3] and adetailed analysis of the tokenization change can be found in a ticket in the PHP_CodeSniffer repo [4] (details in a fold out in the comment). In my opinion the change is a bug and should be reverted, but opinions on this are divided. After a discussion in the ticket on GitHub, it was suggested to ask for a third/fourth/fifth opinion here on the list. Pros for reverting it: - Consistency with other tokens in the token stream, none of which allow comments within the token. - Consistency with the 7 years before in which this was a parse error. - The change was never documented, not in the changelog, news, nor in the upgrading guide. Cons against reverting it: - The change has been in PHP 8.3 releases for a little over seven months. - The odd few people may have accidentally discovered the bug and taken advantage by introducing code containing `yield /*comment*/ from` into their project, which with a revert would become a parse error again. Opinions ? Of course, the ticket yielded discussion on whether the tokenization of `yield from` should be changed anyhow, but please consider that discussion out of scope. The current question is only about reverting the bug versus regarding it as a new feature and properly documenting it. Smile, Juliette 1: https://github.com/php/php-src/issues/14926 2: https://wiki.php.net/rfc/namespaced_names_as_token#backward_incompatible_changes 3: https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/529#issuecomment-2209337419 4: https://github.com/php/php-src/issues/14926#issuecomment-2227152061

Bob Weinand

2 years ago
Am 18.07.2024 um 01:15 schrieb Juliette Reinders Folmer <php-internals_nospam@adviesenzo.nl>:  Hi all, I recently discovered a change was made to the Tokenizer in PHP 8.3.0 which now allows for a comment to exist between the `yield` and `from` keyword from the `yield from` keyword. Before PHP 8.3, this was a parse error. This change was not documented (anywhere) nor publicized, which is why I only recently came across it and opened a bug report on GitHub about it [1]. This change is a breaking change for projects consuming the token stream, but more than that, it introduces an inconsistency in the Tokenizer as there is no other single token in PHP which can contain a comment (aside from comments tokens of course). Comments were even explicitly forbidden in the PHP 8.0 RFC which changed the tokenization of namespaced names [2]. Some more detailed explanation and examples can be found in the GH thread [3] and a detailed analysis of the tokenization change can be found in a ticket in the PHP_CodeSniffer repo [4] (details in a fold out in the comment). In my opinion the change is a bug and should be reverted, but opinions on this are divided. After a discussion in the ticket on GitHub, it was suggested to ask for a third/fourth/fifth opinion here on the list. Pros for reverting it: - Consistency with other tokens in the token stream, none of which allow comments within the token. - Consistency with the 7 years before in which this was a parse error. - The change was never documented, not in the changelog, news, nor in the upgrading guide. Cons against reverting it: - The change has been in PHP 8.3 releases for a little over seven months. - The odd few people may have accidentally discovered the bug and taken advantage by introducing code containing `yield /*comment*/ from` into their project, which with a revert would become a parse error again. Opinions ? Of course, the ticket yielded discussion on whether the tokenization of `yield from` should be changed anyhow, but please consider that discussion out of scope. The current question is only about reverting the bug versus regarding it as a new feature and properly documenting it. Smile, Juliette 1: https://github.com/php/php-src/issues/14926 2: https://wiki.php.net/rfc/namespaced_names_as_token#backward_incompatible_changes 3: https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/529#issuecomment-2209337419 4: https://github.com/php/php-src/issues/14926#issuecomment-2227152061 Thanks for bringing this up. I'm honestly not sure what the best behavior for PHP 8.3 is. But I'm strongly thinking the behavior of allowing comments between yield and from is natural and expected. So, for PHP 8.4 I propose we ensure proper tokenization of yield /* comment */ from. Changing this in PHP 8.3 is likely too much of a compatibility break. So, assuming we want the proper tokenization behavior for PHP 8.4, I think changing the behavior intermittently to disallow comments between yield and from in PHP 8.3 is counterproductive. Moreover, it can - at least - be worked around in tooling by special casing the T_YIELD_FROM token and extracting the comment from the raw parsed string: var_dump(token_get_all('<?php yield /* comment */ from $foo;')); will contain: [1]=> array(3) { [0]=> int(270) [1]=> string(24) "yield /* comment */ from" [2]=> int(1) } It's not optimal, but probably the least bad solution to leave it unchanged in PHP 8.3, have tooling special case it and properly fix it in PHP 8.4. Bob

Christoph Becker

2 years ago
Hi Bob! On 18.07.2024 at 15:41, Bob Weinand wrote:
> Moreover, it can - at least - be worked around in tooling by special casing the T_YIELD_FROM token and extracting the comment from the raw parsed string: > > var_dump(token_get_all('<?php yield /* comment */ from $foo;')); > > will contain: > > [1]=> array(3) { [0]=> int(270) [1]=> string(24) "yield /* comment */ from" [2]=> int(1) } > > It's not optimal, but probably the least bad solution to leave it unchanged in PHP 8.3, have tooling special case it and properly fix it in PHP 8.4.
And what about "code" like <https://3v4l.org/4CLhM>? Is Codesniffer supposed to scan the result of <https://3v4l.org/dKDcs> for possible CS violations? Cheers, Christoph

Bob Weinand

2 years ago
Hey Christoph, Am 19.07.2024 um 00:51 schrieb Christoph M. Becker <cmbecker69@gmx.de>: Hi Bob! On 18.07.2024 at 15:41, Bob Weinand wrote: Moreover, it can - at least - be worked around in tooling by special casing the T_YIELD_FROM token and extracting the comment from the raw parsed string: var_dump(token_get_all('<?php yield /* comment */ from $foo;')); will contain: [1]=> array(3) { [0]=> int(270) [1]=> string(24) "yield /* comment */ from" [2]=> int(1) } It's not optimal, but probably the least bad solution to leave it unchanged in PHP 8.3, have tooling special case it and properly fix it in PHP 8.4. And what about "code" like <https://3v4l.org/4CLhM>? Is Codesniffer supposed to scan the result of <https://3v4l.org/dKDcs> for possible CS violations? Cheers, Christoph I suppose you mean https://3v4l.org/IMi8Y, (you missed the <?php tag). If you want to scan that, it's quite easy to strip the leading yield and trailing from, and tokenize that again to extract all comments. Sure, it's a hack, but it'll work: https://3v4l.org/8eAiV. Bob

Unnamed Person

2 years ago
On 19-7-2024 1:09, Bob Weinand wrote:
> Hey Christoph, > >> Am 19.07.2024 um 00:51 schrieb Christoph M. Becker <cmbecker69@gmx.de>: >> >> Hi Bob! >> >> On 18.07.2024 at 15:41, Bob Weinand wrote: >> >>> Moreover, it can - at least - be worked around in tooling by special >>> casing the T_YIELD_FROM token and extracting the comment from the >>> raw parsed string: >>> >>> var_dump(token_get_all('<?php yield /* comment */ from $foo;')); >>> >>> will contain: >>> >>> [1]=> array(3) { [0]=> int(270) [1]=> string(24) "yield /* comment >>> */ from" [2]=> int(1) } >>> >>> It's not optimal, but probably the least bad solution to leave it >>> unchanged in PHP 8.3, have tooling special case it and properly fix >>> it in PHP 8.4. >> >> And what about "code" like <https://3v4l.org/4CLhM>? Is Codesniffer >> supposed to scan the result of <https://3v4l.org/dKDcs> for possible CS >> violations? >> >> Cheers, >> Christoph > > I suppose you mean https://3v4l.org/IMi8Y, (you missed the <?php tag). > If you want to scan that, it's quite easy to strip the leading yield > and trailing from, and tokenize that again to extract all comments. > > Sure, it's a hack, but it'll work: https://3v4l.org/8eAiV. > > Bob >
Hi Bob, Of course, everything can be hacked around, but that still leaves the question what should be the "proper tokenization". Having this change in PHP 8.3 and then - as you suggest - yet another in PHP 8.4, makes it mighty hard to have a consistent token stream in tooling, especially as it is unclear what the "proper tokenization" should/would be. More than anything, I find it concerning that this change sets a precedent for tokens to include comments. Just as an example: what does this mean for the PHP 8.0 nullsafe object operator ? Should we now suddenly allow that to be written as `? /*comment*/ ->` ? Or what about a cast token ? Should that be allowed to be `(string /*for reasons*/)` ? Allowing this change to stay in, without having the discussion about what the "proper tokenization" should be, feels off and random to me and opens the door for more random changes. As for the impact on tooling: a change in the tokenization of any token has an impact not only on tooling like PHPCS itself, but also on every single external standard build on top of it and is a breaking change. To give you some perspective - for PHPCS we even went as far as to "undo" the PHP 8.0 tokenization of namespaced names for the time being (in the PHPCS 3.x releases) and we'll only change the PHPCS tokenizer to use the PHP 8.0 tokenization in the PHPCS 4.0 release as it would otherwise break too many existing sniffs. [1] Smile, Juliette 1: https://github.com/squizlabs/PHP_CodeSniffer/issues/3041

Tim Düsterhus

2 years ago
Hi On 7/19/24 07:22, Juliette Reinders Folmer wrote:
> More than anything, I find it concerning that this change sets a > precedent for tokens to include comments. > > Just as an example: what does this mean for the PHP 8.0 nullsafe object > operator ? Should we now suddenly allow that to be written as `? > /*comment*/ ->` ? > Or what about a cast token ? Should that be allowed to be `(string /*for > reasons*/)` ?
The difference between `yield from` and `?->` is that the former looks and feels like it would be two separate keywords, because of the *required* whitespace between the `yield` and the `from`. The fact that a `yield` keyword actually exists also contributes to that. `?->` on the other hand looks and feels like a single operator, just like `++`, `!==`, `<=>` and others. Except for `yield from` the rule where comments may be placed as far as I can tell is "comments may appear where whitespace may appear", which is easy enough to explain and understand. So it makes sense to allow for comments between `yield` and `from`, but I agree that ideally those would be emitted as separate tokens. Best regards Tim Düsterhus

Unnamed Person

2 years ago
On 20-7-2024 16:51, Tim Düsterhus wrote:
> Hi > > On 7/19/24 07:22, Juliette Reinders Folmer wrote: >> More than anything, I find it concerning that this change sets a >> precedent for tokens to include comments. >> >> Just as an example: what does this mean for the PHP 8.0 nullsafe object >> operator ? Should we now suddenly allow that to be written as `? >> /*comment*/ ->` ? >> Or what about a cast token ? Should that be allowed to be `(string /*for >> reasons*/)` ? > > The difference between `yield from` and `?->` is that the former looks > and feels like it would be two separate keywords, because of the > *required* whitespace between the `yield` and the `from`. The fact > that a `yield` keyword actually exists also contributes to that. `?->` > on the other hand looks and feels like a single operator, just like > `++`, `!==`, `<=>` and others. > > Except for `yield from` the rule where comments may be placed as far > as I can tell is "comments may appear where whitespace may appear", > which is easy enough to explain and understand. So it makes sense to > allow for comments between `yield` and `from`, but I agree that > ideally those would be emitted as separate tokens.
Tim, "comments may appear where whitespace may appear" ? You'd think so, except it isn't true. I already mentioned cast tokens before. Whitespace is perfectly acceptable within the parentheses of these. Comments are not: https://3v4l.org/A6Sgj and https://3v4l.org/nE9H8 Now you may argue that cast tokens "feel like" a single operator, but that's subjective and there's even a sniff to enforce no spacing within cast parentheses as apparently people do pad them with spaces - and doing so is allowed in PHP. * _* Qualifier: spaces and tabs are allowed inside cast parentheses, but new lines are not..._ Along the same lines and I'm beginning to repeat myself, the PHP 8.0 RFC which changed the tokenization of namespaced names explicitly disallowed whitespace and comments _inside_ namespaced names tokenized as a single token, while in the previous, multi-token situation, whitespace and comments were allowed in namespaced names. So to get back to my original point, as of PHP 8.3 is the **only** token which allows for a comment to be tokenized as part of the token. There is no other token which allows that. Whitespace is one thing, comments is a different matter. And even the whitespace is an interesting one as I've seen bug reports in PHPCS about a sniff breaking on `yield from` with a new line and indentation between the keywords. PHP allows this, the sniff in question does not handle it correctly. So, what "feels" natural (whitespace-wise) to one person may not be the same for the next, but comments _within_ tokens is different thing and should in my opinion, not be allowed. Smile, Juliette

Tim Düsterhus

2 years ago
Hi On 7/20/24 17:42, Juliette Reinders Folmer wrote:
> I already mentioned cast tokens before. Whitespace is perfectly > acceptable within the parentheses of these. Comments are not: > https://3v4l.org/A6Sgj and https://3v4l.org/nE9H8 > > Now you may argue that cast tokens "feel like" a single operator, but > that's subjective and there's even a sniff to enforce no spacing within > cast parentheses as apparently people do pad them with spaces - and > doing so is allowed in PHP. * > _* Qualifier: spaces and tabs are allowed inside cast parentheses, but > new lines are not..._
I stand corrected. Though that indeed is another odd special case with not allowing newlines. Looking at the lexer, there's another case where tabs and spaces may appear, but not other whitespace. Between '<<<' and the delimiter of a heredoc.
> So to get back to my original point, as of PHP 8.3 is the **only** token > which allows for a comment to be tokenized as part of the token. There > is no other token which allows that.
PHP users have no idea what a token is internally. I'm looking at this from a PHP user perspective. It looks like two keywords, it walks like two keywords and it quacks like two keywords. I find it reasonable for users to consider this as two keywords and not care about how it's implemented internally.
> So, what "feels" natural (whitespace-wise) to one person may not be the > same for the next, but comments _within_ tokens is different thing and > should in my opinion, not be allowed.
As I've said: I agree that the current situation is unfortunate. But the correct solution is not "disallow comments", but "split the T_YIELD_FROM into T_YIELD T_WHITESPACE T_FROM_FOR_YIELD_FROM". Best regards Tim Düsterhus

Unnamed Person

2 years ago
On 20-7-2024 18:04, Tim Düsterhus wrote:
> PHP users have no idea what a token is internally. I'm looking at this > from a PHP user perspective. It looks like two keywords, it walks like > two keywords and it quacks like two keywords. I find it reasonable for > users to consider this as two keywords and not care about how it's > implemented internally. > >> So, what "feels" natural (whitespace-wise) to one person may not be the >> same for the next, but comments _within_ tokens is different thing and >> should in my opinion, not be allowed. > > As I've said: I agree that the current situation is unfortunate. But > the correct solution is not "disallow comments", but "split the > T_YIELD_FROM into T_YIELD T_WHITESPACE T_FROM_FOR_YIELD_FROM". >
Tim, you're making my point for me. This is *exactly* why the current change should be reverted. I'm not against changing the tokenization of "yield from" and the GH ticket thread also contains an alternative proposal for this from Bob [1], but like Matthew also said [2]: if that's something we want to do, let's have a proper discussion about it and let it go through an RFC and be a documented change. And not, like it is now, an undocumented, random change creating an inconsistency in the Tokenizer. Smile, Juliette 1: https://github.com/php/php-src/issues/14926#issuecomment-2228855422 2: https://externals.io/message/124462#124515

Tim Düsterhus

2 years ago
Hi On 7/20/24 18:40, Juliette Reinders Folmer wrote:
> Tim, you're making my point for me. This is *exactly* why the current > change should be reverted.
I am not sure how you read "PHP users have no idea what a token is" as an argument in favor of reverting the change, because reverting the change means that completely reasonable code suddenly stops working with a parser error in a patch version and PHP users will rightfully come to PHP's issue tracker to complain.
> And not, like it is now, an undocumented, random change creating an > inconsistency in the Tokenizer.
The tokenizer is doing the right thing: It tokenizes the PHP source code. It is absolutely normal that PHP first and second-digit updates make changes to the token stream. New tokens are added, old tokens are removed, tokens may appear in places where they previously could not appear for well-formed PHP programs. Tools working on the token stream need to adapt and this change is no different from any other change to PHP's syntax in that regard (except that documenting the change was forgotten). Best regards Tim Düsterhus

Ilija Tovilo

2 years ago
Hi Juliette On Sat, Jul 20, 2024 at 6:40 PM Juliette Reinders Folmer <php-internals_nospam@adviesenzo.nl> wrote:
> > On 20-7-2024 18:04, Tim Düsterhus wrote: > > As I've said: I agree that the current situation is unfortunate. But the correct solution is not "disallow comments", but "split the T_YIELD_FROM into T_YIELD T_WHITESPACE T_FROM_FOR_YIELD_FROM". > > Tim, you're making my point for me. This is *exactly* why the current change should be reverted. > > I'm not against changing the tokenization of "yield from" and the GH ticket thread also contains an alternative proposal for this from Bob [1], but like Matthew also said [2]: if that's something we want to do, let's have a proper discussion about it and let it go through an RFC and be a documented change. > > And not, like it is now, an undocumented, random change creating an inconsistency in the Tokenizer.
Thank you for raising this issue. Let me share my viewpoint as the person who has made the change. First of all, while allowing comments between yield and from was obviously intentional, the side-effect of embedding the comment in the yield_from token was something I did not sufficiently consider. That's my bad. I agree that it's not very likely that many people started placing comments between yield and from, but certainly not impossible. The main problem with reverting this change is that such code will completely stop working. Patch updates are routinely installed on servers without additional testing, so it's very much possible that we would brick peoples production environments. To compare the impact on PHP_CodeSniffer: From what I understand (please correct me if I'm wrong), there are really only two possible scenarios when encountering comments between yield and from: * PHP_CodeSniffer doesn't see the comment and acts as if it weren't there, and potentially removes it when fixing errors automatically. * PHP_CodeSniffer thinks this is a syntax error and thus reports it. While that's not optimal, it is limited to a subset of the already small set of people using this new feature, namely the ones also using PHP_CodeSniffer. It is also limited to development environments, where PHP_CodeSniffer is executed. What I'd suggest instead is fixing this for master in a way that makes it simple for PHP_CodeSniffer to add support, or to just revert it there. If it is indeed true that nobody uses this feature after 8 months, then waiting another 4 for a proper fix doesn't seem like an issue. In the likely case of not finding consensus on this issue: Setting up a vote doesn't seem straight-forward to me either. Would we consider 8.3 the baseline, with reverting the change requiring the 2/3 majority, or do we consider 8.2 the baseline? Ilija

Deleu

2 years ago
> On 17 Jul 2024, at 20:16, Juliette Reinders Folmer <php-internals_nospam@adviesenzo.nl> wrote: > >  Hi all, > > I recently discovered a change was made to the Tokenizer in PHP 8.3.0 which now allows for a comment to exist between the `yield` and `from` keyword from the `yield from` keyword. > Before PHP 8.3, this was a parse error. > > This change was not documented (anywhere) nor publicized, which is why I only recently came across it and opened a bug report on GitHub about it [1]. > > This change is a breaking change for projects consuming the token stream, but more than that, it introduces an inconsistency in the Tokenizer as there is no other single token in PHP which can contain a comment (aside from comments tokens of course). > Comments were even explicitly forbidden in the PHP 8.0 RFC which changed the tokenization of namespaced names [2]. > Some more detailed explanation and examples can be found in the GH thread [3] and a detailed analysis of the tokenization change can be found in a ticket in the PHP_CodeSniffer repo [4] (details in a fold out in the comment). > > In my opinion the change is a bug and should be reverted, but opinions on this are divided. > After a discussion in the ticket on GitHub, it was suggested to ask for a third/fourth/fifth opinion here on the list. > > Pros for reverting it: > - Consistency with other tokens in the token stream, none of which allow comments within the token. > - Consistency with the 7 years before in which this was a parse error. > - The change was never documented, not in the changelog, news, nor in the upgrading guide. > > Cons against reverting it: > - The change has been in PHP 8.3 releases for a little over seven months. > - The odd few people may have accidentally discovered the bug and taken advantage by introducing code containing `yield /*comment*/ from` into their project, which with a revert would become a parse error again. > > Opinions ? > > Of course, the ticket yielded discussion on whether the tokenization of `yield from` should be changed anyhow, but please consider that discussion out of scope. > The current question is only about reverting the bug versus regarding it as a new feature and properly documenting it. > > Smile, > Juliette > > > 1: https://github.com/php/php-src/issues/14926 > 2: https://wiki.php.net/rfc/namespaced_names_as_token#backward_incompatible_changes > 3: https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/529#issuecomment-2209337419 > 4: https://github.com/php/php-src/issues/14926#issuecomment-2227152061 >
My opinion would be to revert it. Chances of someone accidentally discovering and actually using it seems unlikely at best. Even then, the parser error could be easily resolved by moving the comment out of the middle of the token with insignificant readability change. Forcing all tooling that uses token_get_all() to handle this unintentional change seems to generate more unnecessary and real busywork for something only theoretical possible to break.

Tim Düsterhus

2 years ago
Hi On 7/18/24 19:48, Marco Aurélio Deleu wrote:
> Forcing all tooling that uses token_get_all() to handle this unintentional change seems to generate more unnecessary and real busywork for something only theoretical possible to break.
The tools are required to handle this either way, because there are released version with this specific tokenization and they are not going away. Ubuntu 24.04 LTS ships with PHP 8.3.6 and generally Ubuntu backports security fixes instead of upgrading to newer patch versions. As an example, Ubuntu 22.04 LTS ships with PHP 8.1.2 + security fixes, not with 8.1.29 (which is the newest 8.1.x as of now). Thus the ship has effectively sailed due to the inclusion in Ubuntu 24.04 LTS as the arguably most widely used Linux distro. Best regards Tim Düsterhus

Deleu

2 years ago
I can't speak for Juliette's plans but I would advocate for pinning PHP 8.3.{fixed} on Composer and having an error message for PHAR. It's also worth mentioning that just because it's possible to add comments in those versions doesn't mean we must assume that it wi be done and it must be supported. If a Ubuntu LTS user gets an error, pointing out that they're relying on an accidental behavior only present in 9 patch releases of the entire PHP lifecycle seems good enough.

Christoph Becker

2 years ago
Hi Tim! On 18.07.2024 at 21:05, Tim Düsterhus wrote:
> On 7/18/24 19:48, Marco Aurélio Deleu wrote: > >> Forcing all tooling that uses token_get_all() to handle this >> unintentional change seems to generate more unnecessary and real >> busywork for something only theoretical possible to break. > > The tools are required to handle this either way, because there are > released version with this specific tokenization and they are not going > away.
Well, these tools could reject such code, and tell users to update to a version where this is no longer valid syntax.
> Ubuntu 24.04 LTS ships with PHP 8.3.6 and generally Ubuntu backports > security fixes instead of upgrading to newer patch versions. As an > example, Ubuntu 22.04 LTS ships with PHP 8.1.2 + security fixes, not > with 8.1.29 (which is the newest 8.1.x as of now). > > Thus the ship has effectively sailed due to the inclusion in Ubuntu > 24.04 LTS as the arguably most widely used Linux distro.
I have to agree that this is a strong argument, but I don't think we're absolutely obliged to stick with what have right now. We still can claim that we've made an unintended behavioral change, aka. introduced a bug, and fix it – downstream projects have to deal with this (the same as we may have to deal with unwelcome upstream fixes). And frankly, how much code would be affected? I mean, does anybody actually put a comment between `yield` and `from`? Is there a case where this may make sense? "Because we can" isn't a strong argument, in my opinion. Cheers, Christoph

Tim Düsterhus

2 years ago
Hi On 7/19/24 00:51, Christoph M. Becker wrote:
> And frankly, how much code would be affected? I mean, does anybody > actually put a comment between `yield` and `from`? Is there a case > where this may make sense? "Because we can" isn't a strong argument, in > my opinion.
I don't really follow this line of argumentation: If folks do not use the syntax anyways, then we do not need to have this discussion, because the tools can just ignore it existing. That also means we do not need to revert the change in PHP. If folks use the syntax, then reverting the change is a breaking change for them. So either the revert is not doing anything at all, or the revert is actively harmful. I do not see a situation where reverting the change is a value-add. Best regards Tim Düsterhus

Deleu

2 years ago
> On 20 Jul 2024, at 11:30, Tim Düsterhus <tim@bastelstu.be> wrote: > > Hi > >> On 7/19/24 00:51, Christoph M. Becker wrote: >> And frankly, how much code would be affected? I mean, does anybody >> actually put a comment between `yield` and `from`? Is there a case >> where this may make sense? "Because we can" isn't a strong argument, in >> my opinion. > > I don't really follow this line of argumentation: > > If folks do not use the syntax anyways, then we do not need to have this discussion, because the tools can just ignore it existing. That also means we do not need to revert the change in PHP. > > If folks use the syntax, then reverting the change is a breaking change for them. > > So either the revert is not doing anything at all, or the revert is actively harmful. I do not see a situation where reverting the change is a value-add. > > Best regards > Tim Düsterhus
The value add of the revert is because time is a moving target. We don't think anyone is using it _yet_, given the circumstances that made this happen. Wait long enough and the only guarantee we have is entropy.

Rob Landers

2 years ago
On Sat, Jul 20, 2024, at 16:35, Marco Aurélio Deleu wrote:
> > > On 20 Jul 2024, at 11:30, Tim Düsterhus <tim@bastelstu.be> wrote: > > > > Hi > > > >> On 7/19/24 00:51, Christoph M. Becker wrote: > >> And frankly, how much code would be affected? I mean, does anybody > >> actually put a comment between `yield` and `from`? Is there a case > >> where this may make sense? "Because we can" isn't a strong argument, in > >> my opinion. > > > > I don't really follow this line of argumentation: > > > > If folks do not use the syntax anyways, then we do not need to have this discussion, because the tools can just ignore it existing. That also means we do not need to revert the change in PHP. > > > > If folks use the syntax, then reverting the change is a breaking change for them. > > > > So either the revert is not doing anything at all, or the revert is actively harmful. I do not see a situation where reverting the change is a value-add. > > > > Best regards > > Tim Düsterhus > > The value add of the revert is because time is a moving target. We don't think anyone is using it _yet_, given the circumstances that made this happen. Wait long enough and the only guarantee we have is entropy.
Even if people are using it, if fixing it is better than leaving it… Oh well. A perfect example is the GMP class being left not “final” that allows for some really nice semantics. It’s currently being voted on and it appears an unanimous vote to make it final will pass. The language can and will change. Sometimes in ways we don’t like. — Rob

Matthew Weier O'Phinney

2 years ago
On Sat, Jul 20, 2024, 9:31 AM Tim Düsterhus <tim@bastelstu.be> wrote:
> Hi > > On 7/19/24 00:51, Christoph M. Becker wrote: > > And frankly, how much code would be affected? I mean, does anybody > > actually put a comment between `yield` and `from`? Is there a case > > where this may make sense? "Because we can" isn't a strong argument, in > > my opinion. > > I don't really follow this line of argumentation: > > If folks do not use the syntax anyways, then we do not need to have this > discussion, because the tools can just ignore it existing. That also > means we do not need to revert the change in PHP. > > If folks use the syntax, then reverting the change is a breaking change > for them. >
By this line of reasoning, any bug that makes it into the engine that somebody finds a use for shouldn't be fixed. I totally understand wanting to keep this in, but because it made it in without an RFC, it's (a) inconsistent with other areas of the language, and (b) does not have a full design which includes tokenizer support. Considering the brief time it's been in the engine, and the fact that it's not documented, I feel it should be reverted and only re-added if someone takes the time and effort to propose the change properly.