Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:109635
MIME-Version: 1.0
References: <CAEPJdunSr8PkBtSLO8g_JMLzM4cFVnXsQSEMFmu2RN25XT=grw@mail.gmail.com>
 <CAEPJdumF8GaAqLj+=gNi-dMP1GYSvk_DYJuy3gG0N5J1vSXRqQ@mail.gmail.com> <16620f3a-1eb6-437e-aa6f-05e5be2aa713@www.fastmail.com>
In-Reply-To: <16620f3a-1eb6-437e-aa6f-05e5be2aa713@www.fastmail.com>
Date: Tue, 14 Apr 2020 17:42:32 +0200
Message-ID: <CAEPJdun15CfEjxBsbMbnv8XWHnOvdZ0fnB-E1aYsrPbRWavVTw@mail.gmail.com>
To: Larry Garfield <larry@garfieldtech.com>
Cc: php internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000e6662805a3420c37"
Subject: Re: [PHP-DEV] Re: [RFC] Attributes v2
From: kontakt@beberlei.de (Benjamin Eberlei)

--000000000000e6662805a3420c37
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Apr 14, 2020 at 5:24 PM Larry Garfield <larry@garfieldtech.com>
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 descripti=
on
> >    when each of them gets evaluated/validated
> >    - Reduce number of examples to shorten RFC a bit and expand the othe=
r
> >    examples instead
> >    - Introduced validation of compiler attributes at compile time using
> a C
> >    callback
> >    - Offer alternative syntax =E2=80=9C@:=E2=80=9D using new token T_AT=
TRIBUTE 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 wi=
th
> > prototype). So we will have a secondary vote on syntax being either
> > <<Attribute>> 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" a=
n
> 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:
>
> <<Foo(1, "B", Bar('blah'))>>
> 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 t=
o
> 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 =3D array_filter(array_map(function(ReflectionAttribu=
te
> $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
>
>

--000000000000e6662805a3420c37--