Skip to content

Commit 9286101

Browse files
GirgiasGirgias
committed
Fix GH-9516: (A&B)|D as a param should allow AB or D. Not just A.
The issue was that we didn't compute enough cache slots for DNF types. Nor progressed throught the CE's in the cache slot, meaning we were only checking if the value passed satisfied the first type of the nested intersection type.
1 parent 80315ed commit 9286101

File tree

3 files changed

+186
-7
lines changed

3 files changed

+186
-7
lines changed

‎Zend/tests/type_declarations/dnf_types/gh9516.phpt

+157
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
--TEST--
2+
GH-9516: (A&B)|D as a param should allow AB or D. Not just A.
3+
--FILE--
4+
<?php
5+
6+
interface A { }
7+
interface B { }
8+
interface D { }
9+
10+
class A_ implements A {}
11+
class B_ implements B {}
12+
class AB_ implements A, B {}
13+
class D_ implements D {}
14+
15+
class T {
16+
public function method1((A&B)|D $arg): void {}
17+
public function method2((B&A)|D $arg): void {}
18+
public function method3(D|(A&B) $arg): void {}
19+
public function method4(D|(B&A) $arg): void {}
20+
}
21+
22+
$t = new T;
23+
24+
try {
25+
$t->method1(new A_);
26+
echo 'Fail', \PHP_EOL;
27+
} catch (\Throwable $throwable) {
28+
echo $throwable->getMessage(), \PHP_EOL;
29+
}
30+
31+
try {
32+
$t->method1(new B_);
33+
echo 'Fail', \PHP_EOL;
34+
} catch (\Throwable $throwable) {
35+
echo $throwable->getMessage(), \PHP_EOL;
36+
}
37+
38+
try {
39+
$t->method1(new AB_);
40+
echo 'Pass', \PHP_EOL;
41+
} catch (\Throwable $throwable) {
42+
echo $throwable->getMessage(), \PHP_EOL;
43+
}
44+
45+
try {
46+
$t->method1(new D_);
47+
echo 'Pass', \PHP_EOL;
48+
} catch (\Throwable $throwable) {
49+
echo $throwable->getMessage(), \PHP_EOL;
50+
}
51+
52+
// Lets try in reverse?
53+
try {
54+
$t->method2(new A_);
55+
echo 'Fail', \PHP_EOL;
56+
} catch (\Throwable $throwable) {
57+
echo $throwable->getMessage(), \PHP_EOL;
58+
}
59+
60+
try {
61+
$t->method2(new B_);
62+
echo 'Fail', \PHP_EOL;
63+
} catch (\Throwable $throwable) {
64+
echo $throwable->getMessage(), \PHP_EOL;
65+
}
66+
67+
try {
68+
$t->method2(new AB_);
69+
echo 'Pass', \PHP_EOL;
70+
} catch (\Throwable $throwable) {
71+
echo $throwable->getMessage(), \PHP_EOL;
72+
}
73+
74+
try {
75+
$t->method2(new D_);
76+
echo 'Pass', \PHP_EOL;
77+
} catch (\Throwable $throwable) {
78+
echo $throwable->getMessage(), \PHP_EOL;
79+
}
80+
81+
/* Single before intersection */
82+
try {
83+
$t->method3(new A_);
84+
echo 'Fail', \PHP_EOL;
85+
} catch (\Throwable $throwable) {
86+
echo $throwable->getMessage(), \PHP_EOL;
87+
}
88+
89+
try {
90+
$t->method3(new B_);
91+
echo 'Fail', \PHP_EOL;
92+
} catch (\Throwable $throwable) {
93+
echo $throwable->getMessage(), \PHP_EOL;
94+
}
95+
96+
try {
97+
$t->method3(new AB_);
98+
echo 'Pass', \PHP_EOL;
99+
} catch (\Throwable $throwable) {
100+
echo $throwable->getMessage(), \PHP_EOL;
101+
}
102+
103+
try {
104+
$t->method3(new D_);
105+
echo 'Pass', \PHP_EOL;
106+
} catch (\Throwable $throwable) {
107+
echo $throwable->getMessage(), \PHP_EOL;
108+
}
109+
110+
// Lets try in reverse?
111+
try {
112+
$t->method4(new A_);
113+
echo 'Fail', \PHP_EOL;
114+
} catch (\Throwable $throwable) {
115+
echo $throwable->getMessage(), \PHP_EOL;
116+
}
117+
118+
try {
119+
$t->method4(new B_);
120+
echo 'Fail', \PHP_EOL;
121+
} catch (\Throwable $throwable) {
122+
echo $throwable->getMessage(), \PHP_EOL;
123+
}
124+
125+
try {
126+
$t->method4(new AB_);
127+
echo 'Pass', \PHP_EOL;
128+
} catch (\Throwable $throwable) {
129+
echo $throwable->getMessage(), \PHP_EOL;
130+
}
131+
132+
try {
133+
$t->method4(new D_);
134+
echo 'Pass', \PHP_EOL;
135+
} catch (\Throwable $throwable) {
136+
echo $throwable->getMessage(), \PHP_EOL;
137+
}
138+
139+
140+
?>
141+
--EXPECTF--
142+
T::method1(): Argument #1 ($arg) must be of type (A&B)|D, A_ given, called in %s on line %d
143+
T::method1(): Argument #1 ($arg) must be of type (A&B)|D, B_ given, called in %s on line %d
144+
Pass
145+
Pass
146+
T::method2(): Argument #1 ($arg) must be of type (B&A)|D, A_ given, called in %s on line %d
147+
T::method2(): Argument #1 ($arg) must be of type (B&A)|D, B_ given, called in %s on line %d
148+
Pass
149+
Pass
150+
T::method3(): Argument #1 ($arg) must be of type D|(A&B), A_ given, called in %s on line %d
151+
T::method3(): Argument #1 ($arg) must be of type D|(A&B), B_ given, called in %s on line %d
152+
Pass
153+
Pass
154+
T::method4(): Argument #1 ($arg) must be of type D|(B&A), A_ given, called in %s on line %d
155+
T::method4(): Argument #1 ($arg) must be of type D|(B&A), B_ given, called in %s on line %d
156+
Pass
157+
Pass

‎Zend/zend_compile.c

+17-1
Original file line numberDiff line numberDiff line change
@@ -2380,7 +2380,23 @@ static size_t zend_type_get_num_classes(zend_type type) {
23802380
return 0;
23812381
}
23822382
if (ZEND_TYPE_HAS_LIST(type)) {
2383-
return ZEND_TYPE_LIST(type)->num_types;
2383+
/* Intersection types cannot have nested list types */
2384+
if (ZEND_TYPE_IS_INTERSECTION(type)) {
2385+
return ZEND_TYPE_LIST(type)->num_types;
2386+
}
2387+
ZEND_ASSERT(ZEND_TYPE_IS_UNION(type));
2388+
size_t count = 0;
2389+
zend_type *list_type;
2390+
2391+
ZEND_TYPE_LIST_FOREACH(ZEND_TYPE_LIST(type), list_type) {
2392+
if (ZEND_TYPE_IS_INTERSECTION(*list_type)) {
2393+
count += ZEND_TYPE_LIST(*list_type)->num_types;
2394+
} else {
2395+
ZEND_ASSERT(!ZEND_TYPE_HAS_LIST(*list_type));
2396+
count += 1;
2397+
}
2398+
} ZEND_TYPE_LIST_FOREACH_END();
2399+
return count;
23842400
}
23852401
return 1;
23862402
}

‎Zend/zend_execute.c

+12-6
Original file line numberDiff line numberDiff line change
@@ -1035,19 +1035,25 @@ static zend_always_inline zend_class_entry *zend_fetch_ce_from_cache_slot(
10351035
}
10361036

10371037
static bool zend_check_intersection_type_from_cache_slot(zend_type_list *intersection_type_list,
1038-
zend_class_entry *arg_ce, void **cache_slot)
1038+
zend_class_entry *arg_ce, void ***cache_slot_ptr)
10391039
{
1040+
void **cache_slot = *cache_slot_ptr;
10401041
zend_class_entry *ce;
10411042
zend_type *list_type;
1043+
bool status = true;
10421044
ZEND_TYPE_LIST_FOREACH(intersection_type_list, list_type) {
10431045
ce = zend_fetch_ce_from_cache_slot(cache_slot, list_type);
10441046
/* If type is not an instance of one of the types taking part in the
10451047
* intersection it cannot be a valid instance of the whole intersection type. */
10461048
if (!ce || !instanceof_function(arg_ce, ce)) {
1047-
return false;
1049+
status = false;
10481050
}
1051+
PROGRESS_CACHE_SLOT();
10491052
} ZEND_TYPE_LIST_FOREACH_END();
1050-
return true;
1053+
if (HAVE_CACHE_SLOT) {
1054+
*cache_slot_ptr = cache_slot;
1055+
}
1056+
return status;
10511057
}
10521058

10531059
static zend_always_inline bool zend_check_type_slow(
@@ -1073,19 +1079,19 @@ static zend_always_inline bool zend_check_type_slow(
10731079
} else {
10741080
ZEND_TYPE_LIST_FOREACH(ZEND_TYPE_LIST(*type), list_type) {
10751081
if (ZEND_TYPE_IS_INTERSECTION(*list_type)) {
1076-
if (zend_check_intersection_type_from_cache_slot(ZEND_TYPE_LIST(*list_type), Z_OBJCE_P(arg), cache_slot)) {
1082+
if (zend_check_intersection_type_from_cache_slot(ZEND_TYPE_LIST(*list_type), Z_OBJCE_P(arg), &cache_slot)) {
10771083
return true;
10781084
}
1085+
/* The cache_slot is progressed in zend_check_intersection_type_from_cache_slot() */
10791086
} else {
10801087
ZEND_ASSERT(!ZEND_TYPE_HAS_LIST(*list_type));
10811088
ce = zend_fetch_ce_from_cache_slot(cache_slot, list_type);
10821089
/* Instance of a single type part of a union is sufficient to pass the type check */
10831090
if (ce && instanceof_function(Z_OBJCE_P(arg), ce)) {
10841091
return true;
10851092
}
1093+
PROGRESS_CACHE_SLOT();
10861094
}
1087-
1088-
PROGRESS_CACHE_SLOT();
10891095
} ZEND_TYPE_LIST_FOREACH_END();
10901096
}
10911097
} else {

0 commit comments

Comments
 (0)