Re: [PHP-DEV] Re: [RFC] Attributes v2
Hi Niklas, On Mon, 20 Apr 2020 at 07:48, Niklas Keller wrote: > > What's more important here IMO is the restriction of inheritance, > mainly because the raw arguments are exposed via reflection and won't > be compatible between parent and child attribute in any case. > Could you explain a bit more about the problem you see here? My understanding is that the reflection API is designed to allow two usage styles: * Treating attributes as untyped mappings from a name to a list of arguments (fetch all, or filter by exact name, then call getName() and getArguments()) * Treating attributes as classes instantiated with the argument given (filter by INSTANCE_OF, then call newInstance(), formerly called getAsObject()) For the first style, inheritance doesn't matter, because the attribute name is never resolved as a class anyway, it's just an identifier (it might happen to look like one, to avoid naming collisions, in the same way XML namespaces look like URLs). For the second style, it's up to the user to provide the right arguments for the attribute they used, just as it would be if they instantiated it directly with the "new" keyword. The only case I can see where incompatible constructors would be a problem would be if someone filtered by INSTANCE_OF, and then called getArguments() and expected them to be consistent rather than using newInstance(), but I'm not sure why anyone would want to do that, so a one line note in the manual pointing out it's a bad idea would seem sufficient. But perhaps I'm missing something more fundamental that makes you feel that inheritance should be restricted? Regards, -- Rowan Tommins [IMSoP]
Re: [PHP-DEV] Re: [RFC] Attributes v2
Hey Benjamin, thanks for your answers! > <> would best require a namespace (PHP\Attributes) as I feel > claminig "Attribute" class in the main namespace might cause problems for > users. While this is true, I don't see how Attribute is different to any classes introduced in the "recent" versions like \Generator or \Error. Maybe Nikita can run his analyzer to see how widespread its usage is? > But there is no PHP namespace yet and proposals about this have just > gotten to the list. I have therefore looked to PhpToken from nikitas recent > RFC as an example for naming, because several contributors mentioned that > the PHP\Attributes namespace I suggested in an earlier version of the RFC > would be an instant "no" vote, because of the lack of previous decision on > this topic. Some prefix is pretty similar to a namespace, and I don't like a new naming convention being part of this RFC. For PhpToken the naming might make sense, as "Token" alone might be seen as a too generic term. > The reason these are plain PHP objects and not some special case is that > you can re-use them for configuration from different sources. Symfony > Validator is a good example where the "attributes" could map to the > validator configuration classes that are also used from XML, YAML, ... Sounds fine! > Yes, INSTANCE_OF attempts to load each attributes class, but if an > attribute class cannot be looked up (not autoloadable) it gets skipped > without error (subject to error handling of autoloader implementation, but > for Composer it skips). This seems rather strange to me, but I can definitely see the value if you're using no-dev deployments, for example. I think I'd be better to disallow inheritance for attributes and skip the autoloading and INSTANCE_OF parameter here. > > > > I expect annotations to be readonly, which classes as outlined in the > > RFC cannot currently enforce in PHP. A syntax similar to interfaces > > might be appropriate, I'm not sure whether that has been discussed: > > > > The write-once / readonly RFC was rejected and only internal classes can > implement this behavior (see ext/dom). But userland attributes also map to > userland classes, so as you say this is not possible. However given this > RFC maps to existing concepts. I'd be possible if the actual objects are created in core and userland only provides the contracts (interfaces) for these attributes. > I don't see how preventing instantiation or requiring readonly in userland > produces any benefits over this described use-case in the RFC. This would > make attributes much stricter than everything else in the language, I don't > see how its fair to impose this on a new feature like Attributes, when the > language doesn't even have concepts for this strictness for regular classes > (containing more important code). Mapping to "normal" classes is the way C# > implements attributes, I don't think this should be more complex than that. Readonly is just something I'd like to see, it's not a requirement and I'll still vote yes if the more important points are solved. What's more important here IMO is the restriction of inheritance, mainly because the raw arguments are exposed via reflection and won't be compatible between parent and child attribute in any case. There are a few options I see to solve this: - Require constructor compatibility for attribute classes (only other classes) - Forbid extending attribute classes entirely - Remove getArguments() from reflection I think my preferred solution would be to forbid inheritance, because that also solves the autoloading inconsistencies (given that implemented interfaces aren't respected in `ReflectionFunction::getAttributes()` and others. > Extensions, and Third Party library authors can easily guard their > attribute classes against writes from the outside with the usual OOP > abstractions and if application authors deem their attributes to be > writable that is their business. That's probably fine, yes, if `getAsObject()` / `toObject()` / `createObject()` always returns a new object. > Named parameters might some day come to PHP in the future, and this is > precisely the argument for treating an attribute like a regular php class > with a constructor, because the named params syntax would look exactly the > same in attribute declarations then, increasing language consistency. > > The reason I went with the C# way of mapping to a "plain class" is > simplicity. The concept of attributes is already not the easiest nut to > crack, inventing another keyword and a structure that looks like a class, > but has very different implications requires a lot of words in the > documentation and doesn't provide the easiest access to beginners. I guess writing your own annotations and processing them isn't a beginners topic, but using them to attribute code definitely is. > > > > Finally, the naming of "getAsObject" should IMO be improved, > > especially if I read the RFC correctly and autoloading
Re: [PHP-DEV] Re: [RFC] Attributes v2
On Sun, Apr 19, 2020 at 11:30 PM Niklas Keller wrote:
> Hey Benjamin,
>
> I haven't really followed the discussion, so sorry if anything I'm
> writing has already been discussed.
>
> Attributes / annotations are one of the two things I currently miss
> most in PHP (the other being generics), so thanks for the work on
> that!
>
> There are a few points that seem odd to me, I'll start with the "Php"
> prefix:
>
> ```php
>
> namespace My\Attributes;
>
> use PhpAttribute;
>
> <>
> class SingleArgument
> {
> public $value;
>
> public function __construct(string $value)
> {
> $this->value = $value;
> }
> }
> ```
>
> I think this should be <> instead, if we go with that
> syntax. However, I'd even propose another syntax, as attributes aren't
> ordinary classes, I'm not sure whether they should be instantiatable
> from userland and / or be able to use inheritance, especially as
> constructors in PHP aren't subject to variance rules. I guess the
> INSTANCE_OF filter also changes whether the attributes are autoloaded
> or not?
>
<> would best require a namespace (PHP\Attributes) as I feel
claminig "Attribute" class in the main namespace might cause problems for
users. But there is no PHP namespace yet and proposals about this have just
gotten to the list. I have therefore looked to PhpToken from nikitas recent
RFC as an example for naming, because several contributors mentioned that
the PHP\Attributes namespace I suggested in an earlier version of the RFC
would be an instant "no" vote, because of the lack of previous decision on
this topic.
The reason these are plain PHP objects and not some special case is that
you can re-use them for configuration from different sources. Symfony
Validator is a good example where the "attributes" could map to the
validator configuration classes that are also used from XML, YAML, ...
Yes, INSTANCE_OF attempts to load each attributes class, but if an
attribute class cannot be looked up (not autoloadable) it gets skipped
without error (subject to error handling of autoloader implementation, but
for Composer it skips).
>
> I expect annotations to be readonly, which classes as outlined in the
> RFC cannot currently enforce in PHP. A syntax similar to interfaces
> might be appropriate, I'm not sure whether that has been discussed:
>
The write-once / readonly RFC was rejected and only internal classes can
implement this behavior (see ext/dom). But userland attributes also map to
userland classes, so as you say this is not possible. However given this
RFC maps to existing concepts.
I don't see how preventing instantiation or requiring readonly in userland
produces any benefits over this described use-case in the RFC. This would
make attributes much stricter than everything else in the language, I don't
see how its fair to impose this on a new feature like Attributes, when the
language doesn't even have concepts for this strictness for regular classes
(containing more important code). Mapping to "normal" classes is the way C#
implements attributes, I don't think this should be more complex than that.
Extensions, and Third Party library authors can easily guard their
attribute classes against writes from the outside with the usual OOP
abstractions and if application authors deem their attributes to be
writable that is their business.
>
> ```php
>
> namespace My\Attributes;
>
> attribute SingleArgument
> {
> public function value(): string;
> }
> ```
>
> Such a Java-like syntax would unfortunately only work with some kind
> of named parameters.
>
Named parameters might some day come to PHP in the future, and this is
precisely the argument for treating an attribute like a regular php class
with a constructor, because the named params syntax would look exactly the
same in attribute declarations then, increasing language consistency.
The reason I went with the C# way of mapping to a "plain class" is
simplicity. The concept of attributes is already not the easiest nut to
crack, inventing another keyword and a structure that looks like a class,
but has very different implications requires a lot of words in the
documentation and doesn't provide the easiest access to beginners.
>
> Finally, the naming of "getAsObject" should IMO be improved,
> especially if I read the RFC correctly and autoloading and constructor
> invocation is performed by this method. I think "createObject" or
> "toObject" might be better names.
>
The name getAsObject is an implementation detail in my perspective. I am
open for a better name.
>
> In summary, I'd probably vote "no" on the current proposal, even if
> it's one of the most missed features, because I think we can do
> better, and there's only one chance.
>
Sorry to hear and I hope you reconsider after reading my reply, as I built
this RFC around extensibility in the future primarily to address a lot of
voter feedback from the last RFCs.
For example things you mentioned: named parameters, readonly properties,
Re: [PHP-DEV] Re: [RFC] Attributes v2
Hey Benjamin,
I haven't really followed the discussion, so sorry if anything I'm
writing has already been discussed.
Attributes / annotations are one of the two things I currently miss
most in PHP (the other being generics), so thanks for the work on
that!
There are a few points that seem odd to me, I'll start with the "Php" prefix:
```php
>
class SingleArgument
{
public $value;
public function __construct(string $value)
{
$this->value = $value;
}
}
```
I think this should be <> instead, if we go with that
syntax. However, I'd even propose another syntax, as attributes aren't
ordinary classes, I'm not sure whether they should be instantiatable
from userland and / or be able to use inheritance, especially as
constructors in PHP aren't subject to variance rules. I guess the
INSTANCE_OF filter also changes whether the attributes are autoloaded
or not?
I expect annotations to be readonly, which classes as outlined in the
RFC cannot currently enforce in PHP. A syntax similar to interfaces
might be appropriate, I'm not sure whether that has been discussed:
```php
:
>
> As there has only been minimal new discussion after the last changes to the
> RFC I wanted to give a heads up that I will open the vote on Monday
> afternoon.
>
> If you have further remarks or questions about the RFC, please let me know.
>
> On Mon, Mar 9, 2020 at 3:42 PM Benjamin Eberlei wrote:
>
> > Hi all,
> >
> > I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in
> > 2016 with a few changes, incorporating feedback from the mailing list back
> > then and from talking to previous no voters.
> >
> > The RFC is at https://wiki.php.net/rfc/attributes_v2
> >
> > A working patch is at https://github.com/beberlei/php-src/pull/2 though
> > work around the details is still necessary.
> >
> > The RFC contains a section with common criticism and objections to
> > attributes, and I hope to have collected and responded to a good amount
> > already from previous discussions.
> >
> > There is also a fair amount of implementation detail still up for debate,
> > which is noted in "Open Issues". I have pre-committed to one approach, but
> > listed alternatives there. On these issues I am looking for your feedback.
> >
> > greetings
> > Benjamin
> >
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Attributes v2
On Fri, Apr 17, 2020 at 6:11 PM Theodore Brown wrote: > > the RFC says that the alternate T_ATTRIBUTE `@:` token has the > downside "that it does not permit whitespace in attribute names to > allow detecting the ending of the declaration." Can you provide an > example of an attribute name containing whitespace that would be allowed > with the shift left/right tokens but not with the attribute token? >From what I saw in the related PRs (esp. https://github.com/beberlei/php-src/pull/2#issuecomment-609466083), the main reason is that if `@:` were defined as a standalone token, then the following sequence (with whitespace everywhere possible, like in https://3v4l.org/NRYbp): function ( @: A \ B $x ) would be ambiguous between two possible interpretations: function ( @:A \B $x ) // A attribute, \B type declaration function ( @:A\B $x ) // A\B attribute, untyped equivalent to respectively: function ( <> \B $x ) function ( <> $x ) which permit whitespace (unambiguous because delimited): function ( << A >> \ B $x ) function ( << A \ B >> $x ) (although I've never seen code with spaces around namespace separators). -- Guilliam Xavier -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Attributes v2
On Fri, Apr 17, 2020 at 12:43 PM Benjamin Eberlei wrote:
> On Fri, Apr 17, 2020 at 6:11 PM Theodore Brown wrote:
>
> > Can you provide an example of an attribute name containing whitespace
> > that would be allowed with the shift left/right tokens but not with the
> > attribute token?
>
> This is about [whitespace] between token and attribute name, so `@:Foo`
> is allowed but `@: Foo` is not. Whereas with the hugging As, `<>`
> and `<< Foo >>` is allowed.
Ah, that makes a lot more sense. Thanks for the clarification. I guess
allowing whitespace could also be considered a downside, since it will
lead to style guide wars about which spacing convention to use.
> This is a personal assumption here, but I would assume 95% of developers
> have never used >> or << before or for a long time and maybe 80% don't
> even know what bit shift means and how it works. I would think nobody
> needs this in attributes.
Even if bit shifts are rarely used, the fact that such syntax is valid
means there's a higher cognitive load when reading attributes to
understand whether a shift token is part of a parameter value or
delineating the start/end of an attribute declaration.
Quick, does this function have two attributes with one parameter each,
or one attribute with two parameters?
```php
<><>
function foo() {}
```
How about this one?
```php
const Foo = 2;
<>Foo, (4 + 5) * 2)>>
function foo() {}
```
Even if the syntax is technically unambiguous, reusing shift tokens as
attribute delineators results in symbol-heavy code which is harder to
quickly and correctly understand.
FWIW Hack is apparently moving away from the shift token attribute
syntax to one using `@` (see
https://github.com/facebook/hhvm/commit/3983bd2ca6b252a93d98f2bb2d7e8e89f6f004d1);
Sincerely,
Theodore
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Attributes v2
On Fri, Apr 17, 2020 at 6:11 PM Theodore Brown
wrote:
> On Fri, Apr 17, 2020 at 5:49 AM Benjamin Eberlei
> wrote:
>
> > As there has only been minimal new discussion after the last changes to
> the
> > RFC I wanted to give a heads up that I will open the vote on Monday
> > afternoon.
> >
> > If you have further remarks or questions about the RFC, please let me
> know.
> >
> > On Mon, Mar 9, 2020 at 3:42 PM Benjamin Eberlei
> wrote:
> >
> > > Hi all,
> > >
> > > I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in
> > > 2016 with a few changes, incorporating feedback from the mailing list
> back
> > > then and from talking to previous no voters.
> > >
> > > The RFC is at https://wiki.php.net/rfc/attributes_v2
>
> Hi Benjamin,
>
> Thanks for working on this RFC. I have a couple questions and
> thoughts about the proposed syntax options.
>
> First, the RFC says that the alternate T_ATTRIBUTE `@:` token has the
> downside "that it does not permit whitespace in attribute names to
> allow detecting the ending of the declaration." Can you provide an
> example of an attribute name containing whitespace that would be allowed
> with the shift left/right tokens but not with the attribute token?
>
This is about whitespaes between token and attribute name, so "@:Foo" is
allowed but "@: Foo" is not.
Whereas with the hugging As, <> and << Foo >> is allowed. This might
come in handy if potentially we allow grouping attributes in the future
such that:
<<
Foo("bar", 1+1),
Bar("baz", 2+2)
>>
> The RFC says that "Semantically the attribute declaration should be
> read as instantiating a class with the attribute name and passing
> arguments to the constructor." But class names can't contain spaces,
> so how is it a downside for attribute names to not permit them either?
> It seems like it would be a massive footgun to allow attribute names
> that *can't* resolve to a class name, since there would be no way to
> migrate them to a declared attribute class without a BC break. For this
> reason, regardless of the final syntax choice I think whitespace should
> not be permitted in attribute names.
>
This paragraph becomes a non-issue then, the attribute names themselves
don't have spaces in them, so they are always valid.
>
> Secondly, given that attribute arguments are evaluated as constant
> expressions, it's far easier for me to read the T_ATTRIBUTE `@:` syntax
> at a glance than the syntax reusing shift left/right tokens. With the
> latter my eyes tend to confuse shift left/right tokens in a constant
> expression with the open/close token of an attribute (especially since
> the syntax using bit shift tokens allows multiple attribute declarations
> on the same line).
>
> Consider this example using the shift left/right token syntax:
>
> ```php
> <> 1, 4 << 1)>><> 1, 4 << 1)>>
> function foo() {}
> ```
>
> To me the same thing using the T_ATTRIBUTE syntax is far more readable:
>
> ```php
> @:BitShiftExample(4 >> 1, 4 << 1)
> @:OtherAttribute(4 >> 1, 4 << 1)
> function foo() {}
> ```
>
This is a personal assumption here, but I would assume 95% of developers
have never used >> or << before or for a long time and maybe 80% don't even
know what bit shift means and how it works. I would think nobody needs this
in attributes.
The syntax is a secondary vote though, so you can just vote your preference
:-)
> Best regards,
> Theodore
>
Re: [PHP-DEV] Re: [RFC] Attributes v2
On Fri, 17 Apr 2020 at 17:11, Theodore Brown wrote:
> Consider this example using the shift left/right token syntax:
>
> ```php
> <> 1, 4 << 1)>><> 1, 4 << 1)>>
> function foo() {}
> ```
>
> To me the same thing using the T_ATTRIBUTE syntax is far more readable:
>
> ```php
> @:BitShiftExample(4 >> 1, 4 << 1)
> @:OtherAttribute(4 >> 1, 4 << 1)
> function foo() {}
> ```
>
This is an interesting point, but how often are users likely to write
anything like that?
Perhaps part of the reason I'm less bothered by the angle-bracket syntax
than others is that I can count the number of times I've used bit shifts on
the fingers of one hand, so I don't read it as "reusing the left/right
shift token", I read it as "enclosed in double angle brackets".
Regards,
--
Rowan Tommins
[IMSoP]
Re: [PHP-DEV] Re: [RFC] Attributes v2
On Fri, Apr 17, 2020 at 5:49 AM Benjamin Eberlei wrote:
> As there has only been minimal new discussion after the last changes to the
> RFC I wanted to give a heads up that I will open the vote on Monday
> afternoon.
>
> If you have further remarks or questions about the RFC, please let me know.
>
> On Mon, Mar 9, 2020 at 3:42 PM Benjamin Eberlei wrote:
>
> > Hi all,
> >
> > I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in
> > 2016 with a few changes, incorporating feedback from the mailing list back
> > then and from talking to previous no voters.
> >
> > The RFC is at https://wiki.php.net/rfc/attributes_v2
Hi Benjamin,
Thanks for working on this RFC. I have a couple questions and
thoughts about the proposed syntax options.
First, the RFC says that the alternate T_ATTRIBUTE `@:` token has the
downside "that it does not permit whitespace in attribute names to
allow detecting the ending of the declaration." Can you provide an
example of an attribute name containing whitespace that would be allowed
with the shift left/right tokens but not with the attribute token?
The RFC says that "Semantically the attribute declaration should be
read as instantiating a class with the attribute name and passing
arguments to the constructor." But class names can't contain spaces,
so how is it a downside for attribute names to not permit them either?
It seems like it would be a massive footgun to allow attribute names
that *can't* resolve to a class name, since there would be no way to
migrate them to a declared attribute class without a BC break. For this
reason, regardless of the final syntax choice I think whitespace should
not be permitted in attribute names.
Secondly, given that attribute arguments are evaluated as constant
expressions, it's far easier for me to read the T_ATTRIBUTE `@:` syntax
at a glance than the syntax reusing shift left/right tokens. With the
latter my eyes tend to confuse shift left/right tokens in a constant
expression with the open/close token of an attribute (especially since
the syntax using bit shift tokens allows multiple attribute declarations
on the same line).
Consider this example using the shift left/right token syntax:
```php
<> 1, 4 << 1)>><> 1, 4 << 1)>>
function foo() {}
```
To me the same thing using the T_ATTRIBUTE syntax is far more readable:
```php
@:BitShiftExample(4 >> 1, 4 << 1)
@:OtherAttribute(4 >> 1, 4 << 1)
function foo() {}
```
Best regards,
Theodore
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [RFC] Attributes v2
As there has only been minimal new discussion after the last changes to the RFC I wanted to give a heads up that I will open the vote on Monday afternoon. If you have further remarks or questions about the RFC, please let me know. On Mon, Mar 9, 2020 at 3:42 PM Benjamin Eberlei wrote: > Hi all, > > I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in > 2016 with a few changes, incorporating feedback from the mailing list back > then and from talking to previous no voters. > > The RFC is at https://wiki.php.net/rfc/attributes_v2 > > A working patch is at https://github.com/beberlei/php-src/pull/2 though > work around the details is still necessary. > > The RFC contains a section with common criticism and objections to > attributes, and I hope to have collected and responded to a good amount > already from previous discussions. > > There is also a fair amount of implementation detail still up for debate, > which is noted in "Open Issues". I have pre-committed to one approach, but > listed alternatives there. On these issues I am looking for your feedback. > > greetings > Benjamin >
Re: [PHP-DEV] Re: [RFC] Attributes v2
On Thu, Apr 16, 2020 at 4:41 PM Nicolas Grekas
wrote:
> Le jeu. 16 avr. 2020 à 16:29, Larry Garfield a
> écrit :
>
>> On Thu, Apr 16, 2020, at 1:46 AM, Benjamin Eberlei wrote:
>>
>> > > > > 3. I see the most common case for attributes being getting the
>> object
>> > > > > version. With the reflection API as currently described, I see
>> two
>> > > > > shortcomings.
>> > > > >
>> > > > > A) I can't tell if an attribute has a valid object or not before
>> > > trying to
>> > > > > access it, which would presumably fail spectacularly. I believe
>> we
>> > > need a
>> > > > > way to know if getObject() is going to return a valid value before
>> > > trying
>> > > > > to call it. I think this is a hard-requirement.
>> > > > >
>> > > > > B) Related, as is getting all attributes as objects looks to be
>> rather
>> > > > > clunky.
>> > > > >
>> > > > > $attribute_objectgs =
>> > > array_filter(array_map(function(ReflectionAttribute
>> > > > > $r) {
>> > > > > if ($r->getObject()) { // Needs something better here.
>> > > > > return $r->getObject();
>> > > > > }
>> > > > > }, $obj->getAttributes()));
>> > > > >
>> > > > > That's gross. :-) Can "get all the attributes that can be formed
>> into
>> > > > > objects" be its own operation? $obj->getAttributeObjects() or
>> some
>> > > such,
>> > > > > that skips over non-instantiable attributes and instantiates the
>> rest?
>> > > > >
>> > > >
>> > > > I don't see A.) what would you do when the object instantiation
>> fails?
>> > > You
>> > > > would throw an exception I presume, let the engine throw the regular
>> > > > TypeError, ArgumentError, Error if class not exists that everyone is
>> > > > already familiar with.
>> > > >
>> > > > For B.) I believe you are extrapolating based on your own use case.
>> > > Working
>> > > > with Reflection is usually a lot of boilerplate, I don't believe we
>> need
>> > > to
>> > > > have a one liner here.
>> > >
>> > > It depends on the annotation, I suppose. If I'm requesting a specific
>> > > annotation by name, presumably I know if it is supposed to have an
>> > > associated class. If it's supposed to but it's missing, that's a
>> legit
>> > > class-not-found exception/error.
>> > >
>> > > However, I'm thinking of cases where code is integrating with a 3rd
>> party
>> > > optionally, through an annotation. In that case it's a fair question
>> of
>> > > whether the class will be defined or not based on whether some other
>> > > library is present.
>> > >
>> > > Similarly, if a bit of code is requesting all attributes (as above)
>> rather
>> > > than just specific ones by name, it wouldn't know if a given
>> attribute is
>> > > supposed to be defined or not; as written, class-less attributes are
>> > > supported.
>> > >
>> > > I suppose the workaround would be class_exists($r->getName()). Weird
>> but
>> > > I guess works? It would have to be documented as a thing you should
>> do,
>> > > though, which implies to me that it could be made cleaner.
>> > >
>> > > That reflection is usually clunky today (true) is to me not a
>> compelling
>> > > argument that it shouldn't be made less clunky. :-)
>> > >
>> >
>> > You are not safe from these problems when using Doctrine Annotations
>> either
>> > (missing library or class does not exist) and it fails exactly the same
>> way
>> > as trying to instantiate something that doesn't exist.
>> >
>> > I also realized why IS_INSTANCEOF is not the default, because it needs
>> to
>> > resolve all attributes to classes to perform the check. This triggers
>> > autoloading *all* attributes of the reflected declaration (even the ones
>> > not requested), so we felt it should not be the default.
>>
>> Ah, valid. I suppose that's an unavoidable result of allowing
>> non-class-mapped attributes, which means anyone building on it is stuck
>> doing their own class_exists() check for everything.
>>
>> Sad panda. (Still very +1 on the RFC, just sad panda about these
>> details.)
>>
>
>
> Same here, sad panda:
> - we're going to miss nested structure instantly - I wish we could try
> harder here. I'm actually wondering if using the syntax for object literals
> would be the solution here? (we don't have any yet, but some proposals have
> been mentioned recently.
>
Nested attributes are intentionally not part of this RFC, because I believe
it is better to attempt to integrate it with as many existing concepts as
possible, with the potential to benefit from future additions (named
function params, constant AST/expression improvements). Inventing a
completely new syntax will cause future problems for the language, and the
extensibility of attributes themselves.
I can't guarantee you a timeframe for any of that, as mentioned before, I
see it much like scalar types missing nullable in PHP 7. Nikita mentioned
potential improvements to constant AST in a reply to his ctor promotion
RFC, Tyson also discussed it a few weeks ago, and I plan to look into this
myself if this RFC is accepted. And
Re: [PHP-DEV] Re: [RFC] Attributes v2
Le jeu. 16 avr. 2020 à 16:29, Larry Garfield a
écrit :
> On Thu, Apr 16, 2020, at 1:46 AM, Benjamin Eberlei wrote:
>
> > > > > 3. I see the most common case for attributes being getting the
> object
> > > > > version. With the reflection API as currently described, I see two
> > > > > shortcomings.
> > > > >
> > > > > A) I can't tell if an attribute has a valid object or not before
> > > trying to
> > > > > access it, which would presumably fail spectacularly. I believe we
> > > need a
> > > > > way to know if getObject() is going to return a valid value before
> > > trying
> > > > > to call it. I think this is a hard-requirement.
> > > > >
> > > > > B) Related, as is getting all attributes as objects looks to be
> rather
> > > > > clunky.
> > > > >
> > > > > $attribute_objectgs =
> > > array_filter(array_map(function(ReflectionAttribute
> > > > > $r) {
> > > > > if ($r->getObject()) { // Needs something better here.
> > > > > return $r->getObject();
> > > > > }
> > > > > }, $obj->getAttributes()));
> > > > >
> > > > > That's gross. :-) Can "get all the attributes that can be formed
> into
> > > > > objects" be its own operation? $obj->getAttributeObjects() or some
> > > such,
> > > > > that skips over non-instantiable attributes and instantiates the
> rest?
> > > > >
> > > >
> > > > I don't see A.) what would you do when the object instantiation
> fails?
> > > You
> > > > would throw an exception I presume, let the engine throw the regular
> > > > TypeError, ArgumentError, Error if class not exists that everyone is
> > > > already familiar with.
> > > >
> > > > For B.) I believe you are extrapolating based on your own use case.
> > > Working
> > > > with Reflection is usually a lot of boilerplate, I don't believe we
> need
> > > to
> > > > have a one liner here.
> > >
> > > It depends on the annotation, I suppose. If I'm requesting a specific
> > > annotation by name, presumably I know if it is supposed to have an
> > > associated class. If it's supposed to but it's missing, that's a legit
> > > class-not-found exception/error.
> > >
> > > However, I'm thinking of cases where code is integrating with a 3rd
> party
> > > optionally, through an annotation. In that case it's a fair question
> of
> > > whether the class will be defined or not based on whether some other
> > > library is present.
> > >
> > > Similarly, if a bit of code is requesting all attributes (as above)
> rather
> > > than just specific ones by name, it wouldn't know if a given attribute
> is
> > > supposed to be defined or not; as written, class-less attributes are
> > > supported.
> > >
> > > I suppose the workaround would be class_exists($r->getName()). Weird
> but
> > > I guess works? It would have to be documented as a thing you should
> do,
> > > though, which implies to me that it could be made cleaner.
> > >
> > > That reflection is usually clunky today (true) is to me not a
> compelling
> > > argument that it shouldn't be made less clunky. :-)
> > >
> >
> > You are not safe from these problems when using Doctrine Annotations
> either
> > (missing library or class does not exist) and it fails exactly the same
> way
> > as trying to instantiate something that doesn't exist.
> >
> > I also realized why IS_INSTANCEOF is not the default, because it needs to
> > resolve all attributes to classes to perform the check. This triggers
> > autoloading *all* attributes of the reflected declaration (even the ones
> > not requested), so we felt it should not be the default.
>
> Ah, valid. I suppose that's an unavoidable result of allowing
> non-class-mapped attributes, which means anyone building on it is stuck
> doing their own class_exists() check for everything.
>
> Sad panda. (Still very +1 on the RFC, just sad panda about these details.)
>
Same here, sad panda:
- we're going to miss nested structure instantly - I wish we could try
harder here. I'm actually wondering if using the syntax for object literals
would be the solution here? (we don't have any yet, but some proposals have
been mentioned recently.
- having declarative statements possibly turn into failures is also
unfortunate. This makes me wonder if the attributes shouldn't be turned
into plain arrays instead? The logic to hydrate objects would then be moved
to some userland libs. Would that make sense? (I know, that conflicts with
my previous proposal, I'm just trying to explore :))
Nicolas
Re: [PHP-DEV] Re: [RFC] Attributes v2
On Thu, Apr 16, 2020, at 1:46 AM, Benjamin Eberlei wrote:
> > > > 3. I see the most common case for attributes being getting the object
> > > > version. With the reflection API as currently described, I see two
> > > > shortcomings.
> > > >
> > > > A) I can't tell if an attribute has a valid object or not before
> > trying to
> > > > access it, which would presumably fail spectacularly. I believe we
> > need a
> > > > way to know if getObject() is going to return a valid value before
> > trying
> > > > to call it. I think this is a hard-requirement.
> > > >
> > > > B) Related, as is getting all attributes as objects looks to be rather
> > > > clunky.
> > > >
> > > > $attribute_objectgs =
> > array_filter(array_map(function(ReflectionAttribute
> > > > $r) {
> > > > if ($r->getObject()) { // Needs something better here.
> > > > return $r->getObject();
> > > > }
> > > > }, $obj->getAttributes()));
> > > >
> > > > That's gross. :-) Can "get all the attributes that can be formed into
> > > > objects" be its own operation? $obj->getAttributeObjects() or some
> > such,
> > > > that skips over non-instantiable attributes and instantiates the rest?
> > > >
> > >
> > > I don't see A.) what would you do when the object instantiation fails?
> > You
> > > would throw an exception I presume, let the engine throw the regular
> > > TypeError, ArgumentError, Error if class not exists that everyone is
> > > already familiar with.
> > >
> > > For B.) I believe you are extrapolating based on your own use case.
> > Working
> > > with Reflection is usually a lot of boilerplate, I don't believe we need
> > to
> > > have a one liner here.
> >
> > It depends on the annotation, I suppose. If I'm requesting a specific
> > annotation by name, presumably I know if it is supposed to have an
> > associated class. If it's supposed to but it's missing, that's a legit
> > class-not-found exception/error.
> >
> > However, I'm thinking of cases where code is integrating with a 3rd party
> > optionally, through an annotation. In that case it's a fair question of
> > whether the class will be defined or not based on whether some other
> > library is present.
> >
> > Similarly, if a bit of code is requesting all attributes (as above) rather
> > than just specific ones by name, it wouldn't know if a given attribute is
> > supposed to be defined or not; as written, class-less attributes are
> > supported.
> >
> > I suppose the workaround would be class_exists($r->getName()). Weird but
> > I guess works? It would have to be documented as a thing you should do,
> > though, which implies to me that it could be made cleaner.
> >
> > That reflection is usually clunky today (true) is to me not a compelling
> > argument that it shouldn't be made less clunky. :-)
> >
>
> You are not safe from these problems when using Doctrine Annotations either
> (missing library or class does not exist) and it fails exactly the same way
> as trying to instantiate something that doesn't exist.
>
> I also realized why IS_INSTANCEOF is not the default, because it needs to
> resolve all attributes to classes to perform the check. This triggers
> autoloading *all* attributes of the reflected declaration (even the ones
> not requested), so we felt it should not be the default.
Ah, valid. I suppose that's an unavoidable result of allowing non-class-mapped
attributes, which means anyone building on it is stuck doing their own
class_exists() check for everything.
Sad panda. (Still very +1 on the RFC, just sad panda about these details.)
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Attributes v2
On Tue, Apr 14, 2020 at 6:14 PM Larry Garfield
wrote:
> On Tue, Apr 14, 2020, at 10:42 AM, Benjamin Eberlei wrote:
> > On Tue, Apr 14, 2020 at 5:24 PM Larry Garfield
> > wrote:
>
> > > 2. Regarding sub-annotations, can you still do classes as parameters
> even
> > > if not as an annotation marker? Eg:
> > >
> > > <>
> > > function foo()
> > >
> > > Or is that also a no-go?
> > >
> >
> > This is a no go because it would require reimplementing constant ASTs,
> > which is as of now 300 lines of tricky code evaluating ASTs and allowing
> > this would also clash with Bar("Blah") reading like a function call,
> which
> > is confusing and would prevent reconciliation with constant ASTs in the
> > future.
>
> Sad panda.
>
> > > 3. I see the most common case for attributes being getting the object
> > > version. With the reflection API as currently described, I see two
> > > shortcomings.
> > >
> > > A) I can't tell if an attribute has a valid object or not before
> trying to
> > > access it, which would presumably fail spectacularly. I believe we
> need a
> > > way to know if getObject() is going to return a valid value before
> trying
> > > to call it. I think this is a hard-requirement.
> > >
> > > B) Related, as is getting all attributes as objects looks to be rather
> > > clunky.
> > >
> > > $attribute_objectgs =
> array_filter(array_map(function(ReflectionAttribute
> > > $r) {
> > > if ($r->getObject()) { // Needs something better here.
> > > return $r->getObject();
> > > }
> > > }, $obj->getAttributes()));
> > >
> > > That's gross. :-) Can "get all the attributes that can be formed into
> > > objects" be its own operation? $obj->getAttributeObjects() or some
> such,
> > > that skips over non-instantiable attributes and instantiates the rest?
> > >
> >
> > I don't see A.) what would you do when the object instantiation fails?
> You
> > would throw an exception I presume, let the engine throw the regular
> > TypeError, ArgumentError, Error if class not exists that everyone is
> > already familiar with.
> >
> > For B.) I believe you are extrapolating based on your own use case.
> Working
> > with Reflection is usually a lot of boilerplate, I don't believe we need
> to
> > have a one liner here.
>
> It depends on the annotation, I suppose. If I'm requesting a specific
> annotation by name, presumably I know if it is supposed to have an
> associated class. If it's supposed to but it's missing, that's a legit
> class-not-found exception/error.
>
> However, I'm thinking of cases where code is integrating with a 3rd party
> optionally, through an annotation. In that case it's a fair question of
> whether the class will be defined or not based on whether some other
> library is present.
>
> Similarly, if a bit of code is requesting all attributes (as above) rather
> than just specific ones by name, it wouldn't know if a given attribute is
> supposed to be defined or not; as written, class-less attributes are
> supported.
>
> I suppose the workaround would be class_exists($r->getName()). Weird but
> I guess works? It would have to be documented as a thing you should do,
> though, which implies to me that it could be made cleaner.
>
> That reflection is usually clunky today (true) is to me not a compelling
> argument that it shouldn't be made less clunky. :-)
>
You are not safe from these problems when using Doctrine Annotations either
(missing library or class does not exist) and it fails exactly the same way
as trying to instantiate something that doesn't exist.
I also realized why IS_INSTANCEOF is not the default, because it needs to
resolve all attributes to classes to perform the check. This triggers
autoloading *all* attributes of the reflected declaration (even the ones
not requested), so we felt it should not be the default.
> --Larry Garfield
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
Re: [PHP-DEV] Re: [RFC] Attributes v2
On Tue, Apr 14, 2020, at 10:42 AM, Benjamin Eberlei wrote:
> On Tue, Apr 14, 2020 at 5:24 PM Larry Garfield
> wrote:
> > 2. Regarding sub-annotations, can you still do classes as parameters even
> > if not as an annotation marker? Eg:
> >
> > <>
> > function foo()
> >
> > Or is that also a no-go?
> >
>
> This is a no go because it would require reimplementing constant ASTs,
> which is as of now 300 lines of tricky code evaluating ASTs and allowing
> this would also clash with Bar("Blah") reading like a function call, which
> is confusing and would prevent reconciliation with constant ASTs in the
> future.
Sad panda.
> > 3. I see the most common case for attributes being getting the object
> > version. With the reflection API as currently described, I see two
> > shortcomings.
> >
> > A) I can't tell if an attribute has a valid object or not before trying to
> > access it, which would presumably fail spectacularly. I believe we need a
> > way to know if getObject() is going to return a valid value before trying
> > to call it. I think this is a hard-requirement.
> >
> > B) Related, as is getting all attributes as objects looks to be rather
> > clunky.
> >
> > $attribute_objectgs = array_filter(array_map(function(ReflectionAttribute
> > $r) {
> > if ($r->getObject()) { // Needs something better here.
> > return $r->getObject();
> > }
> > }, $obj->getAttributes()));
> >
> > That's gross. :-) Can "get all the attributes that can be formed into
> > objects" be its own operation? $obj->getAttributeObjects() or some such,
> > that skips over non-instantiable attributes and instantiates the rest?
> >
>
> I don't see A.) what would you do when the object instantiation fails? You
> would throw an exception I presume, let the engine throw the regular
> TypeError, ArgumentError, Error if class not exists that everyone is
> already familiar with.
>
> For B.) I believe you are extrapolating based on your own use case. Working
> with Reflection is usually a lot of boilerplate, I don't believe we need to
> have a one liner here.
It depends on the annotation, I suppose. If I'm requesting a specific
annotation by name, presumably I know if it is supposed to have an associated
class. If it's supposed to but it's missing, that's a legit class-not-found
exception/error.
However, I'm thinking of cases where code is integrating with a 3rd party
optionally, through an annotation. In that case it's a fair question of
whether the class will be defined or not based on whether some other library is
present.
Similarly, if a bit of code is requesting all attributes (as above) rather than
just specific ones by name, it wouldn't know if a given attribute is supposed
to be defined or not; as written, class-less attributes are supported.
I suppose the workaround would be class_exists($r->getName()). Weird but I
guess works? It would have to be documented as a thing you should do, though,
which implies to me that it could be made cleaner.
That reflection is usually clunky today (true) is to me not a compelling
argument that it shouldn't be made less clunky. :-)
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Attributes v2
On Tue, Apr 14, 2020 at 5:24 PM Larry Garfield
wrote:
> On Tue, Apr 14, 2020, at 6:00 AM, Benjamin Eberlei wrote:
> > Hi everyone,
> >
> > I have updated the RFC with much of the feedback received here, on
> Twitter
> > and Reddit.
> >
> > https://wiki.php.net/rfc/attributes_v2
> >
> > The following changes were made:
> >
> >- Changed to support the same attribute multiple times on the same
> >declaration
> >- Added support for attributes on method and function parameters
> >- Replaced *PhpAttribute* interface with an attribute instead
> >- Distinction between userland and compiler attributes and description
> >when each of them gets evaluated/validated
> >- Reduce number of examples to shorten RFC a bit and expand the other
> >examples instead
> >- Introduced validation of compiler attributes at compile time using
> a C
> >callback
> >- Offer alternative syntax “@:” using new token T_ATTRIBUTE which will
> >be included with a secondary vote
> >
> > You may have seen me mentioning that I don't want to deviate from the
> <<>>
> > syntax, a topic of heated debate. As Martin helped me tremendously with
> the
> > RFC and patches he earned to propose an alternative (including patch with
> > prototype). So we will have a secondary vote on syntax being either
> > <> or @:Attribute.
> >
> > Let us know what you think about the changes.
> >
> > greetings
> > Benjamin
>
> This looks lovely and I look forward to being able to use it!
>
> Questions:
>
> 1. Why is exact-match the default for getAttributes(), and "instanceof" an
> extra flag? I would expect it to the other way around. The whole point of
> LSP is that any subclass is a viable replacement for its parent; if not,
> You're Doing It Wrong(tm). It also means that requesting by interface
> mandates adding the second parameter or else it will always return
> nothing. What is the reason for not making instanceof the default match
> and offering an EXACT opt-in mode?
>
Yes, you make a good point here.
>
> 2. Regarding sub-annotations, can you still do classes as parameters even
> if not as an annotation marker? Eg:
>
> <>
> function foo()
>
> Or is that also a no-go?
>
This is a no go because it would require reimplementing constant ASTs,
which is as of now 300 lines of tricky code evaluating ASTs and allowing
this would also clash with Bar("Blah") reading like a function call, which
is confusing and would prevent reconciliation with constant ASTs in the
future.
>
> 3. I see the most common case for attributes being getting the object
> version. With the reflection API as currently described, I see two
> shortcomings.
>
> A) I can't tell if an attribute has a valid object or not before trying to
> access it, which would presumably fail spectacularly. I believe we need a
> way to know if getObject() is going to return a valid value before trying
> to call it. I think this is a hard-requirement.
>
> B) Related, as is getting all attributes as objects looks to be rather
> clunky.
>
> $attribute_objectgs = array_filter(array_map(function(ReflectionAttribute
> $r) {
> if ($r->getObject()) { // Needs something better here.
> return $r->getObject();
> }
> }, $obj->getAttributes()));
>
> That's gross. :-) Can "get all the attributes that can be formed into
> objects" be its own operation? $obj->getAttributeObjects() or some such,
> that skips over non-instantiable attributes and instantiates the rest?
>
I don't see A.) what would you do when the object instantiation fails? You
would throw an exception I presume, let the engine throw the regular
TypeError, ArgumentError, Error if class not exists that everyone is
already familiar with.
For B.) I believe you are extrapolating based on your own use case. Working
with Reflection is usually a lot of boilerplate, I don't believe we need to
have a one liner here.
>
> This isn't a requirement, but without it I predict virtually everyone
> using attributes is going to have to recreate the knot of code above.
>
> Thanks again!
>
> --Larry Garfield
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
Re: [PHP-DEV] Re: [RFC] Attributes v2
On Tue, Apr 14, 2020, at 6:00 AM, Benjamin Eberlei wrote:
> Hi everyone,
>
> I have updated the RFC with much of the feedback received here, on Twitter
> and Reddit.
>
> https://wiki.php.net/rfc/attributes_v2
>
> The following changes were made:
>
>- Changed to support the same attribute multiple times on the same
>declaration
>- Added support for attributes on method and function parameters
>- Replaced *PhpAttribute* interface with an attribute instead
>- Distinction between userland and compiler attributes and description
>when each of them gets evaluated/validated
>- Reduce number of examples to shorten RFC a bit and expand the other
>examples instead
>- Introduced validation of compiler attributes at compile time using a C
>callback
>- Offer alternative syntax “@:” using new token T_ATTRIBUTE which will
>be included with a secondary vote
>
> You may have seen me mentioning that I don't want to deviate from the <<>>
> syntax, a topic of heated debate. As Martin helped me tremendously with the
> RFC and patches he earned to propose an alternative (including patch with
> prototype). So we will have a secondary vote on syntax being either
> <> or @:Attribute.
>
> Let us know what you think about the changes.
>
> greetings
> Benjamin
This looks lovely and I look forward to being able to use it!
Questions:
1. Why is exact-match the default for getAttributes(), and "instanceof" an
extra flag? I would expect it to the other way around. The whole point of LSP
is that any subclass is a viable replacement for its parent; if not, You're
Doing It Wrong(tm). It also means that requesting by interface mandates adding
the second parameter or else it will always return nothing. What is the reason
for not making instanceof the default match and offering an EXACT opt-in mode?
2. Regarding sub-annotations, can you still do classes as parameters even if
not as an annotation marker? Eg:
<>
function foo()
Or is that also a no-go?
3. I see the most common case for attributes being getting the object version.
With the reflection API as currently described, I see two shortcomings.
A) I can't tell if an attribute has a valid object or not before trying to
access it, which would presumably fail spectacularly. I believe we need a way
to know if getObject() is going to return a valid value before trying to call
it. I think this is a hard-requirement.
B) Related, as is getting all attributes as objects looks to be rather clunky.
$attribute_objectgs = array_filter(array_map(function(ReflectionAttribute $r) {
if ($r->getObject()) { // Needs something better here.
return $r->getObject();
}
}, $obj->getAttributes()));
That's gross. :-) Can "get all the attributes that can be formed into objects"
be its own operation? $obj->getAttributeObjects() or some such, that skips
over non-instantiable attributes and instantiates the rest?
This isn't a requirement, but without it I predict virtually everyone using
attributes is going to have to recreate the knot of code above.
Thanks again!
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Attributes v2
On Tue, Apr 14, 2020 at 3:04 PM Nicolas Grekas
wrote:
> Hi Benjamin,
>
> I have updated the RFC with much of the feedback received here, on Twitter
>> and Reddit.
>>
>> https://wiki.php.net/rfc/attributes_v2
>
>
> Thanks for the update and for giving this a try!
>
> I'm wondering about nested annotations: as you know, they're quite common
> in apps that use doctrine/annotation.
> How do you think these should be migrated to the new system?
>
Nested attributes are not supported in this step, because they might
interfere with potential changes to constant AST. Tyson dipped into that
topic a few weeks ago.
If constants were to allow something like const $foo = new Foo(); - then
this would automatically be available for attributes (they use the same
concept).
For now I suppose the approach would be to flatten the attributes, for
example:
@JoinTable(name="users_posts", joinColumns={@JoinColumn(name="id",
referencedColumnName="user_id"}, inverseJoinColumns={@JoinColumn(name="id",
"referencedColumnName="post_id"})
becomes:
<>
<>
<>
As you can see with this example, named arguments is another under
construction area in the language that would automatically spill its
benefits to attributes.
Think of this like scalar types in 7.0 that missed nullable, which was
added in 7.1.
> Nicolas
>
Re: [PHP-DEV] Re: [RFC] Attributes v2
On Tue, Apr 14, 2020 at 2:56 PM Iván Arias wrote: > > > Hi everyone, > > > > I have updated the RFC with much of the feedback received here, on > Twitter > and Reddit. > > > > https://wiki.php.net/rfc/attributes_v2 > > > Hi Benjamin, > > Thanks a lot for you effort, same for Martin. I really hope to see this in > PHP8. > > I have a simple question. From the RFC: > > > Attributes are added before the declaration they belong to, similar to > do-block comments. They can be declared before or after a doc-block > comment that documents a declaration. > > Could annotations be declared before AND after the doc-block? > What would be the behavior in that case? > Attributes are handled as a list, so you always append to them. So it is sort of like merging the list of attributes before and after the doc comment. See here: https://gist.github.com/beberlei/6cf033a3d2cd1a8d991fcabdecce0626 Regards, > Iván Arias. >
Re: [PHP-DEV] Re: [RFC] Attributes v2
Hi Benjamin, I have updated the RFC with much of the feedback received here, on Twitter > and Reddit. > > https://wiki.php.net/rfc/attributes_v2 Thanks for the update and for giving this a try! I'm wondering about nested annotations: as you know, they're quite common in apps that use doctrine/annotation. How do you think these should be migrated to the new system? Nicolas
Re: [PHP-DEV] Re: [RFC] Attributes v2
> Hi everyone, > > I have updated the RFC with much of the feedback received here, on Twitter and Reddit. > > https://wiki.php.net/rfc/attributes_v2 Hi Benjamin, Thanks a lot for you effort, same for Martin. I really hope to see this in PHP8. I have a simple question. From the RFC: > Attributes are added before the declaration they belong to, similar to > do-block comments. They can be declared before or after a doc-block comment > that documents a declaration. Could annotations be declared before AND after the doc-block? What would be the behavior in that case? Regards, Iván Arias.
[PHP-DEV] Re: [RFC] Attributes v2
Hi everyone, I have updated the RFC with much of the feedback received here, on Twitter and Reddit. https://wiki.php.net/rfc/attributes_v2 The following changes were made: - Changed to support the same attribute multiple times on the same declaration - Added support for attributes on method and function parameters - Replaced *PhpAttribute* interface with an attribute instead - Distinction between userland and compiler attributes and description when each of them gets evaluated/validated - Reduce number of examples to shorten RFC a bit and expand the other examples instead - Introduced validation of compiler attributes at compile time using a C callback - Offer alternative syntax “@:” using new token T_ATTRIBUTE which will be included with a secondary vote You may have seen me mentioning that I don't want to deviate from the <<>> syntax, a topic of heated debate. As Martin helped me tremendously with the RFC and patches he earned to propose an alternative (including patch with prototype). So we will have a secondary vote on syntax being either <> or @:Attribute. Let us know what you think about the changes. greetings Benjamin On Mon, Mar 9, 2020 at 3:42 PM Benjamin Eberlei wrote: > Hi all, > > I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in > 2016 with a few changes, incorporating feedback from the mailing list back > then and from talking to previous no voters. > > The RFC is at https://wiki.php.net/rfc/attributes_v2 > > A working patch is at https://github.com/beberlei/php-src/pull/2 though > work around the details is still necessary. > > The RFC contains a section with common criticism and objections to > attributes, and I hope to have collected and responded to a good amount > already from previous discussions. > > There is also a fair amount of implementation detail still up for debate, > which is noted in "Open Issues". I have pre-committed to one approach, but > listed alternatives there. On these issues I am looking for your feedback. > > greetings > Benjamin >
Re: [PHP-DEV] Re: [RFC] Attributes v2
pon., 9 mar 2020 o 15:45 Benjamin Eberlei napisał(a): > On Mon, Mar 9, 2020 at 3:42 PM Benjamin Eberlei > wrote: > > > Hi all, > > > > I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in > > 2016 with a few changes, incorporating feedback from the mailing list > back > > then and from talking to previous no voters. > > > > The RFC is at https://wiki.php.net/rfc/attributes_v2 > > > > A working patch is at https://github.com/beberlei/php-src/pull/2 though > > work around the details is still necessary. > > > > The RFC contains a section with common criticism and objections to > > attributes, and I hope to have collected and responded to a good amount > > already from previous discussions. > > > > There is also a fair amount of implementation detail still up for debate, > > which is noted in "Open Issues". I have pre-committed to one approach, > but > > listed alternatives there. On these issues I am looking for your > feedback. > > > > Obviously I am looking for feedback on the whole RFC, not just the open > issues :-) > > > > > greetings > > Benjamin > > > Hi Benjamin, I'm really glad you're taking up the baton! IMHO a bunch of straw polls could provide a way more beneficial knowledge about feelings people who can vote up for this feature. What do you think? It could be good to discover the negative or positive feelings as well as "I'm not opposite" feelings. >From what I can see now and I didn't think of it earlier are some details but we should ask the question how far we wanna go with this feature for its an initial shape. Are we gonna provide complete attributes feature like C#, Java etc. which allows for annotating annotations (whatever it's called for me annotations/attributes has no difference for now) meaning they allow to put metadata which puts restrictions on: 1. Usage of the attribute should be allowed once or multiple times regarding the same target? 2. Targetting attributes should be annotated - no matter if through an interface or annotation? 3. Allow for inheritance should be annotated - meaning when reading attributes for class Bar which extends Foo on which attributes are set, should I get attributes or not? 4. Should attributes be resolvable all the time, what if the class won't exist on runtime? In some languages like Java there are for eg. documentary annotations which live with code but are not stored on runtime. Or we simply want to provide a simple way of retrieving the metadata from class/trait/interface/properties/methods/functions without checking their repeateness, without inheritance, without validating the targets where they appear, and just allow to move any kind of attributes into the language and allow the userland to decide what to do with them and how to handle them properly? The thing which I haven't consider earlier and I find it nice features is the ability to filter annotations against some abstract/base annotation - it may be described as a better cause I do feel like it should be done through extending of the interface and not by an abstract class. Like from the RFC itself the `\Validator\Attributes\Attribute::class` it wouldn't make sense for it to be a class, not even an abstract class, there's probably no method which this abstract will provide. Attributes should be forbidden for instantiation and this if funny cause they do have a public __construct so they look like classes but are not actually classes which you should be allowed to instantiate wherever you want, right? So we agree it looks a little weird here but dunno if there's something we can do. cheers, Michał Brzuchalski
Re: [PHP-DEV] Re: [RFC] Attributes v2
On Mon, Mar 9, 2020 at 8:30 PM David Rodrigues wrote: > As I am not a good expert on parser (not to say that I don't do anything), > could you tell me if I can write a note like that? > > <<[space]Annotation()[space]>> > << MyAnnotation(1, 2, 3) >> > > It's just because I think the code is more "breathable". Until the PSR > staff decides how best to write. > Yes, you can add as many spaces as you want between << Attr >> On your other question: It is named Attributes in C#, C++, Rust and Hack, so this name is not something novell. The naming discussion is a coin flip issue in my opinion. You can't really argue to name it Annotations, just because Doctrine uses it this way, because historically it would have been docblock "tags" how php-doc named them. > > > Atenciosamente, > David Rodrigues > > > Em seg., 9 de mar. de 2020 às 16:19, Andrea Faulds escreveu: > > > Benjamin Eberlei wrote: > > > I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in > > > 2016 with a few changes, incorporating feedback from the mailing list > > back > > > then and from talking to previous no voters. > > > > > > The RFC is at https://wiki.php.net/rfc/attributes_v2 > > > > Hi, > > > > I have concerns about these two statements in the RFC: > > > > > The name of an attribute is resolved against the currently active > > namespace import scope during compilation. The resolved class names are > > then autoloaded to make sure they exist. > > > > > Consistent with PHP expressions in general, no validation is > > performed if the provided attribute arguments are fullfilling the > > contract of the attribute class constructor. This would happen only when > > accessing attributes as objects in the Reflection API (below). > > > > These two details are inconsistent with eachother: use of an annotation > > triggers an autoload, yet we aren't using the class that is autoloaded > > to validate it? This seems quite wasteful: if we have loaded the class, > > we might as well use it to check the arguments are correct. Also, why > > are we privileging the class existing over the arguments to the class > > being correct? If the arguments can be validated at Reflection time, > > surely the autoloading can be done then too? Both types of coding > > mistake are important. > > > > It also seems inconsistent with existing PHP behaviour, I think normally > > mentioning a class either triggers an immediate autoload and actual > > execution/validation (`new`) or it doesn't (a type declaration). This > > proposal is a strange half-way house. > > > > Is this being done to avoid paying the cost of creating the object at > > compilation time? Because I think triggering the autoload is going to be > > expensive anyway, possibly moreso. > > > > On a different note, the wording here is syntactically ambiguous. It can > > be read as both "if the provided attribute arguments are fullfilling the > > contract […], then no validation is performed" and "no validation is > > performed as to whether the provided attribute arguments are fullfilling > > the contract". I read it as the former the first time, which confused me > > for a moment. > > > > Another thing: > > > > > Thanks to class name resolving, IDEs or static analysis tools can > > perform this validation for the developer. > > > > Is this referencing the autoloading behaviour? I don't see why that > > would be required. (You could also be referring to the fact you use > > classes, which IDEs can look for, instead of arbitrary string > > attributes, which IDEs can not, which does make sense.) > > > > Thanks, > > Andrea > > > > -- > > PHP Internals - PHP Runtime Development Mailing List > > To unsubscribe, visit: http://www.php.net/unsub.php > > > > >
Re: [PHP-DEV] Re: [RFC] Attributes v2
On Mon, Mar 9, 2020 at 8:19 PM Andrea Faulds wrote:
> Benjamin Eberlei wrote:
> > I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in
> > 2016 with a few changes, incorporating feedback from the mailing list
> back
> > then and from talking to previous no voters.
> >
> > The RFC is at https://wiki.php.net/rfc/attributes_v2
>
> Hi,
>
> I have concerns about these two statements in the RFC:
>
> > The name of an attribute is resolved against the currently active
> namespace import scope during compilation. The resolved class names are
> then autoloaded to make sure they exist.
>
> > Consistent with PHP expressions in general, no validation is
> performed if the provided attribute arguments are fullfilling the
> contract of the attribute class constructor. This would happen only when
> accessing attributes as objects in the Reflection API (below).
>
> These two details are inconsistent with eachother: use of an annotation
> triggers an autoload, yet we aren't using the class that is autoloaded
> to validate it? This seems quite wasteful: if we have loaded the class,
> we might as well use it to check the arguments are correct. Also, why
> are we privileging the class existing over the arguments to the class
> being correct? If the arguments can be validated at Reflection time,
> surely the autoloading can be done then too? Both types of coding
> mistake are important.
>
> It also seems inconsistent with existing PHP behaviour, I think normally
> mentioning a class either triggers an immediate autoload and actual
> execution/validation (`new`) or it doesn't (a type declaration). This
> proposal is a strange half-way house.
>
> Is this being done to avoid paying the cost of creating the object at
> compilation time? Because I think triggering the autoload is going to be
> expensive anyway, possibly moreso.
>
Thinking about how I got to my thought process here, I believe it was
something like
When compiling
new Foo("abcdefg", 1234);
then it resolves the class name (does it autoload?) but it wouldn't check
if the arguments match until its actually executed.
I believe I am wrong to propose autoloading directly at compiling the code
is not how PHP does it. It should autoload only during
Reflection*::getAttributes(). this is something I haven't implemented in
the prototype PR yet, maybe I would have seen the problems.
Going to mule this one over and adjust the RFC and this thread with an
update.
> On a different note, the wording here is syntactically ambiguous. It can
> be read as both "if the provided attribute arguments are fullfilling the
> contract […], then no validation is performed" and "no validation is
> performed as to whether the provided attribute arguments are fullfilling
> the contract". I read it as the former the first time, which confused me
> for a moment.
>
Will update to clarify.
>
> Another thing:
>
> > Thanks to class name resolving, IDEs or static analysis tools can
> perform this validation for the developer.
>
> Is this referencing the autoloading behaviour? I don't see why that
> would be required. (You could also be referring to the fact you use
> classes, which IDEs can look for, instead of arbitrary string
> attributes, which IDEs can not, which does make sense.)
>
That is right, an IDE can find this out without the autoloading behavior.
This sentence means like you inferred, that because attributes are
semantically mapping to class names, that is the reason why IDEs can better
validate them in their own static analysis.
>
> Thanks,
> Andrea
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
Re: [PHP-DEV] Re: [RFC] Attributes v2
As I am not a good expert on parser (not to say that I don't do anything), could you tell me if I can write a note like that? <<[space]Annotation()[space]>> << MyAnnotation(1, 2, 3) >> It's just because I think the code is more "breathable". Until the PSR staff decides how best to write. Atenciosamente, David Rodrigues Em seg., 9 de mar. de 2020 às 16:19, Andrea Faulds escreveu: > Benjamin Eberlei wrote: > > I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in > > 2016 with a few changes, incorporating feedback from the mailing list > back > > then and from talking to previous no voters. > > > > The RFC is at https://wiki.php.net/rfc/attributes_v2 > > Hi, > > I have concerns about these two statements in the RFC: > > > The name of an attribute is resolved against the currently active > namespace import scope during compilation. The resolved class names are > then autoloaded to make sure they exist. > > > Consistent with PHP expressions in general, no validation is > performed if the provided attribute arguments are fullfilling the > contract of the attribute class constructor. This would happen only when > accessing attributes as objects in the Reflection API (below). > > These two details are inconsistent with eachother: use of an annotation > triggers an autoload, yet we aren't using the class that is autoloaded > to validate it? This seems quite wasteful: if we have loaded the class, > we might as well use it to check the arguments are correct. Also, why > are we privileging the class existing over the arguments to the class > being correct? If the arguments can be validated at Reflection time, > surely the autoloading can be done then too? Both types of coding > mistake are important. > > It also seems inconsistent with existing PHP behaviour, I think normally > mentioning a class either triggers an immediate autoload and actual > execution/validation (`new`) or it doesn't (a type declaration). This > proposal is a strange half-way house. > > Is this being done to avoid paying the cost of creating the object at > compilation time? Because I think triggering the autoload is going to be > expensive anyway, possibly moreso. > > On a different note, the wording here is syntactically ambiguous. It can > be read as both "if the provided attribute arguments are fullfilling the > contract […], then no validation is performed" and "no validation is > performed as to whether the provided attribute arguments are fullfilling > the contract". I read it as the former the first time, which confused me > for a moment. > > Another thing: > > > Thanks to class name resolving, IDEs or static analysis tools can > perform this validation for the developer. > > Is this referencing the autoloading behaviour? I don't see why that > would be required. (You could also be referring to the fact you use > classes, which IDEs can look for, instead of arbitrary string > attributes, which IDEs can not, which does make sense.) > > Thanks, > Andrea > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >
[PHP-DEV] Re: [RFC] Attributes v2
Benjamin Eberlei wrote: I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in 2016 with a few changes, incorporating feedback from the mailing list back then and from talking to previous no voters. The RFC is at https://wiki.php.net/rfc/attributes_v2 Hi, I have concerns about these two statements in the RFC: > The name of an attribute is resolved against the currently active namespace import scope during compilation. The resolved class names are then autoloaded to make sure they exist. > Consistent with PHP expressions in general, no validation is performed if the provided attribute arguments are fullfilling the contract of the attribute class constructor. This would happen only when accessing attributes as objects in the Reflection API (below). These two details are inconsistent with eachother: use of an annotation triggers an autoload, yet we aren't using the class that is autoloaded to validate it? This seems quite wasteful: if we have loaded the class, we might as well use it to check the arguments are correct. Also, why are we privileging the class existing over the arguments to the class being correct? If the arguments can be validated at Reflection time, surely the autoloading can be done then too? Both types of coding mistake are important. It also seems inconsistent with existing PHP behaviour, I think normally mentioning a class either triggers an immediate autoload and actual execution/validation (`new`) or it doesn't (a type declaration). This proposal is a strange half-way house. Is this being done to avoid paying the cost of creating the object at compilation time? Because I think triggering the autoload is going to be expensive anyway, possibly moreso. On a different note, the wording here is syntactically ambiguous. It can be read as both "if the provided attribute arguments are fullfilling the contract […], then no validation is performed" and "no validation is performed as to whether the provided attribute arguments are fullfilling the contract". I read it as the former the first time, which confused me for a moment. Another thing: > Thanks to class name resolving, IDEs or static analysis tools can perform this validation for the developer. Is this referencing the autoloading behaviour? I don't see why that would be required. (You could also be referring to the fact you use classes, which IDEs can look for, instead of arbitrary string attributes, which IDEs can not, which does make sense.) Thanks, Andrea -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [RFC] Attributes v2
On Mon, Mar 9, 2020 at 3:42 PM Benjamin Eberlei wrote: > Hi all, > > I want to resurrect Dmitrys Attributes RFC that was rejected for 7.1 in > 2016 with a few changes, incorporating feedback from the mailing list back > then and from talking to previous no voters. > > The RFC is at https://wiki.php.net/rfc/attributes_v2 > > A working patch is at https://github.com/beberlei/php-src/pull/2 though > work around the details is still necessary. > > The RFC contains a section with common criticism and objections to > attributes, and I hope to have collected and responded to a good amount > already from previous discussions. > > There is also a fair amount of implementation detail still up for debate, > which is noted in "Open Issues". I have pre-committed to one approach, but > listed alternatives there. On these issues I am looking for your feedback. > Obviously I am looking for feedback on the whole RFC, not just the open issues :-) > > greetings > Benjamin >
