Re: [PHP-DEV][RFC][VOTE] Rename T_PAAMAYIM_NEKUDOTAYIM to T_DOUBLE_COLON

2020-06-27 Thread Stanislav Malyshev
Hi!

> Better error messages are obviously better than just replacing the name of
> the token, however this argument is saying that because this isn't perfect
> let's do nothing.

I don't think this is the argument. I think the argument is rather than
half-fix the problem wrong way, let's fully fix it the right way.

> I find it highly frustrating, and borderline offensive, that we are being
> asked to go from a simple, non BC breaking, easy to enact change, to a
> semi-major overhaul of how error messages should look like. I personally

I am not sure why is this "offensive" to you. Is there something in
overhauling error messages that goes against your principles or
religious beliefs? Sometimes overhauling is exactly what is the right
way to go, and not showing internal parser tokens seems to be quite
reasonable idea. If this idea is somehow "offensive" to you, I feel
sorry for you but it's not a reason to abandon this idea. It is also
quite a common things - many proposals in PHP have been rejected because
the community thought it's not the right way to approach things, I
myself have had some proposals rejected because of this. There's no
reason to feel offended by that.

> My perception is that most of the community finds it baffling why anyone
> would be against this change.

My perception is that most of the community couldn't care less for how
the tokens are named, and really shouldn't. It's an internal thing and
should stay this way. If they made to care, that's our fault and we
should fix it.

-- 
Stas Malyshev
smalys...@gmail.com

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV][RFC][VOTE] Rename T_PAAMAYIM_NEKUDOTAYIM to T_DOUBLE_COLON

2020-06-27 Thread G. P. B.
Hello Andrea, Benjamin, and others,

Better error messages are obviously better than just replacing the name of
the token, however this argument is saying that because this isn't perfect
let's do nothing.

I wasn't aware that Rowan had made good progress on his patch, however, if
this patch needs an RFC, which it might not, there is still no guarantee on
having better error messages for PHP 8.0.

Now, this may just me, but I find the argument that "::" is present
relatively weak as when I scan a PHP parse error, the first thing I look
for is the token name as there are only 3 parse errors which have the added
information of what the token represents, namely T_SL, T_SR, and
T_PAAMAYIM_NEKUDOTAYIM. T_SL and T_SR also have this issue but I would
expect beginners to not hit this as often as I don't expect them to do
anything with bit shifts, and the only other way that they may encounter it
is by doing a bad git merge.

I find it highly frustrating, and borderline offensive, that we are being
asked to go from a simple, non BC breaking, easy to enact change, to a
semi-major overhaul of how error messages should look like. I personally
have no interest in learning Bison and how to implement that as a separate
improvement to PHP. I'm just glad that Rowan decided to take on this
challenge.

Even if better error messages come about, some people will still need to
deal with the token, such as static analyser, code sniffers, code style
tools, etc. Obviously people working on these tools aren't beginners and
deal with the weird token name just fine, or use the T_DOUBLE_COLON alias
if they can. But why should it still be like this? This change has no BC
break, makes English the consistent language for token names. Moreover,
something being historic doesn't mean it shouldn't be touched.

My perception is that most of the community finds it baffling why anyone
would be against this change.

On Sat, 27 Jun 2020 at 15:57, Andrea Faulds  wrote:

> As for parser errors, I don't know how easy they would be to improve… is
> it even possible for us to do so without using a hand-written parser
> instead of an auto-generated one? (I have no idea.)

This may be done using Bison 3.6, which got released in May of this year,
as seen by this PR: https://github.com/php/php-src/pull/5416 However, I
don't expect us to be able to use this as Bison 3.6, won't be present on
most distrib until a couple of years.

Best regards
George P. Banyard


Re: [PHP-DEV][RFC][VOTE] Rename T_PAAMAYIM_NEKUDOTAYIM to T_DOUBLE_COLON

2020-06-27 Thread Dan Ackroyd
On Sat, 27 Jun 2020 at 11:20, Benjamin Eberlei  wrote:

> 1. The token name has historic value and should be preserved as such

I don't think it has historic value. It might have nostalgic value but
nostalgia for:

> I think everyone hits it once or twice, has the "wtf?" moment, googles, and
> never forgets it again - even if they can't pronounce it...

is not a great piece of nostalgia.

But regardless of the past value, it's the current and future
trade-offs we should be considering.

i.e. is the cost of changing it bigger or smaller than the benefit of
not having a token name that can only be reasoned about after
googling.

I haven't seen anyone say any cost for changing it. Even if the work
is done to improve the error messages, avoiding having a token that
you can't grok the meaning from the name of it, is a small
improvement.

So the tradeoff appears to be very small or zero cost, versus a
benefit for everyone who hasn't already stumbled into it as a blocker
for their understanding of internals.

cheers
Dan
Ack

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Trait constants

2020-06-27 Thread tyson andre
> Can anyone working on internals explain why traits don’t allow constants 
> (either technically or philosophically)?
> Moreover, what’s the opinion(s) of the list, on adding support for this? 
> Would an RFC be needed? 

I don't think there's any insurmountable obstacles, but it would be useful to 
add a lot of tests because of potential edge cases (resolution, opcache, 
reference counting)
involved in traits and adding new places where constants can go.

(I'd also worked on a declined RFC and implementation for changing what could 
be used in a constant expression)

```
getMessage() . "\n";
}
D::main();
/*
Output:

from c
Undefined class constant 'self::OTHER_CONST'
>From D
 */
```

- Tyson
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Trait constants

2020-06-27 Thread tyson andre
> It’s always struck me as slightly odd that traits don’t support constants the 
> way classes and interfaces do.
> I tried to find an explanation of the lack of support in the original RFC, 
> and came up empty.
> 
> A consequent discussion in R11 has led me here.
> Can anyone working on internals explain why traits don’t allow constants 
> (either technically or philosophically)?
> Moreover, what’s the opinion(s) of the list, on adding support for this? 
> Would an RFC be needed? 

I'm in favor of adding it. I doubt there's any insurmountable technical 
obstacles.
(I work on a static analyzer for php (https://github.com/phan/phan) and don't 
expect technical issues with that proposal)
I personally find it inconvenient to put constants used by traits in different 
classes/interfaces or in properties.

Other languages allow adding constants to traits:

- Java allows defining constants on interfaces, which allow defining default 
method implementations like PHP traits
- 
https://doc.rust-lang.org/edition-guide/rust-2018/trait-system/associated-constants.html
 allows defining constants with values on traits
  (it also does various other things that impractical for php)

One thing to document/add tests for would be the resolution of `self::MY_CONST`.
In a trait, `self::method()` may be overridden by the method of the class 
that's using the trait.
I'd expect `self::MY_CONST` to be the same.

```php
https://www.php.net/manual/en/language.oop5.traits.php#language.oop5.traits.conflict
  (e.g. `use A, B { const A::MY_CONST insteadof B; }`)
- We already check if there are conflicting values when inheriting constants 
from an interface and another interface/trait.

Cheers,
- Tyson
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] throwable_string_param_max_len: Configurable string length in getTraceAsString()

2020-06-27 Thread tyson andre
Hi Dan Ack,

> Also, I didn't understand why there was a problem with formatting
> traces in userland. I saw a link to some code, but no clear
> description of what the problem was.

I expanded the description of how `getTraceAsString()` might be improperly used 
in existing code and moved it to
https://wiki.php.net/rfc/throwable_string_param_max_len#impact_of_raising_string_param_length_limit
(and how the code would already be unsafe, but raising the limit may make the 
impact of unsafe code like that worse)

(e.g. if the code or dependency may `echo $exception` if it was written by an 
author unaware of potential https://en.wikipedia.org/wiki/Cross-site_scripting 
or sensitive data exposure)

This is mostly included to explain why I don't propose raising the defaults in 
the RFC

- Tyson
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] throwable_string_param_max_len: Configurable string length in getTraceAsString()

2020-06-27 Thread tyson andre
> >
> > I've created a new RFC 
> > https://wiki.php.net/rfc/throwable_string_param_max_len
> 
> How come you're proposing an ini setting instead of adding a parameter
> to getTraceAsString() that specifies the length for params?

I don't expect many people to use a parameter for getTraceAsString(),
and that alternative doesn't help with implicit `__toString()` or the default 
output for fatal throwables.

The application developer and end user may have different ideas of what to use 
as a length.
If an application would manually use 
`getTraceAsString(Config::getMaxStringLengthForEnvironment())`,
it'd be much shorter as an ini setting.

> Also, I didn't understand why there was a problem with formatting
> traces in userland. I saw a link to some code, but no clear
> description of what the problem was.

If there's an uncaught exception and no exception handler,
you'll get it truncated to 15 bytes.
Setting up a more complicated solution to format traces of uncaught traces in 
userland is probably not worth the effort for short php scripts,
and many people would call getTraceAsString() over getTrace() for the 
convenience (especially in rarely used code paths).

If you're an end user of an application or composer library,
patching the code to format traces in userland is inconvenient if the 
application developers are using stringified exceptions or getTraceAsString().

- Tyson
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] throwable_string_param_max_len: Configurable string length in getTraceAsString()

2020-06-27 Thread Dan Ackroyd
On Sat, 27 Jun 2020 at 16:16, tyson andre  wrote:
>
> I've created a new RFC https://wiki.php.net/rfc/throwable_string_param_max_len

How come you're proposing an ini setting instead of adding a parameter
to getTraceAsString() that specifies the length for params?

Also, I didn't understand why there was a problem with formatting
traces in userland. I saw a link to some code, but no clear
description of what the problem was.

cheers
Dan
Ack

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] [RFC] \PHP namespace usage heuristics

2020-06-27 Thread Mike Schinkel


> On Jun 23, 2020, at 8:30 PM, Larry Garfield  wrote:
> 
> Greetings, Internalians.
> 
> There has been much talk of the \PHP namespace of late, including one 
> unsuccessful RFC.  In the discussion, the pushback broke down into two main 
> camps:
> 
> * We should never namespace anything ever.
> * We can namespace things but we need something more concrete than "RFCs can 
> namespace things if they feel like it."
> 
> I can't do much about the former, but the latter is a solvable problem.  To 
> that end, Mark Randall and I have put together a new RFC on the topic, based 
> on a fruitful discussion in Room 11 a few weeks ago to brainstorm what actual 
> guidelines should be for what goes where.
> 
> https://wiki.php.net/rfc/php_namespace_policy
> 
> This proposal provides guidance to short circuit future subjective 
> bikeshedding, while still leaving some wiggle room for case-by-case 
> evaluation as needed.  That makes it different from prior attempts that did 
> not provide clear guidance for future RFC authors.
> 
> The specific guidelines offered may or may not appeal to you; those are open 
> to discussion (within reason; we don't want to end up back in "do whatever" 
> land as we know that won't help), but the more important point is that clear 
> guidelines are provided.
> 
> Also of note, although it uses existing code to demonstrate where classes 
> *would* go under this plan it does not immediately move anything.  Those are 
> left for future RFCs that would have to stand or fall on their own merit.  It 
> also provides for a very long grace period for any such transitions to 
> minimize disruption.
> 
> The intent is to bring this proposal to a vote in time for 8.0's freeze one 
> way or another, even though it's unlikely to have any impact on 8.0 itself.  
> It's still a convenient deadline.
> 
> *dons flame retardant suit*
> 

This looks really good Larry. Very well thought-out. 

If I could vote it would be a definitive "Yes."

-Mike

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] Trait constants

2020-06-27 Thread Mike Schinkel
> On Jun 27, 2020, at 9:52 AM, Stephen Reay  wrote:
> 
> Hi,
> 
> It’s always struck me as slightly odd that traits don’t support constants the 
> way classes and interfaces do.
> I tried to find an explanation of the lack of support in the original RFC, 
> and came up empty.
> 
> A consequent discussion in R11 has led me here.
> Can anyone working on internals explain why traits don’t allow constants 
> (either technically or philosophically)?
> Moreover, what’s the opinion(s) of the list, on adding support for this? 


Yes.  Please.


-Mike

[PHP-DEV] [RFC] throwable_string_param_max_len: Configurable string length in getTraceAsString()

2020-06-27 Thread tyson andre
Hi internals,

I've created a new RFC https://wiki.php.net/rfc/throwable_string_param_max_len

Since 2003, `Throwable->getTraceAsString()` and `Throwable->__toString()`
have limited the length of string function arguments in stringified stack 
traces to 15 bytes
(e.g. `#0 /path/to/file.php(line) function(“012345678901234…”)`.
This is not enough space to render information such as paths, URLs, UUIDs, etc. 
if an end user wants to see them when debugging an issue or developing locally.

This proposes a new ini setting `throwable_string_param_max_len` that would 
allow changing the string byte limit to any value between 15 and 100, 
keeping the current default of 15 bytes.

Earlier discussion can be found in https://externals.io/message/110717
(Making the hardcoded string length limit of Throwable->getTraceAsString() 
configurable)

Thanks,
- Tyson
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV][RFC][VOTE] Rename T_PAAMAYIM_NEKUDOTAYIM to T_DOUBLE_COLON

2020-06-27 Thread Rowan Tommins

On 27/06/2020 10:56, Andrea Faulds wrote:
Of course, if T_PAAMAYIM_NEKUDOTAYIM was never encountered by userland 
developers, this RFC wouldn't exist. The thing is, I don't think 
T_DOUBLE_COLON should be encountered by userland developers either — 
in my view, as an implementation detail, token names shouldn't be part 
of parser error messages at all. If we were to remove token names from 
the parser errors, we would avoid the problem this RFC seeks to solve. 
For most tokens we could simply display the characters it corresponds 
to (e.g. "::" for T_PAAMAYIM_NEKUDOTAYIM, which we already do!), and 
for those with variable content (e.g. T_STRING) we could display a 
human-readable description of what is expected (e.g. "an identifier"). 



Just to confirm, I am actively working on exactly this, and although 
slightly delayed by an outbreak of sunny weather, fully expect to have a 
patch (and, if deemed necessary, RFC) in plenty of time for 8.0.


I intend to post a new thread with examples of old and new messages once 
I've finalised the details.



Regards,

--
Rowan Tommins (né Collins)
[IMSoP]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV][RFC][VOTE] Rename T_PAAMAYIM_NEKUDOTAYIM to T_DOUBLE_COLON

2020-06-27 Thread Andrea Faulds

Hi again,

A further and perhaps more important thought: I think the token names 
are actually the least confusing part of parser errors, even for the 
famous T_PAAMAYIM_NEKUDOTAYIM. Changing it to T_DOUBLE_COLON may not 
help much, because the parser only tells you what the next token it 
expected was, not *why* it expected it, i.e. what is wrong with the 
syntax. A user might think they need to add a :: but it's not their 
actual problem.


For example, if you google for “T_PAAMAYIM_NEKUDOTAYIM” errors, one of 
the classic examples where you got such an error was:


  var_dump(empty(TRUE));

If the error had said T_DOUBLE_COLON it would still be mystifying: why 
did PHP think you needed a :: there? And just adding one won't fix the 
problem! The actual issue was that empty() used to not support arbitrary 
expressions, only variables, and the expected T_PAAMAYIM_NEKUDOTAYIM is 
because the only way TRUE could be part of a variable would be if it was 
a class name (TRUE::$foo). The way to fix it is to replace empty() with 
the ! operator, but you'd have a hard time figuring that out from the 
error. I think this is the real reason T_PAAMAYIM_NEKUDOTAYIM was 
famous: even if you knew it meant double colon, the error message is 
still cryptic.


The good news is T_PAAMAYIM_NEKUDOTAYIM is no longer quite the menace it 
once was. PHP 7's parser and syntax overhaul (thank you Nikita!) fixed 
it in some places:


  $ php -r 'var_dump(isset(TRUE));'
  Fatal error: Cannot use isset() on the result of an expression (you 
can use "null !== expression" instead)


And other places where you might have once seen T_PAAMAYIM_NEKUDOTAYIM 
now give a different unhelpful parser error, which renaming 
T_PAAMAYIM_NEKUDOTAYIM will not help with:


  $ php -r 'unset(TRUE);'
  Parse error: syntax error, unexpected ')', expecting '[' in Command 
line code on line 1


So if there was ever a time to rename T_PAAMAYIM_NEKUDOTAYIM, it would 
have been many years ago. There's much less benefit to renaming it now, 
especially given it says “'::' (T_PAAMAYIM_NEKUDOTAYIM)” if you manage 
to get an error containing it, so you don't even need to google it. The 
specific name a token is given is the least of the problems there.


As for parser errors, I don't know how easy they would be to improve… is 
it even possible for us to do so without using a hand-written parser 
instead of an auto-generated one? (I have no idea.)


Regards,
Andrea


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



[PHP-DEV] Trait constants

2020-06-27 Thread Stephen Reay
Hi,

It’s always struck me as slightly odd that traits don’t support constants the 
way classes and interfaces do.
I tried to find an explanation of the lack of support in the original RFC, and 
came up empty.

A consequent discussion in R11 has led me here.
Can anyone working on internals explain why traits don’t allow constants 
(either technically or philosophically)?
Moreover, what’s the opinion(s) of the list, on adding support for this? Would 
an RFC be needed? 


Cheers 



Stephen  
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV][RFC][VOTE] Rename T_PAAMAYIM_NEKUDOTAYIM to T_DOUBLE_COLON

2020-06-27 Thread Nikita Popov
On Sat, Jun 27, 2020 at 11:57 AM Andrea Faulds  wrote:

> Hi,
>
> G. P. B. wrote:
> > https://wiki.php.net/rfc/rename-double-colon-token
>
> I have voted No to this, and I hope I can convince some others to do the
> same.
>
> T_PAAMAYIM_NEKUDOTAYIM is such a famous token that there is probably
> nobody in internals who doesn't know what it means, and for new
> contributors, it is easy to find the definition, and note that it is
> hardly the only token name that they will need to look up. It is also a
> fun nod to the history of PHP, and I think it would be a shame to lose
> that.
>
> I mention the internals usage first and foremost, because it should be
> remembered that token names are merely an implementation detail of the
> PHP interpreter, unless you're using token_get_all (which by the way
> already has the alias T_DOUBLE_COLON). In other words, it's not
> something the vast majority of userland developers should ever encounter
> or have to think about.
>
> Of course, if T_PAAMAYIM_NEKUDOTAYIM was never encountered by userland
> developers, this RFC wouldn't exist. The thing is, I don't think
> T_DOUBLE_COLON should be encountered by userland developers either — in
> my view, as an implementation detail, token names shouldn't be part of
> parser error messages at all. If we were to remove token names from the
> parser errors, we would avoid the problem this RFC seeks to solve. For
> most tokens we could simply display the characters it corresponds to
> (e.g. "::" for T_PAAMAYIM_NEKUDOTAYIM, which we already do!), and for
> those with variable content (e.g. T_STRING) we could display a
> human-readable description of what is expected (e.g. "an identifier").
>
> I think the case for not renaming T_PAAMAYIM_NEKUDOTAYIM, and instead
> improving the error messages, is stronger when you consider that is not
> the only token with a name that might confuse people outside internals.
> For example, T_STRING is a very common token, but the name is probably
> going to surprise most userland developers who encounter it in an error
> message, because it doesn't mean a literal string. Even for tokens with
> more conventional names, it is unnecessary extra information. I think
> renaming just T_PAAMAYIM_NEKUDOTAYIM is not a full solution to the
> problem this RFC intends to solve.
>
> Apropos of that:
>
>  > We did acknowledge the suggestion of dropping the token name from the
>  > error message directly, but in our opinion this is an orthogonal
>  > change to the one proposed, and has the risk of not landing in PHP
>  > 8.0.
>
> Is PHP 8.0 an all-important? If we _don't_ rename the tokens, but simply
> improve the error message, that might be allowable in a patch release
> (e.g. 8.0.1).
>
> (I also don't think we should rush things if we are unsure about them,
> given the consequences that has had in the past.)
>
> Thanks,
> Andrea
>

Hey Andrea,

You convinced me! To vote "yes" on this RFC :P

I initially thought that this proposal is simply redundant, because the
issue would be fully addressed by

a) not displaying token names in errors, which I expect to happen for PHP
8.0, and
b) renaming T_PAAMAYIM_NEKUDOTAYIM as far as internal usage is concerned,
which is an implementation detail on which internals@ input is not required,

but I now realize that there might be friction on point b) without this
RFC, so it is better to accept it.

Regards,
Nikita


Re: [PHP-DEV][RFC][VOTE] Rename T_PAAMAYIM_NEKUDOTAYIM to T_DOUBLE_COLON

2020-06-27 Thread Benjamin Eberlei
On Sat, Jun 27, 2020 at 11:57 AM Andrea Faulds  wrote:

> Hi,
>
> G. P. B. wrote:
> > https://wiki.php.net/rfc/rename-double-colon-token
>
> I have voted No to this, and I hope I can convince some others to do the
> same.
>
> T_PAAMAYIM_NEKUDOTAYIM is such a famous token that there is probably
> nobody in internals who doesn't know what it means, and for new
> contributors, it is easy to find the definition, and note that it is
> hardly the only token name that they will need to look up. It is also a
> fun nod to the history of PHP, and I think it would be a shame to lose
> that.
>
> I mention the internals usage first and foremost, because it should be
> remembered that token names are merely an implementation detail of the
> PHP interpreter, unless you're using token_get_all (which by the way
> already has the alias T_DOUBLE_COLON). In other words, it's not
> something the vast majority of userland developers should ever encounter
> or have to think about.
>
> Of course, if T_PAAMAYIM_NEKUDOTAYIM was never encountered by userland
> developers, this RFC wouldn't exist. The thing is, I don't think
> T_DOUBLE_COLON should be encountered by userland developers either — in
> my view, as an implementation detail, token names shouldn't be part of
> parser error messages at all. If we were to remove token names from the
> parser errors, we would avoid the problem this RFC seeks to solve. For
> most tokens we could simply display the characters it corresponds to
> (e.g. "::" for T_PAAMAYIM_NEKUDOTAYIM, which we already do!), and for
> those with variable content (e.g. T_STRING) we could display a
> human-readable description of what is expected (e.g. "an identifier").
>
> I think the case for not renaming T_PAAMAYIM_NEKUDOTAYIM, and instead
> improving the error messages, is stronger when you consider that is not
> the only token with a name that might confuse people outside internals.
> For example, T_STRING is a very common token, but the name is probably
> going to surprise most userland developers who encounter it in an error
> message, because it doesn't mean a literal string. Even for tokens with
> more conventional names, it is unnecessary extra information. I think
> renaming just T_PAAMAYIM_NEKUDOTAYIM is not a full solution to the
> problem this RFC intends to solve.
>
> Apropos of that:
>
>  > We did acknowledge the suggestion of dropping the token name from the
>  > error message directly, but in our opinion this is an orthogonal
>  > change to the one proposed, and has the risk of not landing in PHP
>  > 8.0.
>
> Is PHP 8.0 an all-important? If we _don't_ rename the tokens, but simply
> improve the error message, that might be allowable in a patch release
> (e.g. 8.0.1).
>
> (I also don't think we should rush things if we are unsure about them,
> given the consequences that has had in the past.)
>

This is the same reason that I voted no.

1. The token name has historic value and should be preserved as such
2. as several people suggested in the original RFC discussion, we should
just not display tokens in error messages anymore. That should have been
the RFCs solution, not renaming the token

>
> Thanks,
> Andrea
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>


Re: [PHP-DEV][RFC][VOTE] Rename T_PAAMAYIM_NEKUDOTAYIM to T_DOUBLE_COLON

2020-06-27 Thread Andrea Faulds

Hi,

G. P. B. wrote:

https://wiki.php.net/rfc/rename-double-colon-token


I have voted No to this, and I hope I can convince some others to do the 
same.


T_PAAMAYIM_NEKUDOTAYIM is such a famous token that there is probably 
nobody in internals who doesn't know what it means, and for new 
contributors, it is easy to find the definition, and note that it is 
hardly the only token name that they will need to look up. It is also a 
fun nod to the history of PHP, and I think it would be a shame to lose 
that.


I mention the internals usage first and foremost, because it should be 
remembered that token names are merely an implementation detail of the 
PHP interpreter, unless you're using token_get_all (which by the way 
already has the alias T_DOUBLE_COLON). In other words, it's not 
something the vast majority of userland developers should ever encounter 
or have to think about.


Of course, if T_PAAMAYIM_NEKUDOTAYIM was never encountered by userland 
developers, this RFC wouldn't exist. The thing is, I don't think 
T_DOUBLE_COLON should be encountered by userland developers either — in 
my view, as an implementation detail, token names shouldn't be part of 
parser error messages at all. If we were to remove token names from the 
parser errors, we would avoid the problem this RFC seeks to solve. For 
most tokens we could simply display the characters it corresponds to 
(e.g. "::" for T_PAAMAYIM_NEKUDOTAYIM, which we already do!), and for 
those with variable content (e.g. T_STRING) we could display a 
human-readable description of what is expected (e.g. "an identifier").


I think the case for not renaming T_PAAMAYIM_NEKUDOTAYIM, and instead 
improving the error messages, is stronger when you consider that is not 
the only token with a name that might confuse people outside internals. 
For example, T_STRING is a very common token, but the name is probably 
going to surprise most userland developers who encounter it in an error 
message, because it doesn't mean a literal string. Even for tokens with 
more conventional names, it is unnecessary extra information. I think 
renaming just T_PAAMAYIM_NEKUDOTAYIM is not a full solution to the 
problem this RFC intends to solve.


Apropos of that:

> We did acknowledge the suggestion of dropping the token name from the
> error message directly, but in our opinion this is an orthogonal
> change to the one proposed, and has the risk of not landing in PHP
> 8.0.

Is PHP 8.0 an all-important? If we _don't_ rename the tokens, but simply 
improve the error message, that might be allowable in a patch release 
(e.g. 8.0.1).


(I also don't think we should rush things if we are unsure about them, 
given the consequences that has had in the past.)


Thanks,
Andrea

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php