Skip to content

Covariant Returns and Contravariant Parameters #3712

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions Zend/tests/bug75573.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ Bug #75573 (Segmentation fault in 7.1.12 and 7.0.26)
class A
{
var $_stdObject;
function initialize($properties = FALSE) {
$this->_stdObject = $properties ? (object) $properties : new stdClass();
parent::initialize();
}
function &__get($property)
{
if (isset($this->_stdObject->{$property})) {
Expand All @@ -31,10 +27,6 @@ class A

class B extends A
{
function initialize($properties = array())
{
parent::initialize($properties);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose, you removed this, because of some compatibility break, but you didn't reflect this in RFC.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See this comment on bugs.php.net:

Did the unused initialize methods have some bearing on this bug?
Since A does not have a parent it would fatal if it were ever called. I'm adding compile-time warnings to uses of parent when there isn't one and ran across this bug, but since it's "fixed" I can't tell if removing the unused methods has any bearing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just don't make unrelated "improvements" in PHPT tests, because they look suspicious, and require reviewers to verify them. Better, commit these chunks separately.

Copy link
Contributor Author

@morrisonlevi morrisonlevi Dec 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are not unrelated. The unused method A::initialize has erroneous parent:: usage, which this includes. I know you said you would have preferred that to be a different patch, but the code that deals with parent is needed for resolving variance correctly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to add compile-time warning. This would clearly explain the change in behavior, and its correctness.

function &__get($property)
{
if (isset($this->settings) && isset($this->settings[$property])) {
Expand Down
4 changes: 4 additions & 0 deletions Zend/tests/bug76451.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?php

class Foo {}
class_alias('Foo', 'Bar');
20 changes: 20 additions & 0 deletions Zend/tests/bug76451.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
--TEST--
Aliases during inheritance type checks affected by opcache
--INI--
opcache.enable=1
opcache.enable_cli=1
opcache.optimization_level=-1
--SKIPIF--
<?php if (!extension_loaded('Zend OPcache') || php_sapi_name() != "cli") die("skip CLI only"); ?>
Copy link
Contributor

@carusogabriel carusogabriel Dec 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Off topic: The second part of this condition, when does it happen? I mean, can we run the tests without been via the CLI?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@carusogabriel run-tests can actually execute with php-cgi aswell, see: https://github.com/php/php-src/blob/master/run-tests.php#L123 and below and below :)

--FILE--
<?php
require __DIR__ . "/bug76451.inc";

class A {
public function test(Foo $foo) {}
}
class B extends A {
public function test(Bar $foo) {}
}
?>
--EXPECT--
5 changes: 5 additions & 0 deletions Zend/tests/class_name_as_scalar.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ namespace Foo\Bar {
// compile time constants
const A = self::class;
const B = Two::class;

}
class Two extends One {
const A = parent::class;

public static function run() {
var_dump(self::class); // self compile time lookup
var_dump(static::class); // runtime lookup
Expand All @@ -18,6 +21,8 @@ namespace Foo\Bar {
}
}
class Three extends Two {
const B = parent::class;

// compile time static lookups
public static function checkCompileTime(
$one = self::class,
Expand Down
4 changes: 3 additions & 1 deletion Zend/tests/class_name_as_scalar_error_002.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,6 @@ namespace Foo\Bar {
}
?>
--EXPECTF--
Fatal error: parent::class cannot be used for compile-time class name resolution in %s on line %d
Deprecated: Cannot use "parent" without a parent class in %s on line %d

Fatal error: Cannot use "parent" without a parent class in %s on line %d
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I would separate parent::class into a different patch.

4 changes: 3 additions & 1 deletion Zend/tests/class_name_as_scalar_error_004.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,6 @@ namespace Foo\Bar {
}
?>
--EXPECTF--
Fatal error: parent::class cannot be used for compile-time class name resolution in %s on line %d
Deprecated: Cannot use "parent" without a parent class in %s on line %d

Fatal error: Cannot use "parent" without a parent class in %s on line %d
11 changes: 11 additions & 0 deletions Zend/tests/compile_time_parent_error_01.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
--TEST--
Using "parent" in class without parent results in compile-time error
--FILE--
<?php
class InvalidClass {
function method(parent $p) {}
}

?>
--EXPECTF--
Deprecated: Cannot use "parent" without a parent class in %s on line %d
11 changes: 11 additions & 0 deletions Zend/tests/compile_time_parent_error_02.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
--TEST--
Using "parent" in class without parent results in compile-time error
--FILE--
<?php
class InvalidClass {
function method(): parent {}
}

?>
--EXPECTF--
Deprecated: Cannot use "parent" without a parent class in %s on line %d
13 changes: 13 additions & 0 deletions Zend/tests/compile_time_parent_error_03.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
--TEST--
Using "parent" in class without parent results in compile-time error
--FILE--
<?php
class InvalidClass {
function method() {
echo parent::class;
}
}

?>
--EXPECTF--
Deprecated: Cannot use "parent" without a parent class in %s on line %d
13 changes: 13 additions & 0 deletions Zend/tests/compile_time_parent_error_04.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
--TEST--
Using "parent" in class without parent results in compile-time error
--FILE--
<?php
$obj = new class() {
function method() {
echo parent::class;
}
}

?>
--EXPECTF--
Deprecated: Cannot use "parent" without a parent class in %s on line %d
17 changes: 17 additions & 0 deletions Zend/tests/compile_time_parent_error_05.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
--TEST--
Using "parent" in class without parent results in compile-time error
--FILE--
<?php
class InvalidClass {
function method() {
$obj = new class() {
function method() {
echo parent::class;
}
};
}
}

?>
--EXPECTF--
Deprecated: Cannot use "parent" without a parent class in %s on line %d
22 changes: 22 additions & 0 deletions Zend/tests/object_types/object_variance.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
--TEST--
Testing object's variance in inheritance
--FILE--
<?php

interface I1 {
function method1(I1 $o): object;
}
interface I2 extends I1 {
function method1(object $o): I1;
}
final class C1 implements I2 {
function method1($o = null): self {
return $this;
}
}

$o = new C1();
echo get_class($o->method1());
?>
--EXPECT--
C1
7 changes: 4 additions & 3 deletions Zend/tests/return_types/008.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class qux implements foo {
}

$qux = new qux();
var_dump($qux->bar());
--EXPECTF--
Fatal error: Declaration of qux::bar(): qux must be compatible with foo::bar(): foo in %s008.php on line 7
echo get_class($qux->bar());

--EXPECT--
qux
7 changes: 4 additions & 3 deletions Zend/tests/return_types/generators003.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class SomeCollection implements Collection {
}

$some = new SomeCollection();
var_dump($some->getIterator());
--EXPECTF--
Fatal error: Declaration of SomeCollection::getIterator(): Generator must be compatible with Collection::getIterator(): Iterator in %sgenerators003.php on line 6
echo get_class($some->getIterator());

--EXPECT--
Generator
7 changes: 5 additions & 2 deletions Zend/tests/return_types/inheritance005.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,8 @@ class Bar extends Foo {
return new Bar;
}
}
--EXPECTF--
Fatal error: Declaration of Bar::test(): Bar must be compatible with Foo::test(): Foo in %sinheritance005.php on line 12

echo get_class(Bar::test());

--EXPECT--
Bar
7 changes: 5 additions & 2 deletions Zend/tests/return_types/inheritance006.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,8 @@ class Bar extends Foo {
return new B;
}
}
--EXPECTF--
Fatal error: Declaration of Bar::test(): B must be compatible with Foo::test(): A in %sinheritance006.php on line 14

echo get_class(Bar::test());

--EXPECT--
B
7 changes: 5 additions & 2 deletions Zend/tests/return_types/inheritance007.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,8 @@ class Bar extends Foo {
return new ArrayObject([1, 2]);
}
}
--EXPECTF--
Fatal error: Declaration of Bar::test(): ArrayObject must be compatible with Foo::test(): Traversable in %sinheritance007.php on line 12

echo get_class(Bar::test());

--EXPECT--
ArrayObject
26 changes: 26 additions & 0 deletions Zend/tests/traits/parent_001.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
--TEST--
Using parent does not create compile-time warnings
--FILE--
<?php

trait P {
public function m(parent $arg): parent {
return $arg;
}
}

class A {}

class B extends A {
use P;
}

$b = new B();
$a = $b->m(new A());

print "OK\n";

?>
--EXPECT--
OK

2 changes: 2 additions & 0 deletions Zend/zend.c
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,8 @@ static void compiler_globals_ctor(zend_compiler_globals *compiler_globals) /* {{
zend_hash_init_ex(compiler_globals->class_table, 64, NULL, ZEND_CLASS_DTOR, 1, 0);
zend_hash_copy(compiler_globals->class_table, global_class_table, zend_class_add_ref);

compiler_globals->unverified_types = NULL;

zend_set_default_compile_time_values();

compiler_globals->auto_globals = (HashTable *) malloc(sizeof(HashTable));
Expand Down
Loading