Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:92708
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 157.56.110.119 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>
CC: Dominic Grostate <codekestrel@googlemail.com>, PHP internals
	<internals@lists.php.net>
Message-ID: <571DCA6A.2070803@zend.com>
Date: Mon, 25 Apr 2016 10:42:34 +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: <CAFp73XtqBtLdoTykrg_1eeJZTFszcmutNpynmyGKEizPiC2Juw@mail.gmail.com>
Content-Type: multipart/alternative;
	boundary="------------050808030000050405020508"
SpamDiagnosticOutput: 1:23
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Apr 2016 07:42:48.7681 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR0201MB1791
Subject: Re: [PHP-DEV] [RFC] PHP Attributes
From: dmitry@zend.com (Dmitry Stogov)

--------------050808030000050405020508
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit



On 04/22/2016 06:39 PM, 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.

>
>
>>
>>     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.
>
>
>
>>
>>     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.

> 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).

>
>
>>
>>     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


--------------050808030000050405020508--