# [RFC][VOTE] Allow abstract function override - Thread: https://externals.io/message/98646 - Original archive: https://news-web.php.net/php.internals/98646 - Started by netmo.php@gmail.com on 2017-03-28T04:50:58+00:00 - Messages: 15 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #98646 — netmo.php@gmail.com — 2017-03-28T04:50:58+00:00 URL: https://externals.io/message/98646#98646 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 --- ## #98647 — Marco Pivetta — 2017-03-28T04:58:33+00:00 URL: https://externals.io/message/98646#98647 Voted "no" because of this paragraph: > This RFC proposes to allow this, even if it has very few uses, and because there is no reason to disallow a compatible redefinition. I couldn't see a use-case. In addition to that, this breaks the assumption that the topmost abstract signature is the authoritative one. That is a subtle BC break, but still a BC break. Yes, I know I'm annoying, sorry 😥 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 --- ## #98648 — netmo.php@gmail.com — 2017-03-28T05:14:27+00:00 URL: https://externals.io/message/98646#98648 · In reply to: #98647 Yes, yes you are :D Quick reminder:​ ```php interface iA { function bar(stdClass $x); } interface iB extends iA { function bar($x): stdClass; } // OK class cA { function bar(stdClass $x) {} } class cB extends cA { function bar($x): stdClass {} } // OK in 7.2 abstract class cC { abstract function bar(stdClass $x); } abstract class cD extends cC { abstract function bar($x): stdClass; } // Fatal error: why? class cE extends cD { function bar($x): stdClass{} } ``` --- ## #98649 — Derick Rethans — 2017-03-28T10:31:40+00:00 URL: https://externals.io/message/98646#98649 · In reply to: #98647 > Voted "no" because of this paragraph: > > > […] > > I couldn't see a use-case. > > In addition to that, this breaks the assumption that the topmost abstract > signature is the authoritative one. That is a subtle BC break, but still a > BC break. Yes, I know I'm annoying, sorry 😥 As there is a subtle BC break, then that needs to be mentioned in the RFC under "Backward Incompatible Changes". cheers, Derick --- ## #98650 — Marco Pivetta — 2017-03-28T11:43:09+00:00 URL: https://externals.io/message/98646#98650 · In reply to: #98648 Hey Wes, Yes, yes you are :D Quick reminder:​ ```php interface iA { function bar(stdClass $x); } interface iB extends iA { function bar($x): stdClass; } // OK class cA { function bar(stdClass $x) {} } class cB extends cA { function bar($x): stdClass {} } // OK in 7.2 abstract class cC { abstract function bar(stdClass $x); } abstract class cD extends cC { abstract function bar($x): stdClass; } // Fatal error: why? class cE extends cD { function bar($x): stdClass{} } ``` What is this useful for, precisely? Also, I really never saw the need for re-defining the signature in multiple abstract inheritance levels (a single abstract class​ already smells in most scenarios). --- ## #98653 — netmo.php@gmail.com — 2017-03-28T14:56:47+00:00 URL: https://externals.io/message/98646#98653 · In reply to: #98650 Hi again. There is no BC break, at all. --- ## #98655 — Bob Weinand — 2017-03-28T16:00:07+00:00 URL: https://externals.io/message/98646#98655 · In reply to: #98647 > Am 28.03.2017 um 06:58 schrieb Marco Pivetta : > > Voted "no" because of this paragraph: > > > […] > because there is no reason to disallow a compatible redefinition. > > I couldn't see a use-case. > > In addition to that, this breaks the assumption that the topmost abstract > signature is the authoritative one. That is a subtle BC break, but still a > BC break. Yes, I know I'm annoying, sorry 😥 Hey, May you please explain where that's a BC break? If there was a BC break, the BC break was in PHP 7, where that guarantee was broken, as the RFC states in its introduction. Now the only guarantee is that the all abstract function signatures are the same ones. Which is not an useful guarantee by any means. The only thing this RFC changes is making abstract and normal classes consistent, which is a good thing. Less special cases. Bob --- ## #98656 — netmo.php@gmail.com — 2017-03-28T16:09:51+00:00 URL: https://externals.io/message/98646#98656 · In reply to: #98649 Hi Derick, sorry, previous message was meant to reply to you. There is no BC break, not even subtle. Ocramius' assumption is wrong. The most authoritative one is the innermost and closest ancestor definition. It is like that for classes, it is like that for interfaces, should be like that for abstract methods as well. And it's just an assumption anyway. Substitutability rules define this and it is very straightforward to me: are the overridden signatures type-compatible with the super definition? Yes. Then it should be allowed; we should not care if it's redundant, we should not care if it has few uses, just keep it consistent, IMO. Use cases are the same of a type change in a sub-interface, which, again, PHP does allow, even if to/from no-type only for now. Regards. --- ## #98657 — Fleshgrinder — 2017-03-28T16:31:45+00:00 URL: https://externals.io/message/98646#98657 · In reply to: #98646 > 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 --- ## #98658 — Marco Pivetta — 2017-03-28T16:41:13+00:00 URL: https://externals.io/message/98646#98658 · In reply to: #98657 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: 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/ --- ## #98659 — netmo.php@gmail.com — 2017-03-28T16:53:21+00:00 URL: https://externals.io/message/98646#98659 · In reply to: #98658 No problem. I instead thank you for checking this, because I forgot to check it myself (sorry). --- ## #98756 — Patrick ALLAERT — 2017-04-10T09:34:23+00:00 URL: https://externals.io/message/98646#98756 · In reply to: #98646 Le mar. 28 mars 2017 à 06:51, Wes 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): : > Le mar. 28 mars 2017 à 06:51, Wes a écrit : > > > […] > it > > […] > on > > […] > > 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): > > 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 --- ## #98764 — netmo.php@gmail.com — 2017-04-11T04:32:43+00:00 URL: https://externals.io/message/98646#98764 · In reply to: #98757 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! --- ## #98765 — Patrick ALLAERT — 2017-04-11T06:30:01+00:00 URL: https://externals.io/message/98646#98765 · In reply to: #98764 Le mar. 11 avr. 2017 à 06:32, Wes 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