Re: [PHP-DEV] Method compatibility checks and order in which files are included

2021-07-22 Thread Nikita Popov
On Thu, Jul 22, 2021 at 10:32 AM Sebastian Bergmann 
wrote:

> We noticed that the PHARs of PHPUnit stopped working with PHP 8.1 [1].
> Before [2], a commit pushed today, PHPUnit loaded all sourcecode files
> packaged in the PHAR on startup (to work around problems that (hopefully)
> no longer exist because PHPUnit's PHAR is scoped using PHP-Scoper
> nowadays). This loading of all sourcecode files lead to the error reported
> in [1]. The "static include list" used for this is generated by PHPAB [3]
> which performs a topological sort based "class implements interface",
> "class uses trait", "class extends class", etc. dependencies. This
> topological sort does not, however, consider type declarations for
> properties as well parameters and return values as dependencies.
>
> What follows is a minimal, reproducing example:
>
> .
> ├── autoload.php
> ├── Collection.php
> └── CollectionIterator.php
>
> 0 directories, 3 files
>
> This is the contents of autoload.php:
>
>  require __DIR__ . '/Collection.php';
> require __DIR__ . '/CollectionIterator.php';
>
> This is the contents of Collection.php:
>
>  class Collection implements IteratorAggregate
> {
>  public function getIterator(): CollectionIterator
>  {
>  return new CollectionIterator($this);
>  }
> }
>
> And this is the contents of CollectionIterator.php:
>
>  class CollectionIterator implements Iterator
> {
>  public function __construct(Collection $collection)
>  {
>  }
>
>  public function current(): mixed
>  {
>  }
>
>  public function next(): void
>  {
>  }
>
>  public function key(): int
>  {
>  }
>
>  public function valid(): bool
>  {
>  }
>
>  public function rewind(): void
>  {
>  }
> }
>
> When I run autoload.php then I get
>
> Fatal error: Could not check compatibility between
> Collection::getIterator(): CollectionIterator
> and IteratorAggregate::getIterator(): Traversable, because class
> CollectionIterator is not
> available in /home/sb/example/Collection.php on line 4
>
> I am using PHP 8.1-dev (590af4678bbdbb9422bfcc568a0b8d2d1a6f74f5). The
> error does not occur with PHP 8.0.8.
>
> I assume the following happens:
>
> * The compiler works on Collection.php
> * The compiler sees that Collection implements IteratorAggregate
> * The compiler sees that Collection::getIterator() returns
> CollectionIterator
> * The compiler performs the method compatiblity check
> * The method compatiblity check cannot be performed because
> CollectionIterator has not been compiled yet
>
> When I change autoload.php like so
>
>  require __DIR__ . '/CollectionIterator.php';
> require __DIR__ . '/Collection.php';
>
> then the method compatiblity check can be performed (and succeeds without
> error).
>
> After this long introduction (sorry!), I can finally ask my question: can
> there be situations where this alternate order of requiring the source
> files would also fail? Is there a potential for cyclic dependencies
> between two sourcefiles that can only be resolved when autoloading is
> used? I am "scared" that using "static include list" may become unfeasible
> when more compile-time checks such as the method compatibility check are
> added.
>

Hey Sebastian,

This behavior isn't new in PHP 8.1, it exists since the introduction of
variance support in PHP 7.4. Presumably you just didn't hit a case where a)
a non-trivial subtying relationship had to be established and b) the files
did not happen to be included in the right order. The only thing that PHP
8.1 adds are more types on internal classes, which means that there are
more type constraints to satisfy.

To answer your actual question, yes, it's possible to have cyclic
dependencies, and such cases are (and can only be) supported if an
autoloader is used. A simple example is this structure:

class A {
public function method(): B {}
}
class B extends A {
public function method(): C {}
}
class C extends B {}

There is no legal ordering of these classes that does not involve
autoloading during inheritance.

Regards,
Nikita


[PHP-DEV] Method compatibility checks and order in which files are included

2021-07-22 Thread Sebastian Bergmann
We noticed that the PHARs of PHPUnit stopped working with PHP 8.1 [1]. 
Before [2], a commit pushed today, PHPUnit loaded all sourcecode files 
packaged in the PHAR on startup (to work around problems that (hopefully) 
no longer exist because PHPUnit's PHAR is scoped using PHP-Scoper 
nowadays). This loading of all sourcecode files lead to the error reported 
in [1]. The "static include list" used for this is generated by PHPAB [3] 
which performs a topological sort based "class implements interface", 
"class uses trait", "class extends class", etc. dependencies. This 
topological sort does not, however, consider type declarations for 
properties as well parameters and return values as dependencies.


What follows is a minimal, reproducing example:

.
├── autoload.php
├── Collection.php
└── CollectionIterator.php

0 directories, 3 files

This is the contents of autoload.php:

Fatal error: Could not check compatibility between 
Collection::getIterator(): CollectionIterator
and IteratorAggregate::getIterator(): Traversable, because class 
CollectionIterator is not

available in /home/sb/example/Collection.php on line 4

I am using PHP 8.1-dev (590af4678bbdbb9422bfcc568a0b8d2d1a6f74f5). The 
error does not occur with PHP 8.0.8.


I assume the following happens:

* The compiler works on Collection.php
* The compiler sees that Collection implements IteratorAggregate
* The compiler sees that Collection::getIterator() returns CollectionIterator
* The compiler performs the method compatiblity check
* The method compatiblity check cannot be performed because 
CollectionIterator has not been compiled yet


When I change autoload.php like so

then the method compatiblity check can be performed (and succeeds without 
error).


After this long introduction (sorry!), I can finally ask my question: can 
there be situations where this alternate order of requiring the source 
files would also fail? Is there a potential for cyclic dependencies 
between two sourcefiles that can only be resolved when autoloading is 
used? I am "scared" that using "static include list" may become unfeasible 
when more compile-time checks such as the method compatibility check are 
added.


--
[1] 
https://github.com/sebastianbergmann/phpunit/issues/4740#issuecomment-884667189
[2] 
https://github.com/sebastianbergmann/phpunit/commit/4b5756d8b093d5ae74801586e621703508ec4be1

[3] https://github.com/theseer/Autoload

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php