Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:92734
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 157.56.110.108 as permitted sender)
To: "guilhermeblanco@gmail.com" <guilhermeblanco@gmail.com>
References: <BY2PR0201MB178479B3AF8BD49583233034BF6E0@BY2PR0201MB1784.namprd02.prod.outlook.com>
 <CAFKaYMqg6H2YAgS-ATjcYtXw8GLbp_Kr+VuZFMRPZTkAS1g0hA@mail.gmail.com>
 <571965D1.9020102@zend.com>
 <CAFp73Xs=-sK4ZM-mM8Jp5wWkpYLuen+-K0R9xfZaumBM_sPPFw@mail.gmail.com>
 <5719CDB2.90103@zend.com>
 <CAFp73XtqBtLdoTykrg_1eeJZTFszcmutNpynmyGKEizPiC2Juw@mail.gmail.com>
 <571DCA6A.2070803@zend.com>
 <CAFp73XsVJ=1QxvwLbqvDw8o3BD6Ly7BjrP5X+68O=z6iH-brWA@mail.gmail.com>
CC: Dominic Grostate <codekestrel@googlemail.com>, PHP internals
	<internals@lists.php.net>
Message-ID: <571E35D8.8080504@zend.com>
Date: Mon, 25 Apr 2016 18:20:56 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.7.1
MIME-Version: 1.0
In-Reply-To: <CAFp73XsVJ=1QxvwLbqvDw8o3BD6Ly7BjrP5X+68O=z6iH-brWA@mail.gmail.com>
Content-Type: multipart/alternative;
	boundary="------------010700020904020102090900"
SpamDiagnosticOutput: 1:23
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Apr 2016 15:21:13.6400 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BLUPR0201MB1777
Subject: Re: [PHP-DEV] [RFC] PHP Attributes
From: dmitry@zend.com (Dmitry Stogov)

--------------010700020904020102090900
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit



On 04/25/2016 05:11 PM, guilhermeblanco@gmail.com wrote:
>
>
> On Mon, Apr 25, 2016 at 3:42 AM, Dmitry Stogov <dmitry@zend.com 
> <mailto:dmitry@zend.com>> wrote:
>
>
>
>     On 04/22/2016 06:39 PM, guilhermeblanco@gmail.com
>     <mailto:guilhermeblanco@gmail.com> wrote:
>>
>>     On Fri, Apr 22, 2016 at 3:07 AM, Dmitry Stogov <dmitry@zend.com
>>     <mailto:dmitry@zend.com>> wrote:
>>
>>
>>
>>         On 04/22/2016 04:05 AM, guilhermeblanco@gmail.com
>>         <mailto:guilhermeblanco@gmail.com> wrote:
>>>         Hi Dmitry,
>>>
>>>         As a previous suggester of metadata information built-in
>>>         into PHP, and also one of developers of the most used
>>>         metadata library written in PHP, I understand this feature
>>>         implementation requires several design decisions and also a
>>>         good understanding of specific situations users may require.
>>>
>>>         While I am a strong supporter of a more robust solution,
>>>         this is already a good start.
>>>         A few things I'd like to ask for my own understanding and
>>>         also suggestions too:
>>>
>>>         1- I understand you took a minimalistic approach towards a
>>>         "dumb" implementation for attributes (when I mean "dumb",
>>>         the idea here is towards a non-OO approach). Can you explain
>>>         your motivations towards this approach?
>>>
>>>         I see two distinct approaches of implementation for this
>>>         feature. Both of them have some common demands, like lazy
>>>         initialization of metadata. Here they are:
>>>
>>>         - Simplistic approach, which lets consumers of the feature
>>>         do all the work related to validation, assertion of valid
>>>         keys, values, etc
>>>         This does not invalidate the ability to leverage of some
>>>         features that a more robust implementation demands.
>>>
>>>         - Robust approach: language takes the burden of
>>>         instantiating complex structures, validating, assertion of
>>>         valid keys, values, if this complex structure is allowed to
>>>         be instantiated in that given class, method, etc.
>>
>>         I didn't exactly understand what do you suggest.
>>         If you are talking about Attribute objects initialization
>>         during compilation - this is just not possible from
>>         implementation point of view.
>>         Now attributes may be stored in opcache SHM and relive
>>         request boundary.
>>         Objects can't relive requests.
>>
>>
>>
>>     I know that object instances are not cross-requests. Explicitly,
>>     I mentioned that both approaches require lazy-initialization
>>     (which means, whenever you call getAttributes() or getAttribute()).
>>
>>     What I mentioning is that your approach is basically a new
>>     key/value syntax that are used specifically for Attributes. We
>>     could easily turn this into a more robust approach if instead of
>>     defining key/value pairs, we instantiate objects or call
>>     functions. You already demonstrated interest to support
>>     <<ORM\Entity>> reusing the imports (which is our biggest headache
>>     in Doctrine Annotations), so why not issue constructor or
>>     function calls there? That would simplify the work needed for
>>     consumers and also add room for later improvements.
>>     So basically in this example:
>>
>>     use Doctrine\ORM;
>>
>>     <<ORM\Entity("user")>>
>>     class User {}
>>
>>     $reflClass = new \ReflectionClass("User");
>>     var_dump($reflClass->getAttributes());
>>
>>     We'd be changing from this:
>>
>>     array(1) {
>>     ["Doctrine\ORM\Entity"]=>
>>     array(1) {
>>     [0]=>
>>     string(4) "user"
>>       }
>>     }
>>
>>     Into this:
>>
>>     array(1) {
>>     ["Doctrine\ORM\Entity"]=>
>>     object(Doctrine\ORM\Entity)#1 (1) {
>>     ["tableName"]=>
>>     string(4) "user"
>>       }
>>     }
>
>     As I showed already, it's very easy to do this transformation at
>     higher layer.
>
>     $reflClass = new \ReflectionClass("User");
>     $attributes = $reflClass->getAttributes()
>     foreach ($attributes as $key => &$val) {
>         $val = new $key(...$val);
>     }
>     var_dump($attributes);
>
>     Construction objects directly in Reflection*::getAttributes()
>     method, doesn't make significant benefits and even makes limitation.
>
>
> Sorry, but I don't see how limitations are added. If you call a 
> function, static method or constructor, you actually add whole new 
> level of possibilities, and I fail to see which limitations are added. 
> Could you provide me one?

For example, I like to check an attribute existence, and I don't need to 
construct any objects.
>
> Calling the function/constructor/static method, not only helps to 
> better segregate userland code, but it also adds subsequents 
> extensibility. I can highlight examples:
>
> - Support for Inheritance and overrides, through @Inherit, @Override, 
> etc. While you might not see how it could be used now, other 
> developers might be weirdly creative.
> - Targeting of annotations, such as limiting its scope to be only 
> class, method or property. We use this extensively in Doctrine, where 
> you cannot define Doctrine\ODM\Entity over a property.
> - Separating what can be considered as an annotation and what cannot. 
> Built-in @Annotation as a marker would differentiate that I can do 
> call Doctrine\ORM\Entity and not Doctrine\ORM\UnitOfWork.
> - Make it easier to support an AOP extension, where it could detect 
> annotations being used and override DO_FCALL to call before, after or 
> around through the implementation of interfaces.
> - If we ever decide to support named parameters, taking advantage of 
> that would become natural, like: <<ORM\Entity(tableName => "user")>>

See a new example at https://wiki.php.net/rfc/attributes#use_cases
It's already works.
I don't see any reason to rewrite that few PHP lines in C.
>
>
>
>>
>>
>>>
>>>         1- Your approach is basically defining an array. Could you
>>>         explain your line of thinking on why you didn't consider a
>>>         syntax like the one below?
>>>
>>>         <["key" => "value"]>
>>>         class Foo {}
>>         I didn't try to invite new syntax. Just completely took it
>>         from HHVM.
>>
>>
>>     My idea was based on your current proposal, which is basically a
>>     way to define key/value pairs.
>>     If you decide to go minimalistic, that is probably my best line
>>     of thinking.
>>
>>
>>
>>>
>>>         2- I see that you added support over functions, classes,
>>>         constants and properties. According to the RFC,
>>>         getAttributes() was added over ReflectionFunction. Is there
>>>         a reason why support was not added to methods
>>>         (ReflectionMethod extends ReflectionFunctionAbstract, which
>>>         was not mentioned on RFC)? Any reason to not support it in
>>>         function/method parameters?
>>         ReflectionMethod is a child of ReflectinFunction, so it's
>>         supported.
>>
>>         Attributes are allowed for the same entities as doc-comments
>>         (they are not allowed for parameters)
>>
>>
>>     I was asking if there was a purpose to not support Attributes
>>     over ReflectionParameter. Example:
>>
>>     class Foo {
>>     public function bar(<<Qux>> Bar $bar) : bool {
>>     // ...
>>         }
>>     }
>>
>>     $reflClass = new \ReflectionClas("Foo");
>>     $reflMethod = $reflClass->getMethod("bar");
>>     $reflParameter = $reflMethod->getParameters()[0];
>>
>>     var_dump($reflParameter->getAttributes());
>
>     I understood, we may add this ability later.
>
>
> I'd say we should add this from day one.
> A quick use case that comes to my mind are parameters conversion that 
> happens in Symfony2 through their "extra" bundle (doc: 
> http://symfony.com/doc/current/bundles/SensioFrameworkExtraBundle/annotations/converters.html 
> ). In a controller action (it's a class method), you have the ability 
> to convert the Request object into something else that makes more 
> sense for you. Example:
>
> class UserController extends Controller {
>     public function viewAction(<<UserParameterConverter("userId")>> 
> User $user = null) {
>         if ($user === null) {
>             throw new NotFoundException("User not found");
>         }
>
>         return ["me" => $this->getUser(), "user" => $user];
>     }
> }

I'll take a look into this, but you see, I'm not very interested in 
wasting time implementing particular low-level details while the whole 
idea is not accepted yet.

>
>>
>>
>>>
>>>         3- Did you put any thought on inheritance? What I mentioned
>>>         in comment #1 is even smaller than what you implemented in RFC.
>>>         Assuming you keep the RFC approach, did you consider support
>>>         overrides, inherit, etc?
>>
>>         In my opinion, attributes don't have to be inherited.
>>         If you think differently - please explain your point.
>>
>>
>>     Of source I can.
>>     A simple case would be to increate visibility of the inherited
>>     property. It was declared in a parent class as protected, but now
>>     you want public, and you still want to keep all parent defined
>>     Attributes.
>     Very questionable. If you redefine property, it shouldn't inherit
>     attributes.
>
>
> This leads to some serious copy/paste, highly error prone... =(

If we had a theoretical approach for attribute inheritance, I would 
implement it.
But I wouldn't invite any theory, because anyone is going to depend on 
use-case.

>
>
>>     Another example is like we do in Doctrine. We support a callback
>>     system which we named as lifetime callbacks. Pre-persist is one
>>     of them, which is called every time a given Entity is about to be
>>     persisted into DB. When you're dealing with inheritance, you can
>>     potentially override the method content and you still want to
>>     trigger the same operation as if it was untouched. Example:
>>
>>     use Doctrine\ORM;
>>
>>     trait Timestampable {
>>     protected $created;
>>     protected $updated;
>>
>>     <<ORM\PrePersist>>
>>     public function prePersist() {
>>     $this->created = $this->updated = new \DateTime("now");
>>         }
>>
>>     <<ORM\PreUpdate>>
>>     public function preUpdate() {
>>     $this->updated = new \DateTime("now");
>>         }
>>     }
>>
>>     <<ORM\Entity>>
>>     class User {
>>         use Timestampable;
>>
>>     public function prePersist() {
>>     // Add my custom logic
>>         }
>>     }
>>     The implication is that through a simplistic approach, inheriting
>>     (or overriding) is not clear and I can't figure it out an easy
>>     way to achieve that.
>>     Now if we go towards calling a function or class constructor like
>>     I mentioned before, then we could easily build structures like
>>     __Inherit, __Override, etc.
>     It's definitely, not clear when attribute inheritance make sense
>     and when completely not. For example, if we mark some method to be
>     JIT-ed, it doesn't mean that we like to JIT methods of all
>     children. So, I prefer not to do inheritance at all. The higher
>     layers may emulate "inheritance" of some attributes their selves
>     (like you do this with doc-comments).
>
>
> As I said earlier, if you do a call based approach, we could create 
> @Inherit or @Override, which would not only make us safe from support, 
> but also gives more power to developers.

If we implement built-in @Inherit and/or @Override, it's not a big 
problem to copy attributes from parent.

Thanks. Dmitry.
>
>
>
>>
>>
>>>
>>>         4- I understand that a more robust attribute solution would
>>>         be required to achieve this, but one of the biggest
>>>         advantages of AOP is the ability to perform custom logic
>>>         before, after or around... However, I don't know if any kind
>>>         of triggers came in your head or are planned as a future RFC.
>>>         Let me highlight one example: Every time a class, property
>>>         or method is called that is annotated as <<deprecated>>, I
>>>         would like to issue an E_USER_DEPRECATED warning. A
>>>         trigger-like solution would be required. Did this concept
>>>         came to your mind?
>>         This is not a subject of this RFC.
>>         Attributes provides a storage for metadata, but don't define
>>         how to use them.
>>         Especially, for your use-case:
>>         1) it's possible to create preprocessor that embeds
>>         corresponding trigger_error() call
>>         2) it's possible to write a PHP extension that plugs-into
>>         compiler chain and checks <<deprecated>> attribute for each
>>         compiles function, then sets ZEND_ACC_DEPRECATED flag
>>         3) It's also possible to override DO_FCALL opcodes and
>>         perform checks there (this is inefficient)
>>
>>
>>     With this simplistic approach, I agree there's 0 value into
>>     considering this.
>>     However, taking a more robust approach would potentially open
>>     this possibility through a simpler extension.
>
>     You saw, Sara named even this proposed solution a bit over-designed.
>     it make no sense to implement all functionality at language level.
>     Actually, keeping simple base interface, opens doors for more
>     use-cases.
>
>     Thanks. Dmitry.
>
>
>>
>>         Thanks. Dmitry.
>>
>>
>>>
>>>
>>>
>>>         Regards,
>>>
>>>         On Thu, Apr 21, 2016 at 7:44 PM, Dmitry Stogov
>>>         <dmitry@zend.com <mailto:dmitry@zend.com>> wrote:
>>>
>>>
>>>
>>>             On 04/22/2016 02:16 AM, Dominic Grostate wrote:
>>>
>>>
>>>                 This is amazing.  It would actually allow us to
>>>                 implement our automated assertions ourselves, as
>>>                 opposed to requiring it within the language.
>>>
>>>             this was the idea - to give a good tool instead of
>>>             implementing every possible use-case in the language.
>>>
>>>                 Could it also support references?
>>>
>>>                 <<sanitize(&$a)>>
>>>                 function foo($a) {
>>>
>>>                 }
>>>
>>>             yes. "&$a" is a valid PHP expression.
>>>
>>>             If you plan to use this, I would appreciate, if you to
>>>             build the patched PHP and try it.
>>>             The early we find problems the better feature we will
>>>             get at the end.
>>>
>>>             Thanks. Dmitry.
>>>
>>>
>>>                 On 21 Apr 2016 10:13 p.m., "Dmitry Stogov"
>>>                 <dmitry@zend.com <mailto:dmitry@zend.com>
>>>                 <mailto:dmitry@zend.com <mailto:dmitry@zend.com>>>
>>>                 wrote:
>>>
>>>                     Hi,
>>>
>>>
>>>                     I would like to present an RFC proposing support
>>>                 for native
>>>                     annotation.
>>>
>>>                     The naming, syntax and behavior are mostly
>>>                 influenced by HHVM
>>>                     Hack, but not exactly the same.
>>>
>>>                     The most interesting difference is an ability to
>>>                 use arbitrary PHP
>>>                     expressions as attribute values.
>>>
>>>                     These expressions are not evaluated, but stored
>>>                 as Abstract Syntax
>>>                     Trees, and later may be accessed (node by node)
>>>                 in PHP extensions,
>>>                     preprocessors and PHP scripts their selves. I
>>>                 think this ability
>>>                     may be useful for "Design By Contract", other
>>>                 formal verification
>>>                     systems, Aspect Oriented Programming, etc
>>>
>>>
>>>                 https://wiki.php.net/rfc/attributes
>>>
>>>
>>>                     Note that this approach is going to be native,
>>>                 in contrast to
>>>                     doc-comment approach that uses not well defined
>>>                 syntax, and even
>>>                     not parsed by PHP itself.
>>>
>>>
>>>                     Additional ideas, endorsement and criticism are
>>>                 welcome.
>>>
>>>
>>>                     Thanks. Dmitry.
>>>
>>>
>>>
>>>
>>>
>>>         -- 
>>>         Guilherme Blanco
>>>         Lead Architect at E-Block
>>
>>
>>
>>
>>     -- 
>>     Guilherme Blanco
>>     Lead Architect at E-Block
>
>
>
>
> -- 
> Guilherme Blanco
> Lead Architect at E-Block


--------------010700020904020102090900--