Everything

php

Initial discussion - more deprecation options

php.internals

Daniel Scherzer

1 year ago

Hi internals

I'd like to start off some preliminary discussion about expanding the places
that `#[\Deprecated]` can be used, and more widely how userland code can
have
the engine trigger deprecation warnings (rather than just manually calling
`trigger_error()`). This is intended to be a follow-up to the addition of
attributes on constants - it is now possible to use attributes so that the
engine emits warnings when

* Calling a function or method
* Accessing a class constant
* Accessing a global constant

My inspiration is MediaWiki's "Stable Interface Policy"[1] - there are some
things that MediaWiki (and other libraries) may want to deprecate, but that
do
not trigger deprecation warnings, or require work-arounds to trigger them
manually with `trigger_error()`. Specifically, I want to get the community's
thoughts on adding ways to

* Deprecate `use`ing traits
* Deprecate extending a class
* Deprecate overriding a method
* Deprecate implementing an interface
* Deprecate accessing a public/protected property
* Deprecate class aliases (maybe a new way to declare aliases with
`#[\ClassAlias]`?)

Having learned my lesson from the never-parameters RFC, I want to get a
sense
of support/opposition before spending time on the implementations. I also
want
a sense of which parts would need individual RFCs, and if those RFCs would
be
welcomed without first providing an implementation.

- Daniel

[1] https://www.mediawiki.org/wiki/Stable_interface_policy

Larry Garfield

1 year ago

On Wed, May 7, 2025, at 12:50 PM, Daniel Scherzer wrote:

> Hi internals
>
> I'd like to start off some preliminary discussion about expanding the places
> that `#[\Deprecated]` can be used, and more widely how userland code can have
> the engine trigger deprecation warnings (rather than just manually calling
> `trigger_error()`). This is intended to be a follow-up to the addition of
> attributes on constants - it is now possible to use attributes so that the
> engine emits warnings when
>
> * Calling a function or method
> * Accessing a class constant
> * Accessing a global constant
>
> My inspiration is MediaWiki's "Stable Interface Policy"[1] - there are some
> things that MediaWiki (and other libraries) may want to deprecate, but that do
> not trigger deprecation warnings, or require work-arounds to trigger them
> manually with `trigger_error()`. Specifically, I want to get the community's
> thoughts on adding ways to
>
> * Deprecate `use`ing traits
> * Deprecate extending a class
> * Deprecate overriding a method
> * Deprecate implementing an interface
> * Deprecate accessing a public/protected property
> * Deprecate class aliases (maybe a new way to declare aliases with
> `#[\ClassAlias]`?)
>
> Having learned my lesson from the never-parameters RFC, I want to get a sense
> of support/opposition before spending time on the implementations. I also want
> a sense of which parts would need individual RFCs, and if those RFCs would be
> welcomed without first providing an implementation.
>
> - Daniel
>
> [1] https://www.mediawiki.org/wiki/Stable_interface_policy

I am in favor of expanding the deprecation attribute to more places.  I believe the ask previously (when the attribute was first introduced) was to keep it in sync with C-level deprecations.  Viz, being able to deprecate an interface should be added for both C code and user-space code at the same time, so they're parallel.  I think that is a reasonable requirement, and any RFCs on the topic should follow that recommendation.

RFCs can be proposed regardless of implementation status.  Technically the rules allow the to be voted on and even approved without a patch, but I believe that is generally regarded as uncooth, at best, as implementation details can often dictate design.  (Absolutely true for much of the stuff Ilija and I work on.)  I think the de facto rule is probably "you can talk about anything, but don't call a vote without at least a mostly-working patch that people have reviewed."

--Larry Garfield

Niels Dossche

1 year ago

Hi

Definitely NAK on deprecating properties, which has the potential of adding lots of overhead.
Note also that cache slot merging (during compact_literals optimizer pass) when opcache is active may dodge the deprecation notice, and I don't want more VM workarounds for then doing this anyway.

Kind regards
Niels

Tim Düsterhus

1 year ago

Hi

Am 2025-05-07 21:43, schrieb Niels Dossche:

> Definitely NAK on deprecating properties, which has the potential of
> adding lots of overhead.

FWIW: Deprecating properties is already possibly by means of deprecating
a property hook. I guess this is an acceptable workaround to not require
a dedicated support + associated complexity.

see: https://3v4l.org/C6jRQ

class Foo
{
public string $foo
{
#[\Deprecated]
get {
return $this->foo;
}

#[\Deprecated]
set {
$this->foo = $value;
}
}
}

$f = new Foo();
$f->foo = 'dummy';
var_dump($f->foo);

Best regards
Tim Düsterhus

Niels Dossche

1 year ago

On 07/05/2025 22:02, Tim Düsterhus wrote:

> Hi
>
> Am 2025-05-07 21:43, schrieb Niels Dossche:
>> Definitely NAK on deprecating properties, which has the potential of adding lots of overhead.
>
> FWIW: Deprecating properties is already possibly by means of deprecating a property hook. I guess this is an acceptable workaround to not require a dedicated support + associated complexity.
>

Sure, since that goes via method logic, and hooks should be rare (by design), I don't have a problem with the fact that's possible.
It's a reasonable workaround that you can already use today indeed.

Kind regards
Niels

Daniel Scherzer

1 year ago

On Thu, May 8, 2025 at 8:38 AM Tim Düsterhus <tim@bastelstu.be> wrote:

> Hi
>
> Am 2025-05-07 21:43, schrieb Niels Dossche:
> > Definitely NAK on deprecating properties, which has the potential of
> > adding lots of overhead.
>
> FWIW: Deprecating properties is already possibly by means of deprecating
> a property hook. I guess this is an acceptable workaround to not require
> a dedicated support + associated complexity.
>
> see: https://3v4l.org/C6jRQ
>
>      class Foo
>      {
>          public string $foo
>          {
>              #[\Deprecated]
>              get {
>                  return $this->foo;
>              }
>
>              #[\Deprecated]
>              set {
>                  $this->foo = $value;
>              }
>          }
>      }
>
>      $f = new Foo();
>      $f->foo = 'dummy';
>      var_dump($f->foo);
>
>

So this example would emit deprecation warnings on all getting and setting
operations, but I was thinking that they would *not* be emitted when being
accessed from within the same class (private scope). Consider a project that
* does not want to use hooks (which add overhead)
* wants to support the deprecated behavior

It would then want to
* emit warnings when a property is accessed by protected/public scope
* but need to handle any writes from outside of the class by accessing the
property internally to see if the value changed

I'm not sure what NAK means in this context, but I think that allowing
`#[\Deprecated]` directly on the property instead of needing to use hooks
be done in the same way that the property hooks were implemented in terms
of dealing with VM handling and opcache, where things are not cached (I
assume, haven't looked into this enough, this is just a preliminary
discussion).

Allowing deprecation directly on properties would also be more performant
than needing to use hooks.

-Daniel

Ilija Tovilo

1 year ago

Hi Daniel

On Thu, May 8, 2025 at 5:48 PM Daniel Scherzer
<daniel.e.scherzer@gmail.com> wrote:

>
> So this example would emit deprecation warnings on all getting and setting operations, but I was thinking that they would *not* be emitted when being accessed from within the same class (private scope). Consider a project that
> * does not want to use hooks (which add overhead)
> * wants to support the deprecated behavior
>
> It would then want to
> * emit warnings when a property is accessed by protected/public scope
> * but need to handle any writes from outside of the class by accessing the property internally to see if the value changed
>
> I'm not sure what NAK means in this context, but I think that allowing `#[\Deprecated]` directly on the property instead of needing to use hooks be done in the same way that the property hooks were implemented in terms of dealing with VM handling and opcache, where things are not cached (I assume, haven't looked into this enough, this is just a preliminary discussion).
>
> Allowing deprecation directly on properties would also be more performant than needing to use hooks.

I would echo what Niels said. Deprecations for internal functions were
added before my time, and it made sense to expand this functionality
to userspace, as the two should generally be congruent. I am not in
favor of adding more runtime deprecations to hot code.

Runtime deprecations are not a great solution in the first place,
deprecations are much easier to catch through static analysis.
Function deprecations made sense for us because we needed them
internally, but I don't think we have a use-case for the properties.
If they were trivial and free to add to runtime, I wouldn't mind as
much. But they are neither of those things, in addition to being a bad
solution to the original problem (reminding you to fix your code).
Hence, I would prefer not adding more checks.

Ilija