On Mon, Apr 25, 2016 at 3:42 AM, Dmitry Stogov <dmi...@zend.com> wrote:

>
>
> On 04/22/2016 06:39 PM, guilhermebla...@gmail.com wrote:
>
>
> On Fri, Apr 22, 2016 at 3:07 AM, Dmitry Stogov <dmi...@zend.com> wrote:
>
>>
>>
>> On 04/22/2016 04:05 AM, <guilhermebla...@gmail.com>
>> guilhermebla...@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?

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")>>



>
>
>
>
>>
>> 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];
    }
}


>
>
>
>>
>>
>>
>> 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... =(


>
>
> 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.


>
>
>
>
>>
>>
>> 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 < <dmi...@zend.com>
>> dmi...@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" <dmi...@zend.com <mailto:
>>>> dmi...@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

Reply via email to