Skip to content

Commit 23f6e36

Browse files
nikicnikic
committed
Support full variance if autoloading is used
Keep track of delayed variance obligations and enforce them when class linking depth reaches zero.
1 parent 2b904c3 commit 23f6e36

14 files changed

+514
-55
lines changed

‎Zend/tests/type_declarations/variance/class_order_autoload.phpt renamed to ‎Zend/tests/type_declarations/variance/class_order_autoload1.phpt

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
--TEST--
2-
Returns are covariant, but we don't allow the code due to class ordering (autoload variation)
2+
Class order allowed with autoloading (1)
33
--FILE--
44
<?php
55

@@ -21,5 +21,6 @@ spl_autoload_register(function($class) {
2121
$c = new C;
2222

2323
?>
24-
--EXPECTF--
25-
Fatal error: Could not check compatibility between B::method(): C and A::method(): B, because class C is not available in %s on line %d
24+
===DONE===
25+
--EXPECT--
26+
===DONE===

‎Zend/tests/type_declarations/variance/class_order_autoload2.phpt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
Class order allowed with autoloading (2)
3+
--FILE--
4+
<?php
5+
6+
spl_autoload_register(function($class) {
7+
if ($class === 'A') {
8+
class A {
9+
public function method() : B {}
10+
}
11+
} else if ($class == 'B') {
12+
class B extends A {
13+
public function method() : C {}
14+
}
15+
} else {
16+
class C extends B {
17+
}
18+
}
19+
});
20+
21+
// Same as autoload1 test case, but with a different autoloading root
22+
$b = new B;
23+
24+
?>
25+
===DONE===
26+
--EXPECT--
27+
===DONE===

‎Zend/tests/type_declarations/variance/class_order_autoload3.phpt

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
--TEST--
2+
Class order allowed with autoloading (3)
3+
--FILE--
4+
<?php
5+
6+
spl_autoload_register(function($class) {
7+
if ($class == 'A') {
8+
class A {
9+
public function method(): X {}
10+
}
11+
} else if ($class == 'B') {
12+
class B extends A {
13+
public function method(): Y {}
14+
}
15+
} else if ($class == 'X') {
16+
class X {
17+
public function method(): A {}
18+
}
19+
} else if ($class == 'Y') {
20+
class Y extends X {
21+
public function method(): B {}
22+
}
23+
}
24+
});
25+
26+
$b = new B;
27+
28+
?>
29+
===DONE===
30+
--EXPECT--
31+
===DONE===

‎Zend/tests/type_declarations/variance/class_order_autoload4.phpt

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--TEST--
2+
Class order allowed with autoloading (4)
3+
--FILE--
4+
<?php
5+
6+
// Same as autoload3 test case, but with X, Y being interfaces.
7+
spl_autoload_register(function($class) {
8+
if ($class == 'A') {
9+
class A {
10+
public function method(): X {}
11+
}
12+
} else if ($class == 'B') {
13+
class B extends A {
14+
public function method(): Y {}
15+
}
16+
} else if ($class == 'X') {
17+
interface X {
18+
public function method(): A;
19+
}
20+
} else if ($class == 'Y') {
21+
interface Y extends X {
22+
public function method(): B;
23+
}
24+
}
25+
});
26+
27+
$b = new B;
28+
29+
?>
30+
===DONE===
31+
--EXPECT--
32+
===DONE===

‎Zend/tests/type_declarations/variance/class_order_autoload5.phpt

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
--TEST--
2+
Class order allowed with autoloading (5)
3+
--FILE--
4+
<?php
5+
6+
// Similar to variance3, but one more class hierarchy in the cycle
7+
spl_autoload_register(function($class) {
8+
if ($class == 'A') {
9+
class A {
10+
public function method(): X {}
11+
}
12+
} else if ($class == 'B') {
13+
class B extends A {
14+
public function method(): Y {}
15+
}
16+
} else if ($class == 'X') {
17+
class X {
18+
public function method(): Q {}
19+
}
20+
} else if ($class == 'Y') {
21+
class Y extends X {
22+
public function method(): R {}
23+
}
24+
} else if ($class == 'Q') {
25+
class Q {
26+
public function method(): A {}
27+
}
28+
} else if ($class == 'R') {
29+
class R extends Q {
30+
public function method(): B {}
31+
}
32+
}
33+
});
34+
35+
$b = new B;
36+
37+
?>
38+
===DONE===
39+
--EXPECT--
40+
===DONE===

‎Zend/tests/type_declarations/variance/class_order_autoload_error1.phpt

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
--TEST--
2+
Variance error in the presence of autoloading (1)
3+
--FILE--
4+
<?php
5+
6+
spl_autoload_register(function($class) {
7+
if ($class === 'A') {
8+
class A {
9+
public function method() : C {}
10+
}
11+
} else if ($class == 'B') {
12+
class B extends A {
13+
public function method() : B {}
14+
}
15+
} else {
16+
class C extends B {
17+
}
18+
}
19+
});
20+
21+
$b = new B;
22+
23+
?>
24+
--EXPECTF--
25+
Fatal error: Declaration of B::method(): B must be compatible with A::method(): C in %s on line %d

‎Zend/tests/type_declarations/variance/class_order_autoload_error2.phpt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
Variance error in the presence of autoloading (2)
3+
--FILE--
4+
<?php
5+
6+
// Same as autoload_error1, but for argument types.
7+
spl_autoload_register(function($class) {
8+
if ($class === 'A') {
9+
class A {
10+
public function method(B $x) {}
11+
}
12+
} else if ($class == 'B') {
13+
class B extends A {
14+
public function method(C $x) {}
15+
}
16+
} else {
17+
class C extends B {
18+
}
19+
}
20+
});
21+
22+
$b = new B;
23+
$c = new C;
24+
25+
?>
26+
--EXPECTF--
27+
Warning: Declaration of B::method(C $x) should be compatible with A::method(B $x) in %s on line %d

‎Zend/tests/type_declarations/variance/class_order_autoload_error3.phpt

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
Variance error in the presence of autoloading (3)
3+
--FILE--
4+
<?php
5+
6+
spl_autoload_register(function($class) {
7+
if ($class == 'A') {
8+
class A {
9+
public function method(): X {}
10+
}
11+
} else if ($class == 'B') {
12+
class B extends A {
13+
public function method(): Y {}
14+
}
15+
} else if ($class == 'X') {
16+
class X {
17+
public function method(): Q {}
18+
}
19+
} else if ($class == 'Y') {
20+
class Y extends X {
21+
public function method(): R {}
22+
}
23+
} else if ($class == 'Q') {
24+
class Q {
25+
public function method(): B {}
26+
}
27+
} else if ($class == 'R') {
28+
class R extends Q {
29+
public function method(): A {}
30+
}
31+
}
32+
});
33+
34+
$b = new B;
35+
36+
?>
37+
--EXPECTF--
38+
Fatal error: Declaration of R::method(): A must be compatible with Q::method(): B in %s on line %d

‎Zend/tests/type_declarations/variance/class_order.phpt renamed to ‎Zend/tests/type_declarations/variance/class_order_error.phpt

File renamed without changes.

‎Zend/zend_compile.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,9 @@ void init_compiler(void) /* {{{ */
370370
zend_hash_init(&CG(filenames_table), 8, NULL, ZVAL_PTR_DTOR, 0);
371371
zend_llist_init(&CG(open_files), sizeof(zend_file_handle), (void (*)(void *)) file_handle_dtor, 0);
372372
CG(unclean_shutdown) = 0;
373+
374+
CG(link_depth) = 0;
375+
CG(delayed_variance_obligations) = NULL;
373376
}
374377
/* }}} */
375378

@@ -379,6 +382,12 @@ void shutdown_compiler(void) /* {{{ */
379382
zend_stack_destroy(&CG(delayed_oplines_stack));
380383
zend_hash_destroy(&CG(filenames_table));
381384
zend_arena_destroy(CG(arena));
385+
386+
if (CG(delayed_variance_obligations)) {
387+
zend_hash_destroy(CG(delayed_variance_obligations));
388+
FREE_HASHTABLE(CG(delayed_variance_obligations));
389+
CG(delayed_variance_obligations) = NULL;
390+
}
382391
}
383392
/* }}} */
384393

‎Zend/zend_compile.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,9 @@ typedef struct _zend_oparray_context {
272272
/* Class is being linked. Don't free strings. | | | */
273273
#define ZEND_ACC_LINKING_IN_PROGRESS (1 << 19) /* X | | | */
274274
/* | | | */
275+
/* Class has unresolved variance obligations. | | | */
276+
#define ZEND_ACC_UNRESOLVED_VARIANCE (1 << 20) /* X | | | */
277+
/* | | | */
275278
/* Function Flags (unused: 28...30) | | | */
276279
/* ============== | | | */
277280
/* | | | */
@@ -843,6 +846,7 @@ void zend_assert_valid_class_name(const zend_string *const_name);
843846
#define ZEND_FETCH_CLASS_NO_AUTOLOAD 0x80
844847
#define ZEND_FETCH_CLASS_SILENT 0x0100
845848
#define ZEND_FETCH_CLASS_EXCEPTION 0x0200
849+
#define ZEND_FETCH_CLASS_ALLOW_UNRESOLVED_VARIANCE 0x0400
846850

847851
#define ZEND_PARAM_REF (1<<0)
848852
#define ZEND_PARAM_VARIADIC (1<<1)

‎Zend/zend_execute_API.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -917,6 +917,10 @@ ZEND_API zend_class_entry *zend_lookup_class_ex(zend_string *name, zend_string *
917917
}
918918
ce = (zend_class_entry*)Z_PTR_P(zv);
919919
if (UNEXPECTED(!(ce->ce_flags & ZEND_ACC_LINKED))) {
920+
if ((flags & ZEND_FETCH_CLASS_ALLOW_UNRESOLVED_VARIANCE)
921+
&& (ce->ce_flags & ZEND_ACC_UNRESOLVED_VARIANCE)) {
922+
return ce;
923+
}
920924
return NULL;
921925
}
922926
return ce;

‎Zend/zend_globals.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,9 @@ struct _zend_compiler_globals {
124124
void *map_ptr_base;
125125
size_t map_ptr_size;
126126
size_t map_ptr_last;
127+
128+
uint32_t link_depth;
129+
HashTable *delayed_variance_obligations;
127130
};
128131

129132

0 commit comments

Comments
 (0)