# Inconsistent class behavior and undocumented(?) BC change

- Thread: https://externals.io/message/107902
- Original archive: https://news-web.php.net/php.internals/107902
- Started by Larry Garfield on 2019-12-08T00:27:06+00:00
- Messages: 6
- 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

---

## #107902 — Larry Garfield — 2019-12-08T00:27:06+00:00

URL: https://externals.io/message/107902#107902

I am not sure if this is a bug, a feature behaving in a desired but confusing way, or a feature behaving in a confusing and thus undesireable way.  I am therefore reporting it here in order to defer to those who know the answer to such questions better.

Consider the following:

class Ancestor {
    public function __construct(int $a, string $b) { }
}

class Child extends Ancestor {
    public function __construct(...$args) {
        parent::__construct(...$args);
    }
}

This works with no compile errors.  For any other method however:

class Ancestor {
    public function doStuff(int $a, string $b) { }
}

class Child extends Ancestor {
    public function doStuff(...$args) {
        parent::doStuff(...$args);
    }
}

I get:

Warning: Declaration of Child::doStuff(...$args) should be compatible with Ancestor::doStuff(int $a, string $b)

I am not clear on why __construct() is special in this case; I know __construct() is special where interfaces are concerned, but I didn't realize it was special with regards to basic inheritance.  It seems to happen regardless of the type information presented (or not).

Is this intentional?  Is there a logical way it could be made to work?  Are constructors actually wrong here?  (I hope not, because it's a neat trick.)

Additionally, according to 3v4l.org at least, the error message has changed.  On 7.1-7.3, the exact error message is:

Warning: Declaration of Child::doStuff(int ...$args) should be compatible with Ancestor::doStuff($a, $b) in /in/6NthP on line 15

On 7.4, it reports on a different line:

Warning: Declaration of Child::doStuff(int ...$args) should be compatible with Ancestor::doStuff($a, $b) in /in/6NthP on line 11

Specifically, prior to 7.4, it reports on the LAST line of the class (the closing brace).  As of 7.4 it reports on the line of the method that is inconsistent.

I don't think this is a bad change, per se.  It's actually a good change for debugging, IMO.  But I don't see it listed in the migration guide.  Should it be, in case some failure-mode tests or other automated error systems care?

https://www.php.net/manual/en/migration74.incompatible.php

-- 
  Larry Garfield
  larry@garfieldtech.com

---

## #107903 — Mike Schinkel — 2019-12-08T05:03:00+00:00

URL: https://externals.io/message/107902#107903 · In reply to: #107902

Hi Larry,

> I am not clear on why __construct() is special in this case;

I believe that is the Liskok substitution principle at work, and that fact the principle does not apply to constructors.

For reference:

- https://softwareengineering.stackexchange.com/a/302477/9114
- https://www.sitepoint.com/constructors-and-the-myth-of-breaking-the-lsp/

-Mike

> 
> 
> I am not sure if this is a bug, a feature behaving in a desired but confusing way, or a feature behaving in a confusing and thus undesireable way.  I am therefore reporting it here in order to defer to those who know the answer to such questions better.
> 
> Consider the following:
> 
> class Ancestor {
>    public function __construct(int $a, string $b) { }
> }
> 
> class Child extends Ancestor {
>    public function __construct(...$args) {
>        parent::__construct(...$args);
>    }
> }
> 
> This works with no compile errors.  For any other method however:
> 
> class Ancestor {
>    public function doStuff(int $a, string $b) { }
> }
> 
> class Child extends Ancestor {
>    public function doStuff(...$args) {
>        parent::doStuff(...$args);
>    }
> }
> 
> I get:
> 
> Warning: Declaration of Child::doStuff(...$args) should be compatible with Ancestor::doStuff(int $a, string $b)
> 
> I am not clear on why __construct() is special in this case; I know __construct() is special where interfaces are concerned, but I didn't realize it was special with regards to basic inheritance.  It seems to happen regardless of the type information presented (or not).
> 
> Is this intentional?  Is there a logical way it could be made to work?  Are constructors actually wrong here?  (I hope not, because it's a neat trick.)
> 
> Additionally, according to 3v4l.org at least, the error message has changed.  On 7.1-7.3, the exact error message is:
> 
> Warning: Declaration of Child::doStuff(int ...$args) should be compatible with Ancestor::doStuff($a, $b) in /in/6NthP on line 15
> 
> On 7.4, it reports on a different line:
> 
> Warning: Declaration of Child::doStuff(int ...$args) should be compatible with Ancestor::doStuff($a, $b) in /in/6NthP on line 11
> 
> Specifically, prior to 7.4, it reports on the LAST line of the class (the closing brace).  As of 7.4 it reports on the line of the method that is inconsistent.
> 
> I don't think this is a bad change, per se.  It's actually a good change for debugging, IMO.  But I don't see it listed in the migration guide.  Should it be, in case some failure-mode tests or other automated error systems care?
> 
> https://www.php.net/manual/en/migration74.incompatible.php
> 
> -- 
>  Larry Garfield
>  larry@garfieldtech.com

---

## #107905 — Larry Garfield — 2019-12-08T17:43:10+00:00

URL: https://externals.io/message/107902#107905

> 
> > […]
> 
> 
> I'm not sure either of those links do a good job explaining why 
> constructors in particular are special.
> 
> My first thought was "you call them on the class not an instance", but 
> that's also true of static methods, and yet this is an error:
> 
> class Ancestor {
>      public static function constructMe(int $a, string $b) { }
> }
> class Child extends Ancestor {
>      public static function constructMe(...$args) {
>          return parent::constructMe(...$args);
>      }
> }
> 
> In PHP, it is actually possible to call a static method based on an 
> instance, using the syntax "$foo::somethingStatic()", so it is 
> theoretically logical to constrain a set of related classes to have 
> compatible static methods. But you can also write "new $foo()" to invoke 
> a constructor based on an instance, so that doesn't explain the 
> difference either.
> 
> The best explanation I can think of is more pragmatic than theoretical: 
> it's often useful to have related objects with unrelated constructors, 
> so it's useful for the language not to restrict that. I'd be interested 
> to hear if there's a more fundamental reason, though.

I recall constructors being special with interfaces; I didn't consciously realize that *all* inheritance rules were skipped for them.  I suppose in that case it's logical that it is why it is, just disappointing; the double-splat trick I was trying above is really nifty, but I guess it only works on constructors.  Sad elePHPant...

--Larry Garfield

---

## #108006 — Nikita Popov — 2020-01-06T11:16:56+00:00

URL: https://externals.io/message/107902#108006 · In reply to: #107902

On Sun, Dec 8, 2019 at 1:29 AM Larry Garfield <larry@garfieldtech.com>
wrote:

> I am not sure if this is a bug, a feature behaving in a desired but
> confusing way, or a feature behaving in a confusing and thus undesireable
> way.  I am therefore reporting it here in order to defer to those who know
> the answer to such questions better.
>
> Consider the following:
>
> class Ancestor {
>     public function __construct(int $a, string $b) { }
> }
>
> class Child extends Ancestor {
>     public function __construct(...$args) {
>         parent::__construct(...$args);
>     }
> }
>
> This works with no compile errors.  For any other method however:
>
> class Ancestor {
>     public function doStuff(int $a, string $b) { }
> }
>
> class Child extends Ancestor {
>     public function doStuff(...$args) {
>         parent::doStuff(...$args);
>     }
> }
>
> I get:
>
> Warning: Declaration of Child::doStuff(...$args) should be compatible with
> Ancestor::doStuff(int $a, string $b)
>
> I am not clear on why __construct() is special in this case; I know
> __construct() is special where interfaces are concerned, but I didn't
> realize it was special with regards to basic inheritance.  It seems to
> happen regardless of the type information presented (or not).
>
> Is this intentional?  Is there a logical way it could be made to work?
> Are constructors actually wrong here?  (I hope not, because it's a neat
> trick.)
>

Others have already explained why constructors are exempted from LSP
checks, so let me reply to your other point...

I believe that your example should indeed be legal in general and created a
PR to fix this: https://github.com/php/php-src/pull/5059

With this change, the variadic argument can replace any number of
non-variadic arguments, as long as it is compatible with them, where
compatibility is, as usual, determined based on type contravariance, as
well as reference passing invariance. Effectively this means that
function(...$args) is compatible with all signatures that do not involve
reference passing.

Does anyone see any soundness issues with this change?

Nikita

---

## #108009 — Larry Garfield — 2020-01-06T18:17:44+00:00

URL: https://externals.io/message/107902#108009 · In reply to: #108006

> Others have already explained why constructors are exempted from LSP
> checks, so let me reply to your other point...
> 
> I believe that your example should indeed be legal in general and created a
> PR to fix this: https://github.com/php/php-src/pull/5059
> 
> With this change, the variadic argument can replace any number of
> non-variadic arguments, as long as it is compatible with them, where
> compatibility is, as usual, determined based on type contravariance, as
> well as reference passing invariance. Effectively this means that
> function(...$args) is compatible with all signatures that do not involve
> reference passing.
> 
> Does anyone see any soundness issues with this change?
> 
> Nikita

Nifty!  Since `func(...$args)` is already a universal caller, that gives us a symmetrical way to defer calls, either to a parent or to a delegated method on a wrapped object.  So that means one can implement a pass-through class where most of the methods are:

class Wrapper implements Thing {
  public function thing(...$args) { $this->wrapped->thing(...$args); }
}

The symmetry appeals to me, even if it means types get caught only at the lower level.

One possible question; would this mean bypassing type checks if not delegating?  Viz:

interface Foo {
  public function bar(Foo, $f, Baz $b, Beep $e);
}

class Impl implements Foo {

  public function bar(...$args) { ... }

}

$i = new Impl();

// Will this still report a type error, and if so, where/how?
// If not, does that become a back-door way to dodge an interface?  Is that good?
$i->bar('a', 'b', 'c');

--Larry Garfield

---

## #108010 — Nikita Popov — 2020-01-06T18:40:01+00:00

URL: https://externals.io/message/107902#108010 · In reply to: #108009

On Mon, Jan 6, 2020 at 7:18 PM Larry Garfield <larry@garfieldtech.com>
wrote:

> 
>
> > […]
> created a
> > […]
>
> Nifty!  Since `func(...$args)` is already a universal caller, that gives
> us a symmetrical way to defer calls, either to a parent or to a delegated
> method on a wrapped object.  So that means one can implement a pass-through
> class where most of the methods are:
>
> class Wrapper implements Thing {
>   public function thing(...$args) { $this->wrapped->thing(...$args); }
> }
>
> The symmetry appeals to me, even if it means types get caught only at the
> lower level.
>
> One possible question; would this mean bypassing type checks if not
> delegating?  Viz:
>
> interface Foo {
>   public function bar(Foo, $f, Baz $b, Beep $e);
> }
>
> class Impl implements Foo {
>
>   public function bar(...$args) { ... }
>
> }
>
> $i = new Impl();
>
> // Will this still report a type error, and if so, where/how?
> // If not, does that become a back-door way to dodge an interface?  Is
> that good?
> $i->bar('a', 'b', 'c');
>

Yes, you can "bypass" type checks that way. However, this is not related to
variadics, but general contravariance. In particular, you can also simply
drop the types while keeping the explicit parameters:

class Impl implements Foo {

  public function bar($f, $b, $e) { ... }

}

As of PHP 7.2 (I think) dropping parameter types is always allowed.

Nikita