On Thu, May 7, 2020 at 10:27 AM Davey Shafik <da...@php.net> wrote:
>
>
> On Thu, May 7, 2020 at 01:18 Benjamin Eberlei <kont...@beberlei.de> wrote:
>
>>
>>
>> On Thu, May 7, 2020 at 10:10 AM Davey Shafik <da...@php.net> wrote:
>>
>>>
>>>
>>> On Thu, May 7, 2020 at 00:22 Benjamin Eberlei <kont...@beberlei.de>
>>> wrote:
>>>
>>>> Hi everyone,
>>>>
>>>> The attributes RFC specifically looked into the use of attributes at the
>>>> engine/compiler level.
>>>>
>>>> To have a showcase of this with the 8.0 release I want to propose this
>>>> RFC
>>>> for a <<Deprecated>> attribute:
>>>>
>>>> https://wiki.php.net/rfc/deprecated_attribute
>>>>
>>>> <<Deprecated>> is the most obvious engine attribute to add at the
>>>> beginning
>>>> in my opinion, as
>>>>
>>>> - all other languages with Attributes/Annotations support have it as
>>>> well
>>>> - its effects are quite obvious to understand, as such offer a good
>>>> example
>>>> and intro to attributes
>>>> - with potential property/class constant deprecation it adds something
>>>> that
>>>> was not possible in userland before.
>>>>
>>>> Let me know what you think!
>>>>
>>>> greetings
>>>> Benjamin
>>>
>>>
>>> Hi Benjamin,
>>>
>>> Two things:
>>>
>>> 1) as a matter of course, I prefer things that can be done in userland
>>> be done in userland unless there is a clear performance or other win. This
>>> doesn't qualify for me, except that it would be possible to use the same
>>> mechanism in core, but that really only gives us the ability to reason
>>> around depredations with reflection and I don't see much value in that. I'm
>>> not entirely convinced that a PSR and a defecto implementation isn't a
>>> better solution here. This isn't a show-stopper though.
>>>
>>
>> Can you clarify a bit your argument "ability to reason around
>> deprecations only with reflection"?
>>
>> At the moment we can only reason about deprecations by using both
>> token_get_all and finding that a method calls trigger_error
>> E_USER_DEPRECATED and by looking at the doc comments (via reflection) for
>> "@deprecated". My argument for this attribute is that it leads to both a
>> documentation based deprecation, and a runtime declaration at the same
>> time. It is a better result than what we can do in userland in my opinion.
>>
>
> I don't see any valid reason to be checking for deprecations at runtime
> with reflection. This attribute has more value when reading the code or
> doing static analysis. Neither of which requires that it be implemented in
> core. Plus, userland gives us the reflection ability *anyway*.
>
This attribute is "reflected" by the engine during compilation, userland
doesn't have to reflect it again itself. on a code level the engine
"patches" each function automatically, this would do the following at
compile time:
@@ -1,6 +1,6 @@
<?php
-<<Deprecated>>
function foo() {
+ trigger_error('Function foo() is deprecated', E_USER_DEPRECATED);
return 'foo';
}
>
>
>>
>>
>>>
>>> 2) AFAICT you cannot add an attribute to a function argument, and the
>>> only way I can see to resolve that is to add a "DeprecatedArgument"
>>> attribute applied to the function that takes the name as the first arg (and
>>> the message as the second). Now you're looking at:
>>>
>>> <<DeprecatedArgument('foo')>>
>>> <<DeprecatedArgument('bar')>>
>>> function bat($foo = null, $bar = null) { }
>>>
>>> It's not great IMO. Having said that, I don't think this bit is really
>>> necessary — unlike the other suggested uses, an argument isn't an isolated
>>> thing, so much as part of the function signature as a whole and removing
>>> arguments is better done by deprecating the entire function and introducing
>>> a new version with a different name. So maybe we just don't need this?
>>>
>>
>> It is possible to add attributes to parameters. This was added as an
>> improvement during the Attributes RFC (it was not possible in the first
>> draft).
>>
>
> I went back and looked and still missed that, oops! I still stand by my
> statement about it being bad practice ;)
>
Yes you might be right that the parameter deprecation is the one with the
"least" value from all of them.
>
> - Davey
>
