[RFC] Amendments to Attributes

  110217
May 20, 2020 17:07 kontakt@beberlei.de (Benjamin Eberlei)
Hi everyone,

the Attributes RFC was rather large already, so a few things were left open
or discussions during the vote have made us rethink a things.

https://wiki.php.net/rfc/attribute_amendments

These points are handled by the Amendments RFC to Attributes:

1. Proposing to add a grouped syntax <
2. Rename PhpAttribute to Attribute in global namespace (independent of the
namespace RFC)
3. Add validation of attribute class targets, which internal attributes can
do, but userland can't
4. Specification if an attribute is repeatable or not on the same
declaration and fail otherwise.

Each of them is a rather small issue, so I hope its ok to aggregate all
four of them in a single RFC. Please let me know if it's not.

greetings
Benjamin
  110218
May 20, 2020 17:36 ocramius@gmail.com (Marco Pivetta)
Hey Benjamin,
<http://ocramius.github.com/>


On Wed, May 20, 2020 at 7:08 PM Benjamin Eberlei <kontakt@beberlei.de>
wrote:

> Hi everyone, > > the Attributes RFC was rather large already, so a few things were left open > or discussions during the vote have made us rethink a things. > > https://wiki.php.net/rfc/attribute_amendments > > These points are handled by the Amendments RFC to Attributes: > > 1. Proposing to add a grouped syntax < > 2. Rename PhpAttribute to Attribute in global namespace (independent of the > namespace RFC) > 3. Add validation of attribute class targets, which internal attributes can > do, but userland can't > 4. Specification if an attribute is repeatable or not on the same > declaration and fail otherwise. > > Each of them is a rather small issue, so I hope its ok to aggregate all > four of them in a single RFC. Please let me know if it's not. >
Do you hope to get nested attributes to 8.0, or is it something you'd prefer to happen at a later time? Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.com/
  110223
May 20, 2020 21:31 kontakt@beberlei.de (Benjamin Eberlei)
On Wed, May 20, 2020 at 7:37 PM Marco Pivetta <ocramius@gmail.com> wrote:

> Hey Benjamin, > <http://ocramius.github.com/> > > > On Wed, May 20, 2020 at 7:08 PM Benjamin Eberlei <kontakt@beberlei.de> > wrote: > >> Hi everyone, >> >> the Attributes RFC was rather large already, so a few things were left >> open >> or discussions during the vote have made us rethink a things. >> >> https://wiki.php.net/rfc/attribute_amendments >> >> These points are handled by the Amendments RFC to Attributes: >> >> 1. Proposing to add a grouped syntax < >> 2. Rename PhpAttribute to Attribute in global namespace (independent of >> the >> namespace RFC) >> 3. Add validation of attribute class targets, which internal attributes >> can >> do, but userland can't >> 4. Specification if an attribute is repeatable or not on the same >> declaration and fail otherwise. >> >> Each of them is a rather small issue, so I hope its ok to aggregate all >> four of them in a single RFC. Please let me know if it's not. >> > > > Do you hope to get nested attributes to 8.0, or is it something you'd > prefer to happen at a later time? >
Martin evaluated the technical requirements in more detail and we discussed with others that the timeframe is probably to short to get all the details fleshed out :-( So we are going to table that for 8.1
  110224
May 20, 2020 21:34 benas.molis.iml@gmail.com (Benas IML)
Hello,

Would it possible to make a separate RFC for renaming `PhpAttribute` to
`Attribute` and `PhpCompilerAttribute` to `CompilerAttribute`? Since it
would a huge BC break changing this in PHP 8.1.

Best regards,
Benas Seliuginas
  110219
May 20, 2020 17:40 larry@garfieldtech.com ("Larry Garfield")
On Wed, May 20, 2020, at 12:07 PM, Benjamin Eberlei wrote:
> Hi everyone, > > the Attributes RFC was rather large already, so a few things were left open > or discussions during the vote have made us rethink a things. > > https://wiki.php.net/rfc/attribute_amendments > > These points are handled by the Amendments RFC to Attributes: > > 1. Proposing to add a grouped syntax <
I'm torn here. I can absolutely see the use cases for it, but it also is a can of worms in terms of many different coding styles. In short, this is going to make more work for FIG to sort out.
> 2. Rename PhpAttribute to Attribute in global namespace (independent of the > namespace RFC)
I don't feel strongly enough in any direction to bother wading into this... :-)
> 3. Add validation of attribute class targets, which internal attributes can > do, but userland can't > 4. Specification if an attribute is repeatable or not on the same > declaration and fail otherwise.
It's not clear from the RFC what the failure mode is. If I put an attribute in the wrong place, or repeat it when I shouldn't... at what point does the code blow up and how? Does it get silently skipped somewhere? Is there an exception? Is it purely informational for the user processing it? The RFC needs to be clearer on what happens when validation fails. Even if that's repeating something from the original RFC, it needs to be clarified here so that we know what we're talking about. --Larry Garfield
  110220
May 20, 2020 17:53 ben@benramsey.com (Ben Ramsey)
> On May 20, 2020, at 12:07, Benjamin Eberlei <kontakt@beberlei.de> wrote: > > 2. Rename PhpAttribute to Attribute in global namespace (independent of the > namespace RFC)
I suggested in a previous thread that we consider renaming `PhpAttribute` to `UserlandAttribute`, since this is the intent of the attribute, and it helps distinguish it from `CompilerAttribute`. I noticed that the compiler attribute is actually named `PhpCompilerAttribute`, so unless we change its name to `CompilerAttribute`, I don’t think it makes sense to change `PhpAttribute` to `Attribute`. Perhaps we change it to `PhpUserlandAttribute`, for clarity? Cheers, Ben
  110221
May 20, 2020 18:33 benas.molis.iml@gmail.com (Benas IML)
Hey,

I personally think that `UserlandAttribute` is too verbose and a quite
useless change since just using `Attribute` will make little to no BC
compatibility breaks (we know that thanks to Rowan, please check out
Renaming PhpAttribute thread) and would also sound more natural. This would
also mean that we would rename `PhpCompilerAttribute` to simply
`CompilerAttribute`.


Best regards,
Benas Seliuginas
  110225
May 20, 2020 22:13 kontakt@beberlei.de (Benjamin Eberlei)
On Wed, May 20, 2020 at 7:53 PM Ben Ramsey <ben@benramsey.com> wrote:

> > On May 20, 2020, at 12:07, Benjamin Eberlei <kontakt@beberlei.de> wrote: > > > > 2. Rename PhpAttribute to Attribute in global namespace (independent of > the > > namespace RFC) > > > I suggested in a previous thread that we consider renaming `PhpAttribute` > to `UserlandAttribute`, since this is the intent of the attribute, and it > helps distinguish it from `CompilerAttribute`. > > I noticed that the compiler attribute is actually named > `PhpCompilerAttribute`, so unless we change its name to > `CompilerAttribute`, I don’t think it makes sense to change `PhpAttribute` > to `Attribute`. Perhaps we change it to `PhpUserlandAttribute`, for clarity? >
Ah that's a good point that still needs to be clarified. We realized that PhpAttribute and PhpCompilerAttribute should be merged, because the difference doesn't make a difference to userland code and it complicates things. For example enforcing that PhpCompilerAttribute is just on internal classes would not work for generated stub code in Phan/Psalm and so on that "describe" internal code by imitating it in userland. This would not be allowed by the current implementation and lead to a compiler error, lets say if we imitated Deprecated for documentation purposes: > class Deprecated {} This file could not be compiled by current implementation as an error would prevent Userland Deprecated class from using PhpCompilerAttribute.
> > Cheers, > Ben >
  110229
May 21, 2020 06:06 me@jhdxr.com (=?utf-8?b?Q0hVIFpoYW93ZWk=?=)
Hi Benjamin,

It's good to hear that `PhpCompikerAttribute` can be merged, could you include it in the RFC as well? Thus I think people who support `UserlandAttribute` can agree with `Attribute` now as well.

Regards,
CHU Zhaowei

-----Original Message-----
From: Benjamin Eberlei <kontakt@beberlei.de> 
Sent: Thursday, May 21, 2020 6:13 AM
To: Ben Ramsey <ben@benramsey.com>
Cc: PHP Internals <internals@lists.php.net>
Subject: Re: [PHP-DEV] [RFC] Amendments to Attributes

On Wed, May 20, 2020 at 7:53 PM Ben Ramsey <ben@benramsey.com> wrote:

> > On May 20, 2020, at 12:07, Benjamin Eberlei <kontakt@beberlei.de> wrote: > > > > 2. Rename PhpAttribute to Attribute in global namespace (independent > > of > the > > namespace RFC) > > > I suggested in a previous thread that we consider renaming > `PhpAttribute` to `UserlandAttribute`, since this is the intent of the > attribute, and it helps distinguish it from `CompilerAttribute`. > > I noticed that the compiler attribute is actually named > `PhpCompilerAttribute`, so unless we change its name to > `CompilerAttribute`, I don’t think it makes sense to change > `PhpAttribute` to `Attribute`. Perhaps we change it to `PhpUserlandAttribute`, for clarity? >
Ah that's a good point that still needs to be clarified. We realized that PhpAttribute and PhpCompilerAttribute should be merged, because the difference doesn't make a difference to userland code and it complicates things. For example enforcing that PhpCompilerAttribute is just on internal classes would not work for generated stub code in Phan/Psalm and so on that "describe" internal code by imitating it in userland. This would not be allowed by the current implementation and lead to a compiler error, lets say if we imitated Deprecated for documentation purposes: > class Deprecated {} This file could not be compiled by current implementation as an error would prevent Userland Deprecated class from using PhpCompilerAttribute.
> > Cheers, > Ben >
  110234
May 21, 2020 15:16 ben@benramsey.com (Ben Ramsey)
> On May 21, 2020, at 01:06, CHU Zhaowei <me@jhdxr.com> wrote: > > It's good to hear that `PhpCompikerAttribute` can be merged, could you include it in the RFC as well? Thus I think people who support `UserlandAttribute` can agree with `Attribute` now as well.
I think I’m the only one who supports `UserlandAttribute`. :-) Yes, if these two are merged, then naming it `Attribute` is okay with me, since there won’t be any confusion with the names and naming schemes. Cheers, Ben
  110244
May 22, 2020 11:08 nikita.ppv@gmail.com (Nikita Popov)
On Wed, May 20, 2020 at 7:08 PM Benjamin Eberlei <kontakt@beberlei.de>
wrote:

> Hi everyone, > > the Attributes RFC was rather large already, so a few things were left open > or discussions during the vote have made us rethink a things. > > https://wiki.php.net/rfc/attribute_amendments > > These points are handled by the Amendments RFC to Attributes: > > 1. Proposing to add a grouped syntax <
2. Rename PhpAttribute to Attribute in global namespace (independent of the
> namespace RFC) >
As was mentioned in one of the previous discussions, we expect that PHP is going to ship more language-provided attributes in the future. With this proposal we have the "Attribute" attribute, but I expect we'll at least have "Deprecated" as well, and probably also something along the lines of "Jit" or "NoJit". While I'm happy with "Attribute" living in the global namespace, I don't really think we'd want to introduce "Jit" as a class in the global namespace. The name is simply to generic and does not indicate that this is part of the attribute system. We'd be forced to go with things like DeprecatedAttribute or JitAttribute, which seems rather non-optimal to me, as we're just reinventing namespaces to avoid using them... As such, I would suggest to introduce a common namespace for all attributes provided by PHP. This means we'd have Attributes\Attribute, Attributes\Deprecated, Attributes\Jit, Attributes\NoJit etc. (I'm also okay with the PHP\Attributes\Deprecated variant, but that's a separate question). Nikita 3. Add validation of attribute class targets, which internal attributes can
> do, but userland can't > 4. Specification if an attribute is repeatable or not on the same > declaration and fail otherwise. > > Each of them is a rather small issue, so I hope its ok to aggregate all > four of them in a single RFC. Please let me know if it's not. > > greetings > Benjamin >
  110245
May 22, 2020 11:55 phpmailinglists@gmail.com (Peter Bowyer)
On Fri, 22 May 2020 at 12:09, Nikita Popov ppv@gmail.com> wrote:

> While I'm happy with "Attribute" living in the global > namespace, I don't really think we'd want to introduce "Jit" as a class in > the global namespace. The name is simply to generic and does not indicate > that this is part of the attribute system. We'd be forced to go with things > like DeprecatedAttribute or JitAttribute, which seems rather non-optimal to > me, as we're just reinventing namespaces to avoid using them... > > As such, I would suggest to introduce a common namespace for all attributes > provided by PHP. This means we'd have Attributes\Attribute, > Attributes\Deprecated, Attributes\Jit, Attributes\NoJit etc. (I'm also okay > with the PHP\Attributes\Deprecated variant, but that's a separate > question). >
This is the best real-world argument in favour of PHP namespace in core that I've heard. https://wiki.php.net/rfc/php-namespace-in-core If we don't want to introduce classes in the global namespace, it makes sense to have a reserved PHP namespace we use. Peter
  110246
May 22, 2020 12:19 benas.molis.iml@gmail.com (Benas IML)
Agreed.

I don't think we should pollute any other than the `PHP\` namespace. If
that RFC doesn't pass though, we should keep the `Attribute` class in the
global namespace.

Global namespace is already reserved for PHP and so, BC breaks aren't that
severe whereas `Attributes\` namespace isn't reserved therefore BC breaks
would be "unexpected".

Best regards,
Benas Seliuginas
  110247
May 22, 2020 15:28 nikita.ppv@gmail.com (Nikita Popov)
On Fri, May 22, 2020 at 2:19 PM Benas IML iml@gmail.com> wrote:

> Agreed. > > I don't think we should pollute any other than the `PHP\` namespace. If > that RFC doesn't pass though, we should keep the `Attribute` class in the > global namespace. > > Global namespace is already reserved for PHP and so, BC breaks aren't that > severe whereas `Attributes\` namespace isn't reserved therefore BC breaks > would be "unexpected". > > Best regards, > Benas Seliuginas >
I mean, realistically speaking, BC breaks with Attributes\Attribute are a lot less likely than with just Attribute, regardless of what we reserve or don't. Some pragmatism, please. I don't particularly care what namespace the attribute classes are under, just that they should have a common namespace, because there's going to be many of them, presumably. If it was just a matter of the Attribute class, I'd definitely say this belongs in the global namespace, but that's not what we're dealing with at this point. Nikita
  110263
May 22, 2020 20:31 jakob@givoni.dk (Jakob Givoni)
On Fri, May 22, 2020 at 5:28 PM Nikita Popov ppv@gmail.com> wrote:
> > I don't particularly care what namespace the attribute classes are under, > just that they should have a common namespace, because there's going to be > many of them, presumably. If it was just a matter of the Attribute class, > I'd definitely say this belongs in the global namespace, but that's not > what we're dealing with at this point. >
You've made a really good argument for the \Php namespace here. I get that PHP reserves the global namespace, but I'm pretty certain that if PHP ever needs any subspaces, they should be under a common, non-global, namespace.
  110248
May 22, 2020 15:29 kontakt@beberlei.de (Benjamin Eberlei)
On Fri, May 22, 2020 at 1:08 PM Nikita Popov ppv@gmail.com> wrote:

> On Wed, May 20, 2020 at 7:08 PM Benjamin Eberlei <kontakt@beberlei.de> > wrote: > >> Hi everyone, >> >> the Attributes RFC was rather large already, so a few things were left >> open >> or discussions during the vote have made us rethink a things. >> >> https://wiki.php.net/rfc/attribute_amendments >> >> These points are handled by the Amendments RFC to Attributes: >> >> 1. Proposing to add a grouped syntax < > > 2. Rename PhpAttribute to Attribute in global namespace (independent of the >> namespace RFC) >> > > As was mentioned in one of the previous discussions, we expect that PHP is > going to ship more language-provided attributes in the future. With this > proposal we have the "Attribute" attribute, but I expect we'll at least > have "Deprecated" as well, and probably also something along the lines of > "Jit" or "NoJit". While I'm happy with "Attribute" living in the global > namespace, I don't really think we'd want to introduce "Jit" as a class in > the global namespace. The name is simply to generic and does not indicate > that this is part of the attribute system. We'd be forced to go with things > like DeprecatedAttribute or JitAttribute, which seems rather non-optimal to > me, as we're just reinventing namespaces to avoid using them... > > As such, I would suggest to introduce a common namespace for all > attributes provided by PHP. This means we'd have Attributes\Attribute, > Attributes\Deprecated, Attributes\Jit, Attributes\NoJit etc. (I'm also okay > with the PHP\Attributes\Deprecated variant, but that's a separate question). >
Deprecated would be an "engine level" feature, but Opcache is an extension, so it can have its own namespace "Opcache\Jit". The namespace RFC goes that far mentioning only " core symbols which cannot be unbundled" should go into a PHP namespace, which would exclude Opcache (and its "sub-extension" JIT). https://wiki.php.net/rfc/php-namespace-in-core So for me that is not necessarily an argument against Attribute in global NS, because Jit would live in Opcache\.
> > Nikita > > 3. Add validation of attribute class targets, which internal attributes can >> do, but userland can't >> 4. Specification if an attribute is repeatable or not on the same >> declaration and fail otherwise. >> >> Each of them is a rather small issue, so I hope its ok to aggregate all >> four of them in a single RFC. Please let me know if it's not. >> >> greetings >> Benjamin >> >
  110253
May 22, 2020 15:49 nikita.ppv@gmail.com (Nikita Popov)
On Fri, May 22, 2020 at 5:29 PM Benjamin Eberlei <kontakt@beberlei.de>
wrote:

> > > On Fri, May 22, 2020 at 1:08 PM Nikita Popov ppv@gmail.com> wrote: > >> On Wed, May 20, 2020 at 7:08 PM Benjamin Eberlei <kontakt@beberlei.de> >> wrote: >> >>> Hi everyone, >>> >>> the Attributes RFC was rather large already, so a few things were left >>> open >>> or discussions during the vote have made us rethink a things. >>> >>> https://wiki.php.net/rfc/attribute_amendments >>> >>> These points are handled by the Amendments RFC to Attributes: >>> >>> 1. Proposing to add a grouped syntax < >> >> 2. Rename PhpAttribute to Attribute in global namespace (independent of >>> the >>> namespace RFC) >>> >> >> As was mentioned in one of the previous discussions, we expect that PHP >> is going to ship more language-provided attributes in the future. With this >> proposal we have the "Attribute" attribute, but I expect we'll at least >> have "Deprecated" as well, and probably also something along the lines of >> "Jit" or "NoJit". While I'm happy with "Attribute" living in the global >> namespace, I don't really think we'd want to introduce "Jit" as a class in >> the global namespace. The name is simply to generic and does not indicate >> that this is part of the attribute system. We'd be forced to go with things >> like DeprecatedAttribute or JitAttribute, which seems rather non-optimal to >> me, as we're just reinventing namespaces to avoid using them... >> >> As such, I would suggest to introduce a common namespace for all >> attributes provided by PHP. This means we'd have Attributes\Attribute, >> Attributes\Deprecated, Attributes\Jit, Attributes\NoJit etc. (I'm also okay >> with the PHP\Attributes\Deprecated variant, but that's a separate question). >> > > Deprecated would be an "engine level" feature, but Opcache is an > extension, so it can have its own namespace "Opcache\Jit". The namespace > RFC goes that far mentioning only " core symbols which cannot be unbundled" > should go into a PHP namespace, which would exclude Opcache (and its > "sub-extension" JIT). https://wiki.php.net/rfc/php-namespace-in-core > > So for me that is not necessarily an argument against Attribute in global > NS, because Jit would live in Opcache\. >
Interesting point. I think this is more an argument against using a "PHP" vendor namespace, than against using an "Attributes" namespace. JIT is not "really" an opcache feature, it just lives in opcache right now because it happens to be convenient. There are long term plans to move the optimization-related functionality from opcode into core, and JIT should also live in core (and I think there are plenty of workloads that are interested in JIT without the SHM caching). Given that context, calling the attribute Opcache\Jit seems somewhat ill-advised. I also think we'd want to provide this attribute independently of whether opcache is actually loaded, because annotating functions with it would be rather problematic if it's not always available... Ultimately I'm okay with going with just "Attribute" here, but we need to acknowledge that this comes with future obligations. In particular, introducing a "Deprecated" class is a "no" from me, because the name is too generic and does not indicate that this is part of the attribute system. It will need to be "DeprecatedAttribute" (or "Attribute_Deprecated", ugh). Which also means that you need to "use DeprecatedAttribute as Deprecated" to use it. This isn't terrible, but I also don't think it's ideal. Regards, Nikita
  110265
May 23, 2020 01:34 bobwei9@hotmail.com (Bob Weinand)
> Am 22.05.2020 um 13:08 schrieb Nikita Popov ppv@gmail.com>: > > On Wed, May 20, 2020 at 7:08 PM Benjamin Eberlei <kontakt@beberlei.de> > wrote: > >> 2. Rename PhpAttribute to Attribute in global namespace (independent of the >> namespace RFC) > > As was mentioned in one of the previous discussions, we expect that PHP is > going to ship more language-provided attributes in the future. With this > proposal we have the "Attribute" attribute, but I expect we'll at least > have "Deprecated" as well, and probably also something along the lines of > "Jit" or "NoJit". While I'm happy with "Attribute" living in the global > namespace, I don't really think we'd want to introduce "Jit" as a class in > the global namespace. The name is simply to generic and does not indicate > that this is part of the attribute system. We'd be forced to go with things > like DeprecatedAttribute or JitAttribute, which seems rather non-optimal to > me, as we're just reinventing namespaces to avoid using them... > > As such, I would suggest to introduce a common namespace for all attributes > provided by PHP. This means we'd have Attributes\Attribute, > Attributes\Deprecated, Attributes\Jit, Attributes\NoJit etc. (I'm also okay > with the PHP\Attributes\Deprecated variant, but that's a separate question). > > Nikita
At that point, don't we just want to be able to generically mark all attribute classes visible as being an Attribute? Also the classical examples like <> do not tell you anyhow that they're Attributes, if the class is accessed outside of attribute context. And I guess, with the logic you proposed, it will likely be named "ORM\Attribute\Entity" making it even longer. I think it would be good to be able to later on just write <>, <> etc. without further imports, as they seem to be quite basic functionality. I think it should remain possible to write simple code without much namespace awareness. Perhaps we should actually name attribute classes including their << and >>.. Thus "class <> {}" (attribute decl), "new <>()" (custom instantiation), "$attribute instanceof <>". (and, for a namespaced attribute "$attribute instanceof MyNamespace\<>") The engine could quite easily support that, it's just a little parser work. That way the whole discussion as to where to put attributes in the namespace hierarchy would be quite moot, for PHP itself as well as userland. Bob P.s.: As a caveat, on parameter types, if we expect a value being of an attribute class, we'd need to require a qualified name containing at least one backslash or import-alias there. But that's acceptable I think, it will just happen within some internal attribute processing code.
  110268
May 23, 2020 14:56 rowan.collins@gmail.com (Rowan Tommins)
On 22/05/2020 12:08, Nikita Popov wrote:
> As such, I would suggest to introduce a common namespace for all attributes > provided by PHP. This means we'd have Attributes\Attribute, > Attributes\Deprecated, Attributes\Jit, Attributes\NoJit etc. (I'm also okay > with the PHP\Attributes\Deprecated variant, but that's a separate question).
One possible policy which came up in chat last night was that universal base classes (which tend to be few in number, and act almost as keywords) should be in the root namespace; but built-in implementations of those (at the same level as ones users could create) are prefixed with "PHP\Something\". For instance, if we had a time machine and were adding them all now: - "\Throwable" and "\Exception", but "\PHP\Exceptions\RuntimeException" - "\Iterator" but "\PHP\Iterators\FilterIterator" - "\Attribute", but "\PHP\Attributes\Deprecated" Regards, -- Rowan Tommins (né Collins) [IMSoP]
  110273
May 25, 2020 20:12 benas.molis.iml@gmail.com (Benas IML)
That's a brilliant idea, completely agreed, Rowan!

On the other note, don't want to nitpick here but I believe that it would be
better to name the repeatable attribute simply as `<>`. It would
match other languages (such as Java) and the naming wouldn't be that
verbose.

Also IMO, I think for consistency we should either use only parameters e. g.
`<>` or separate
attributes for both target validation and repeatability e. g.
`<>`  and `<>`.

Best regards,
Benas Seliuginas
  110289
May 27, 2020 22:05 benas.molis.iml@gmail.com (Benas IML)
It seems that the RFC was updated to use the `Attributes` namespace. I
think this is a bad idea since we're reserving a generic namespace that we
haven't even "soft" reserved. Also, the loss of fallback to global
namespace is a turning point for me.

Generally, I think we should instead do something like Rowan said: use
namespaced classes only for implementations (e.g. `\Attribute` but
`\PHP\Deprecated`).

Best regards,
Benas

On Wed, May 20, 2020, 8:08 PM Benjamin Eberlei <kontakt@beberlei.de> wrote:

> Hi everyone, > > the Attributes RFC was rather large already, so a few things were left open > or discussions during the vote have made us rethink a things. > > https://wiki.php.net/rfc/attribute_amendments > > These points are handled by the Amendments RFC to Attributes: > > 1. Proposing to add a grouped syntax < > 2. Rename PhpAttribute to Attribute in global namespace (independent of the > namespace RFC) > 3. Add validation of attribute class targets, which internal attributes can > do, but userland can't > 4. Specification if an attribute is repeatable or not on the same > declaration and fail otherwise. > > Each of them is a rather small issue, so I hope its ok to aggregate all > four of them in a single RFC. Please let me know if it's not. > > greetings > Benjamin >
  110295
May 28, 2020 17:46 kontakt@beberlei.de (Benjamin Eberlei)
On Thu, May 28, 2020 at 12:05 AM Benas IML iml@gmail.com>
wrote:

> It seems that the RFC was updated to use the `Attributes` namespace. I > think this is a bad idea since we're reserving a generic namespace that we > haven't even "soft" reserved. Also, the loss of fallback to global > namespace is a turning point for me. > > Generally, I think we should instead do something like Rowan said: use > namespaced classes only for implementations (e.g. `\Attribute` but > `\PHP\Deprecated`). >
I agree on sentiment, but the PHP namespace vote is failing right now, so that makes this plan just a rehashing of the existing, failing vote. The likelihood of a class Attributes\Attribute or Attributes\Deprecated existing in any code out there is much much lower, than the classes Attribute and Deprecated existing in the global namespace. The "PHP\" namespace would imply some sort of claim of the project, which the failing namespace vote shows does not exist. The way attributes map to classes, suffixes/prefixes make them strange to look at, so for future non-breaks with userland code it will be safer to put them off to a new namespace and this potentially increases the user perception. If this vote fails, this implicitly means that all future internal attributes will probably go into the global namespace.
> > Best regards, > Benas > > On Wed, May 20, 2020, 8:08 PM Benjamin Eberlei <kontakt@beberlei.de> > wrote: > >> Hi everyone, >> >> the Attributes RFC was rather large already, so a few things were left >> open >> or discussions during the vote have made us rethink a things. >> >> https://wiki.php.net/rfc/attribute_amendments >> >> These points are handled by the Amendments RFC to Attributes: >> >> 1. Proposing to add a grouped syntax < >> 2. Rename PhpAttribute to Attribute in global namespace (independent of >> the >> namespace RFC) >> 3. Add validation of attribute class targets, which internal attributes >> can >> do, but userland can't >> 4. Specification if an attribute is repeatable or not on the same >> declaration and fail otherwise. >> >> Each of them is a rather small issue, so I hope its ok to aggregate all >> four of them in a single RFC. Please let me know if it's not. >> >> greetings >> Benjamin >> >
  110323
May 31, 2020 23:48 theodorejb@outlook.com (Theodore Brown)
On Wed, May 20, 2020 at 12:07 PM Benjamin Eberlei <kontakt@beberlei.de> wrote:

> the Attributes RFC was rather large already, so a few things were left > open or discussions during the vote have made us rethink a things. > > https://wiki.php.net/rfc/attribute_amendments > > These points are handled by the Amendments RFC to Attributes: > > 1. Proposing to add a grouped syntax <
Hi Benjamin, I find the grouped attribute proposal somewhat troubling. The RFC contains the following example: ```php <> public function test() { } ``` The problem with this syntax is that adding a new attribute at the start or end of the list, or removing one of the attributes, will require modifying multiple lines. For some time the language has been moving away from this (see the various RFCs to allow trailing commas in more places), so this feels like a step backwards. If trailing commas are allowed in grouped attributes, you could write it this way instead: ```php << Attr2("foo"), Attr2("bar"),
>> public function test()
{ } ``` But to me this still feels rather clunky. It requires two extra lines, and when moving from two attributes to one attribute (or vice versa), you'd still probably end up modifying multiple lines. Another issue with the grouped syntax is that comma separated attributes can be easy to confuse with comma separated attribute arguments. For example: ```php <> <> function bar() {} ``` It can be hard to tell which line contains multiple attributes vs. multiple attribute arguments. Ultimately it seems like the grouped attribute proposal is attempting to work around the poor usability of the current verbose syntax. Maybe it would be better to instead propose a simpler syntax that avoids these issues. I know that some internals members expressed interest in an `@@` token, but this was never voted on. Having a distinct token for attributes would entirely avoid the issues of having to modify multiple lines when adding/removing attributes, as well as confusion with shift operators and comma-separated attribute arguments. E.g. the RFC example would look like this instead: ```php @@Attr2("foo") @@Attr2("bar") public function test() { } ``` To me this would be a lot cleaner and fit in better with the rest of the language. Best regards, Theodore
  110324
June 1, 2020 10:03 rowan.collins@gmail.com (Rowan Tommins)
On Mon, 1 Jun 2020 at 00:48, Theodore Brown <theodorejb@outlook.com> wrote:

> > Having a distinct token for attributes would entirely avoid the issues > of having to modify multiple lines when adding/removing attributes, as > well as confusion with shift operators and comma-separated attribute > arguments. E.g. the RFC example would look like this instead: > > ```php > @@Attr2("foo") > @@Attr2("bar") > public function test() > { > } > ``` >
I'm not sure what you mean by "having a distinct token"; the cases where there is any chance of confusion with bit-shifts are going to be very rare, and aren't related to any of the other issues you're discussing. You make some good points about not having comma-separated attributes, but they don't require us to change the syntax, just not add that feature to it. On a personal note, I find @@ a lot uglier than << and think a bracket-link syntax looks clearer when writing multiple attributes on one line to avoid long thin columns of code: <> <> public function test() { } vs @@Attr2("foo") @@Attr2("bar") public function test() { } Regards, -- Rowan Tommins [IMSoP]
  110332
June 1, 2020 22:48 kontakt@beberlei.de (Benjamin Eberlei)
On Mon, Jun 1, 2020 at 1:48 AM Theodore Brown <theodorejb@outlook.com>
wrote:

> On Wed, May 20, 2020 at 12:07 PM Benjamin Eberlei <kontakt@beberlei.de> > wrote: > > > the Attributes RFC was rather large already, so a few things were left > > open or discussions during the vote have made us rethink a things. > > > > https://wiki.php.net/rfc/attribute_amendments > > > > These points are handled by the Amendments RFC to Attributes: > > > > 1. Proposing to add a grouped syntax < > > Hi Benjamin, > > I find the grouped attribute proposal somewhat troubling. The RFC > contains the following example: > > ```php > < Attr2("bar")>> > public function test() > { > } > ``` > > The problem with this syntax is that adding a new attribute at the > start or end of the list, or removing one of the attributes, will > require modifying multiple lines. For some time the language has been > moving away from this (see the various RFCs to allow trailing commas > in more places), so this feels like a step backwards. > > If trailing commas are allowed in grouped attributes, you could > write it this way instead: > > ```php > << > Attr2("foo"), > Attr2("bar"), > >> > public function test() > { > } > ``` > > But to me this still feels rather clunky. It requires two extra lines, > and when moving from two attributes to one attribute (or vice versa), > you'd still probably end up modifying multiple lines. >
I have added trailing commas to the attribute grouping. Now that its possible everywhere, we should go with it here directly from the beginning. About clunkyness, I am not sure this makes sense as an argument, bceause the extra lines are present for all other cases of trailing commas in popular coding styles: [ "arg1", "arg2", ] or foo( "arg1", "arg2", );
> > Another issue with the grouped syntax is that comma separated > attributes can be easy to confuse with comma separated attribute > arguments. For example: > > ```php > <> > <> > function bar() {} > ``` > > It can be hard to tell which line contains multiple attributes vs. > multiple attribute arguments. >
The same is true about commas in nested short arrays.
> > Ultimately it seems like the grouped attribute proposal is attempting > to work around the poor usability of the current verbose syntax. Maybe > it would be better to instead propose a simpler syntax that avoids > these issues. I know that some internals members expressed interest > in an `@@` token, but this was never voted on. >
If you feel strongly about it, please create your own RFC for this. You would need to put it under discussion within this week though, as the feature freeze deadlines are looming. Personally I have no interest in opening up the syntax question again, because there are really not many objective arguments to be made, its mostly subjective and feeling based.
> > Having a distinct token for attributes would entirely avoid the issues > of having to modify multiple lines when adding/removing attributes, as > well as confusion with shift operators and comma-separated attribute > arguments. E.g. the RFC example would look like this instead: >
@@ is not a distinct token though, because it is already valid syntax to have the error silence operator multiple times after each other.
> > ```php > @@Attr2("foo") > @@Attr2("bar") > public function test() > { > } > ``` > > To me this would be a lot cleaner and fit in better with the rest of > the language. > > Best regards, > Theodore >
  110360
June 4, 2020 09:28 kontakt@beberlei.de (Benjamin Eberlei)
I have changed back the rename from namespacing to Attributes\Attribute to
using just Attribute after a few discussions off list. The reasoning is
that it becomes more clear that a majority of core contributors strongly
prefers using the global namespace as the PHP namespace and opening up this
point again makes no sense. So the state of the RFC is again what it was
when I originally posted it with renaming PhpAttribute to Attribute.

Unless there is some new significant feedback I am going to open up this
RFC for voting on Monday next week.

On Wed, May 20, 2020 at 7:07 PM Benjamin Eberlei <kontakt@beberlei.de>
wrote:

> Hi everyone, > > the Attributes RFC was rather large already, so a few things were left > open or discussions during the vote have made us rethink a things. > > https://wiki.php.net/rfc/attribute_amendments > > These points are handled by the Amendments RFC to Attributes: > > 1. Proposing to add a grouped syntax < > 2. Rename PhpAttribute to Attribute in global namespace (independent of > the namespace RFC) > 3. Add validation of attribute class targets, which internal attributes > can do, but userland can't > 4. Specification if an attribute is repeatable or not on the same > declaration and fail otherwise. > > Each of them is a rather small issue, so I hope its ok to aggregate all > four of them in a single RFC. Please let me know if it's not. > > greetings > Benjamin >
  110365
June 4, 2020 10:53 benas.molis.iml@gmail.com (Benas IML)
Thank you for the update! Given that there is still an open issue, is the
RFC proposing flags or a separate `<>` attribute?

Best regards,
Benas

On Thu, Jun 4, 2020, 12:29 PM Benjamin Eberlei <kontakt@beberlei.de> wrote:

> I have changed back the rename from namespacing to Attributes\Attribute to > using just Attribute after a few discussions off list. The reasoning is > that it becomes more clear that a majority of core contributors strongly > prefers using the global namespace as the PHP namespace and opening up this > point again makes no sense. So the state of the RFC is again what it was > when I originally posted it with renaming PhpAttribute to Attribute. > > Unless there is some new significant feedback I am going to open up this > RFC for voting on Monday next week. > > On Wed, May 20, 2020 at 7:07 PM Benjamin Eberlei <kontakt@beberlei.de> > wrote: > > > Hi everyone, > > > > the Attributes RFC was rather large already, so a few things were left > > open or discussions during the vote have made us rethink a things. > > > > https://wiki.php.net/rfc/attribute_amendments > > > > These points are handled by the Amendments RFC to Attributes: > > > > 1. Proposing to add a grouped syntax < > > 2. Rename PhpAttribute to Attribute in global namespace (independent of > > the namespace RFC) > > 3. Add validation of attribute class targets, which internal attributes > > can do, but userland can't > > 4. Specification if an attribute is repeatable or not on the same > > declaration and fail otherwise. > > > > Each of them is a rather small issue, so I hope its ok to aggregate all > > four of them in a single RFC. Please let me know if it's not. > > > > greetings > > Benjamin > > >
  110377
June 4, 2020 17:49 kontakt@beberlei.de (Benjamin Eberlei)
On Thu, Jun 4, 2020 at 12:54 PM Benas IML iml@gmail.com> wrote:

> Thank you for the update! Given that there is still an open issue, is the > RFC proposing flags or a separate `<>` attribute? >
Good point, we came to the conclusion to simplify. Should attributes be in the global namespace, then we shouldn't arbitrarily add more, so it will be a flag. At that point, because you rarely declare new flags we decided to merge target and flags and only have one flag. You could do the following: <>
> > Best regards, > Benas > > On Thu, Jun 4, 2020, 12:29 PM Benjamin Eberlei <kontakt@beberlei.de> > wrote: > >> I have changed back the rename from namespacing to Attributes\Attribute to >> using just Attribute after a few discussions off list. The reasoning is >> that it becomes more clear that a majority of core contributors strongly >> prefers using the global namespace as the PHP namespace and opening up >> this >> point again makes no sense. So the state of the RFC is again what it was >> when I originally posted it with renaming PhpAttribute to Attribute. >> >> Unless there is some new significant feedback I am going to open up this >> RFC for voting on Monday next week. >> >> On Wed, May 20, 2020 at 7:07 PM Benjamin Eberlei <kontakt@beberlei.de> >> wrote: >> >> > Hi everyone, >> > >> > the Attributes RFC was rather large already, so a few things were left >> > open or discussions during the vote have made us rethink a things. >> > >> > https://wiki.php.net/rfc/attribute_amendments >> > >> > These points are handled by the Amendments RFC to Attributes: >> > >> > 1. Proposing to add a grouped syntax < >> > 2. Rename PhpAttribute to Attribute in global namespace (independent of >> > the namespace RFC) >> > 3. Add validation of attribute class targets, which internal attributes >> > can do, but userland can't >> > 4. Specification if an attribute is repeatable or not on the same >> > declaration and fail otherwise. >> > >> > Each of them is a rather small issue, so I hope its ok to aggregate all >> > four of them in a single RFC. Please let me know if it's not. >> > >> > greetings >> > Benjamin >> > >> >
  110382
June 5, 2020 03:57 guilhermeblanco@gmail.com ("guilhermeblanco@gmail.com")
Hi Benjamin,

Overall, all these amendments are good in my opinion, but I'd like to
challenge a few things:

1- On item 3, the acceptable targets would be: class, function,
method, property, class constant, parameter or all.
If possible, I'd like to ask if it's possible to expand this list and
also allow attribute and constructor.

2- Also on item 3, the validation of PhpAttribute targets, it feels
more natural to have this as an array instead of a bitwise or
operator.
Have you evaluated the performance penalty to judge your decision of
bitwise vs array?

3- Repeatability should be on its own PhpAttribute. It would not block
the expansion of the repeatability in future efforts.
One possibility could be group repeated attributes as another
PhpAttribute. Example: Multiple <> be folded into a
<>.
This code:

<>
<>

Would be equivalent to (sorry if syntax is not 100% correct):

<>,
    <>
])>>

Considering:

<>
<>
class Schedule { public string $cron; /* ... */ }

<>
class Schedules { public array value = []; // Holds an array of Schedule  }

4- Now you might have recall about my initial thoughts on this
subject, but inheritance is something that would be very interesting
to see as part of this amendment.
If we introduce something like <> to the
Attribute definition, we could then mark the Attribute to be inherited
to subclasses of attributed class, while keeping the default to do not
inherit anything (like we have today).

Cheers,

On Thu, Jun 4, 2020 at 1:49 PM Benjamin Eberlei <kontakt@beberlei.de> wrote:
> > On Thu, Jun 4, 2020 at 12:54 PM Benas IML iml@gmail.com> wrote: > > > Thank you for the update! Given that there is still an open issue, is the > > RFC proposing flags or a separate `<>` attribute? > > > > Good point, we came to the conclusion to simplify. Should attributes be in > the global namespace, then we shouldn't arbitrarily add more, so it will be > a flag. > At that point, because you rarely declare new flags we decided to merge > target and flags and only have one flag. You could do the following: > > <> > > > > > Best regards, > > Benas > > > > On Thu, Jun 4, 2020, 12:29 PM Benjamin Eberlei <kontakt@beberlei.de> > > wrote: > > > >> I have changed back the rename from namespacing to Attributes\Attribute to > >> using just Attribute after a few discussions off list. The reasoning is > >> that it becomes more clear that a majority of core contributors strongly > >> prefers using the global namespace as the PHP namespace and opening up > >> this > >> point again makes no sense. So the state of the RFC is again what it was > >> when I originally posted it with renaming PhpAttribute to Attribute. > >> > >> Unless there is some new significant feedback I am going to open up this > >> RFC for voting on Monday next week. > >> > >> On Wed, May 20, 2020 at 7:07 PM Benjamin Eberlei <kontakt@beberlei.de> > >> wrote: > >> > >> > Hi everyone, > >> > > >> > the Attributes RFC was rather large already, so a few things were left > >> > open or discussions during the vote have made us rethink a things. > >> > > >> > https://wiki.php.net/rfc/attribute_amendments > >> > > >> > These points are handled by the Amendments RFC to Attributes: > >> > > >> > 1. Proposing to add a grouped syntax < > >> > 2. Rename PhpAttribute to Attribute in global namespace (independent of > >> > the namespace RFC) > >> > 3. Add validation of attribute class targets, which internal attributes > >> > can do, but userland can't > >> > 4. Specification if an attribute is repeatable or not on the same > >> > declaration and fail otherwise. > >> > > >> > Each of them is a rather small issue, so I hope its ok to aggregate all > >> > four of them in a single RFC. Please let me know if it's not. > >> > > >> > greetings > >> > Benjamin > >> > > >> > >
-- Guilherme Blanco SVP Technology at Statflo Inc. Mobile: +1 647 232 5599
  110395
June 5, 2020 22:01 kontakt@beberlei.de (Benjamin Eberlei)
On Fri, Jun 5, 2020 at 5:58 AM guilhermeblanco@gmail.com <
guilhermeblanco@gmail.com> wrote:

> Hi Benjamin, > > Overall, all these amendments are good in my opinion, but I'd like to > challenge a few things: > > 1- On item 3, the acceptable targets would be: class, function, > method, property, class constant, parameter or all. > If possible, I'd like to ask if it's possible to expand this list and > also allow attribute and constructor. >
I think this is too specific to be valuable in the language, and it could always be validated at the attribute reading level in userland. One thing i was thinking now, what if attributes could be "Reflection declaration aware", example: interface ReflectorAwareAttribute { public function setReflector(Reflector $reflector); } class MyAttribute implements ReflectorAwareAttribute { public function setReflector(Reflector $reflector) { } } ReflectionAttribute::newInstance() would check for this interface and call setReflector if present.
> > 2- Also on item 3, the validation of PhpAttribute targets, it feels > more natural to have this as an array instead of a bitwise or > operator. > Have you evaluated the performance penalty to judge your decision of > bitwise vs array? >
I feel this is subjective, PHP APIs generally use bitmasks for this kind of differentiation and not an array of strings. An array of strings would have the problem of the case with one target only: ["class"], which soon would raise requests for a string "class" only, where a union type is probably more "complex" than a bitmask.
> > 3- Repeatability should be on its own PhpAttribute. It would not block > the expansion of the repeatability in future efforts. > One possibility could be group repeated attributes as another > PhpAttribute. Example: Multiple <> be folded into a > <>. > This code: > > <> > <> > > Would be equivalent to (sorry if syntax is not 100% correct): > > < <>, > <> > ])>> > > Considering: > > <> > <> > class Schedule { public string $cron; /* ... */ } > > <> > class Schedules { public array value = []; // Holds an array of Schedule } >
Without knowing at all how and if nested attributes get supported in the future I feel this introduces a lot of complexity that is unnecessary. If nested attributes are added at some point the validation could be done in the container attributes constructor. Taking your example: <> class Schedules { public function __construct(array $schedules) { foreach ($schedules as $schedule) { $this->addSchedule($schedule); } } private function addSchedule(Schedule $schedule) {} } Any kind of language support for typed arrays would obviously simplify this even more.
> 4- Now you might have recall about my initial thoughts on this > subject, but inheritance is something that would be very interesting > to see as part of this amendment. > If we introduce something like <> to the > Attribute definition, we could then mark the Attribute to be inherited > to subclasses of attributed class, while keeping the default to do not > inherit anything (like we have today). >
An equivalent of Java Annotations @Inherited is not easily possible, so we omitted it for now. The reason is that the compile step does not validate attributes (unless they are internal engine attributes, that can only be provided by extensions). So it doesn't know at that point if the declared attribute exists and what inherit rules it might hvae defined. One solution could be to add a new flag to ::getAttributes($name, $flags = ReflectionAttribute::INCLUDE_INHERITED) that performs the gathering of inherited attributes, however that will have to trigger autoloading. The complexity and the rare use case for me speaks against adding it at the moment.
> > Cheers, > > On Thu, Jun 4, 2020 at 1:49 PM Benjamin Eberlei <kontakt@beberlei.de> > wrote: > > > > On Thu, Jun 4, 2020 at 12:54 PM Benas IML iml@gmail.com> > wrote: > > > > > Thank you for the update! Given that there is still an open issue, is > the > > > RFC proposing flags or a separate `<>` attribute? > > > > > > > Good point, we came to the conclusion to simplify. Should attributes be > in > > the global namespace, then we shouldn't arbitrarily add more, so it will > be > > a flag. > > At that point, because you rarely declare new flags we decided to merge > > target and flags and only have one flag. You could do the following: > > > > <> > > > > > > > > Best regards, > > > Benas > > > > > > On Thu, Jun 4, 2020, 12:29 PM Benjamin Eberlei <kontakt@beberlei.de> > > > wrote: > > > > > >> I have changed back the rename from namespacing to > Attributes\Attribute to > > >> using just Attribute after a few discussions off list. The reasoning > is > > >> that it becomes more clear that a majority of core contributors > strongly > > >> prefers using the global namespace as the PHP namespace and opening up > > >> this > > >> point again makes no sense. So the state of the RFC is again what it > was > > >> when I originally posted it with renaming PhpAttribute to Attribute. > > >> > > >> Unless there is some new significant feedback I am going to open up > this > > >> RFC for voting on Monday next week. > > >> > > >> On Wed, May 20, 2020 at 7:07 PM Benjamin Eberlei <kontakt@beberlei.de > > > > >> wrote: > > >> > > >> > Hi everyone, > > >> > > > >> > the Attributes RFC was rather large already, so a few things were > left > > >> > open or discussions during the vote have made us rethink a things. > > >> > > > >> > https://wiki.php.net/rfc/attribute_amendments > > >> > > > >> > These points are handled by the Amendments RFC to Attributes: > > >> > > > >> > 1. Proposing to add a grouped syntax < > > >> > 2. Rename PhpAttribute to Attribute in global namespace > (independent of > > >> > the namespace RFC) > > >> > 3. Add validation of attribute class targets, which internal > attributes > > >> > can do, but userland can't > > >> > 4. Specification if an attribute is repeatable or not on the same > > >> > declaration and fail otherwise. > > >> > > > >> > Each of them is a rather small issue, so I hope its ok to aggregate > all > > >> > four of them in a single RFC. Please let me know if it's not. > > >> > > > >> > greetings > > >> > Benjamin > > >> > > > >> > > > > > > > -- > Guilherme Blanco > SVP Technology at Statflo Inc. > Mobile: +1 647 232 5599 >