Everything

php

[RFC][VOTE] Allow abstract function override

php.internals

Net Mo

9 years ago

Hello PHPeeps,

There hasn't been much discussion around the proposed feature, and I've
interpreted it as a good sign :P It is not a super important change but it
has some advantages, it's consistent with the recent improvements to type
variance and also with future ones. Also, it's hopefully not too hard to
implement.

https://wiki.php.net/rfc/allow-abstract-function-override

I've decided to start the vote. It will end two weeks from this message, on
10th April 2017.

Thanks in advance for participating.

Wes

Fleshgrinder

9 years ago

On 3/28/2017 6:50 AM, Wes wrote:

> Hello PHPeeps,
>
> There hasn't been much discussion around the proposed feature, and I've
> interpreted it as a good sign :P It is not a super important change but it
> has some advantages, it's consistent with the recent improvements to type
> variance and also with future ones. Also, it's hopefully not too hard to
> implement.
>
> https://wiki.php.net/rfc/allow-abstract-function-override
>
> I've decided to start the vote. It will end two weeks from this message, on
> 10th April 2017.
>
> Thanks in advance for participating.
>
> Wes
>

There is absolutely no BC break at all in this RFC. The current status
of things makes no sense because it is inconsistent. Heck, even the fact
that it is inconsistent shows that there are different rules for the
same things. Semantically there is no difference between a class, an
abstract class, or an interface. They are all classes, some simply omit
implementation details and leave it to their subclasses.

I hope this gets a yes, regardless of possible use cases, this is simply
applying proper variant rules everywhere.

--
Richard "Fleshgrinder" Fussenegger

Marco Pivetta

9 years ago

Hey Folks,

Sorry, my mistake on detecting the BC break. I had a misconception about
how `ReflectionMethod#getDeclaringClass()` works. I thought that the
declaring class was the declaring abstraction, but it's actually always
pointing to the most concrete implementation in the inheritance.

See https://3v4l.org/t7PiL

Code, for reference:

<?php

class Thing {}

interface Foo {
public function bar($baz) : Thing;
}

interface Bar extends Foo {
public function bar($baz) : Thing;
}

final class Baz implements Bar
{
public function bar($baz) : Thing
{
return new Thing;
}
}

var_dump((new
ReflectionClass(Foo::class))->getMethod('bar')->getDeclaringClass()->getName());
// expected `Foo`, it actually is `Foo`
var_dump((new
ReflectionClass(Bar::class))->getMethod('bar')->getDeclaringClass()->getName());
// expected `Foo`, got `Bar` (my assumption was wrong)
var_dump((new
ReflectionClass(Baz::class))->getMethod('bar')->getDeclaringClass()->getName());
// expected `Foo`, got `Baz` (my assumption was wrong)

This is my mistake, as I didn't verify my assumption on code before
reporting the BC break. Sorry again - changing my vote to `yes`, as the
discussed covariance/contravariance changes make sense for interfaces as
well.

Greets,

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

Net Mo

9 years ago

No problem. I instead thank you for checking this, because I forgot to
check it myself (sorry).

Patrick ALLAERT

9 years ago

Le mar. 28 mars 2017 à 06:51, Wes <netmo.php@gmail.com> a écrit :

> Hello PHPeeps,
>
> There hasn't been much discussion around the proposed feature, and I've
> interpreted it as a good sign :P It is not a super important change but it
> has some advantages, it's consistent with the recent improvements to type
> variance and also with future ones. Also, it's hopefully not too hard to
> implement.
>
> https://wiki.php.net/rfc/allow-abstract-function-override
>
> I've decided to start the vote. It will end two weeks from this message, on
> 10th April 2017.
>
> Thanks in advance for participating.
>
> Wes
>

Hello,

Voted "no" because the RFC starts with:

"
PHP 7 introduced type declarations for return and improved ones for
parameters. They currently support a very limited kind of variance (only to
no-type):

<?php
class A { function bar(stdClass $x){} }
class B extends A { function bar($x): stdClass{} }
"

Which looks false in both PHP 7.0 and PHP 7.1.

Sorry if I missed something in a past discussions, but it looks wrong to me
to implement anything based on a false assertion.

I would be happy to change my vote if you can fix the RFC and/or enlight me.

Cheers,
Patrick

Niklas Keller

9 years ago

2017-04-10 11:34 GMT+02:00 Patrick ALLAERT <patrickallaert@php.net>:

> Le mar. 28 mars 2017 à 06:51, Wes <netmo.php@gmail.com> a écrit :
>
> > Hello PHPeeps,
> >
> > There hasn't been much discussion around the proposed feature, and I've
> > interpreted it as a good sign :P It is not a super important change but
> it
> > has some advantages, it's consistent with the recent improvements to type
> > variance and also with future ones. Also, it's hopefully not too hard to
> > implement.
> >
> > https://wiki.php.net/rfc/allow-abstract-function-override
> >
> > I've decided to start the vote. It will end two weeks from this message,
> on
> > 10th April 2017.
> >
> > Thanks in advance for participating.
> >
> > Wes
> >
>
> Hello,
>
> Voted "no" because the RFC starts with:
>
> "
> PHP 7 introduced type declarations for return and improved ones for
> parameters. They currently support a very limited kind of variance (only to
> no-type):
>
> <?php
> class A { function bar(stdClass $x){} }
> class B extends A { function bar($x): stdClass{} }
> "
>
> Which looks false in both PHP 7.0 and PHP 7.1.
>
> Sorry if I missed something in a past discussions, but it looks wrong to me
> to implement anything based on a false assertion.
>
> I would be happy to change my vote if you can fix the RFC and/or enlight
> me.
>
> Cheers,
> Patrick
>

It's implemented in PHP 7.2 (master).

Regards, Niklas

Net Mo

9 years ago

Hi Patrick, this is basically the completion of what Niklas did in his RFC
and that PHP 7.0 started. I could've being more clear :) PHP 7 introduced
variance (variance from mixed) for returns, then Niklas added the same
thing for parameters (variance to mixed) and this extends the support to
abstract declarations as well.

I'm closing the vote as 2 weeks have passed. Thanks to all the voters!

Patrick ALLAERT

9 years ago

Le mar. 11 avr. 2017 à 06:32, Wes <netmo.php@gmail.com> a écrit :

> Hi Patrick, this is basically the completion of what Niklas did in his RFC
> and that PHP 7.0 started. I could've being more clear :) PHP 7 introduced
> variance (variance from mixed) for returns, then Niklas added the same
> thing for parameters (variance to mixed) and this extends the support to
> abstract declarations as well.
>
> I'm closing the vote as 2 weeks have passed. Thanks to all the voters!
>

Yes, I got the intention after digging more. I would also have changed my
vote to 'Yes', but it passes, so it's all good!

Thanks for the clarification!
Patrick