[VOTE] Remove inappropriate inheritance signature checks on private methods

Hey Nikita,

On Tue, Jun 16, 2020 at 11:05 AM Nikita Popov [email protected]
wrote:

Hi internals,

I have opened the vote on the "Remove inappropriate inheritance
signature
checks on private methods" RFC. Which can be found here:

https://wiki.php.net/rfc/inheritance_private_methods

The voting period will end on 2020-06-29 22:00 UTC.

Thanks to those who participated in the discussion and provided
feedback.

Voting no on this one, for the reason I previously mentioned on the PR:
While the abstract and static parts of the RFC make sense, the final part
does not (to me). Changing the behavior of private final just removes
existing functionality, without any clear benefit that I can see. What is
the problem that change tries to solve?

Possibly missing in the RFC, but a parent class declaring a new private
symbol shouldn't affect private API in child classes that may be
pre-existing: helps backwards compatibility.

Right, and I totally agree with that part insofar as it concerns
static/abstract handling. However, "private final" in particular is not an
accidental addition in the base class, it's an explicit design decision to
forbid certain behaviors in child classes.

The original RFC could at least

make a consistency argument, but the final form, which converts an
existing
special case into an even more convoluted special case, can not. Why is
__construct() exempted, but __clone() for example isn't, even though
similar considerations apply to it?

While private final function __construct() prevents child classes from
making the constructor open, or accessing it (important for abstract
types), protected final function __clone() achieves the same without
having to rely on the private visibility modifier (
https://3v4l.org/psR5F, for example).

It is true that cloning is then possible from within a subtype, which is
indeed a bit of a problem: https://3v4l.org/qupHR

Maybe the magic methods would indeed all need to respect final, and we
haven't considered all scenarios?

Or ... we could just not change this part of the behavior at all :) If we
acknowledge that this is definitely useful for constructors and possibly
useful for cloning, then maybe it is useful for other things as well? I
still haven't heard a reason why we would want to do this change (not the
general change, but specifically the "private final" one).

This stuff really isn't super important to me, and I'd be happy to
sacrifice this functionality in the name of the greater good, but I
honestly don't understand what that greater good is in this case. It seems
like a strict loss in functionality.

From how I understood it, right now "private final" cannot possibly
have a real world use-case for other cases except for __construct and
possibly for other fixed named magic methods like __clone.

Someone can use it in a parent class and a child class developed by
someone would have a restriction on what private method it can use to
do his own private stuff.
In reality, that's easily circumvented by choosing a different private
name but this RFC will allow the user to use his prefered name for the
private method. That's mainly why it's useful from my point of view.

Regards,
Alex

Maybe the magic methods would indeed all need to respect final, and we
haven't considered all scenarios?

Or ... we could just not change this part of the behavior at all :) If we
acknowledge that this is definitely useful for constructors and possibly
useful for cloning, then maybe it is useful for other things as well? I
still haven't heard a reason why we would want to do this change (not the
general change, but specifically the "private final" one).

This stuff really isn't super important to me, and I'd be happy to
sacrifice this functionality in the name of the greater good, but I
honestly don't understand what that greater good is in this case. It seems
like a strict loss in functionality.

Regards,
Nikita

Hi Nikita,

The motivations for changing the behavior of final private are the
following:

• Theoretical: Even though private members are technically inherited by
  child classes, they are not overridable. You can already have public
  methods in a child class with the same name as a private parent, but you
  are not overriding it (if you call that method in the parent, you will use
  the private method declared there. If you call it in the child, you will
  use the public method declared there - https://3v4l.org/TImO8). You can
  have different visibility, different signatures (thanks to a bugfix that
  was actually my first contribution to PHP :) ) and the methods are
  completely unrelated except that they share the name. Sharing the name
  shouldn't be a reason to enforce any inheritance rules.
• Implementation: My preferred way to implement this would have been to
  skip the call to do_inheritance_check_on_method_ex on private methods
  completely, as that would make the intention very clear. The only reason
  why I couldn't is the check for abstract private methods coming from
  traits. And currently, the ZEND_ACC_CHANGED flag is also added there, but I
  think that could be easily avoided.
• Other languages:
  • Java: https://www.ideone.com/P71k8J - final has no effect when added
    to a private method (
    https://www.javaworld.com/article/2077399/private-and-final.html)
  • C#: https://www.ideone.com/i1OtFe - In C#, for a method to be
    overridable it has to be marked as virtual. In this snippet, we are not
    marking it as virtual (hence it acts as final) and nothing prevents us from
    declaring it in the child class.

Regards,
Pedro