Skip to content

Commit 6b6dac8

Browse files
committed
fix inheritance chaining
Signed-off-by: Robert Landers <landers.robert@gmail.com>
1 parent c7c692d commit 6b6dac8

3 files changed

Lines changed: 158 additions & 33 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
--TEST--
2+
Parametric LSP: child implementing `I<TN>` with its own TN forwarded to parent's TN
3+
--FILE--
4+
<?php
5+
// Regression: when both parent and child bind the same parameter shape
6+
// (parent's TN, child's TN, both bare type-parameter refs), the
7+
// inheritance check needs to treat them as equivalent. The parent's
8+
// substituted return is itself a type-parameter ref so the substitute
9+
// helper falls back to the erased form; the child must follow the same
10+
// fall-back, or the check sees `mixed` vs `TN` and rejects what's
11+
// structurally an identity binding.
12+
13+
interface GI<TN = mixed, TW = mixed> {
14+
public function getEdgesFrom(TN $from): array;
15+
}
16+
17+
class DG<TN = mixed, TW = mixed> implements GI<TN, TW> {
18+
public function getEdgesFrom(TN $from): array { return []; }
19+
}
20+
21+
echo "OK\n";
22+
?>
23+
--EXPECT--
24+
OK
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
--TEST--
2+
Parametric LSP: method-level type-parameter bound that references a class-scope T substitutes when the subclass binds T
3+
--FILE--
4+
<?php
5+
// Regression: `class A<T> { function set<U : T>(U $x) }` — the method's
6+
// own U has a bound that's a class-scope T-ref. When `class B extends A<string>`
7+
// supplies T = string, the inheritance check on the inherited (or overridden)
8+
// method must see U's bound substituted from T → string, otherwise the parent
9+
// renders as `set(mixed $x)` (T erased) and the child's `set<U:string>(U $x)`
10+
// gets rejected as narrower than mixed.
11+
12+
class A<T> {
13+
public function set<U : T>(U $x): void {}
14+
}
15+
16+
class B extends A<string> {
17+
public function set<U : string>(U $x): void {}
18+
}
19+
20+
class C<Tl, Tr> {
21+
public function set<U : Tl|Tr>(U $x): void {}
22+
}
23+
24+
class D extends C<string, int> {
25+
public function set<U : string|int>(U $x): void {}
26+
}
27+
28+
echo "OK\n";
29+
?>
30+
--EXPECT--
31+
OK

Zend/zend_inheritance.c

Lines changed: 103 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1256,10 +1256,12 @@ static bool zend_get_target_default_args(
12561256
return true;
12571257
}
12581258

1259-
/* If ce supplies type arguments to proto's declaring scope (directly,
1260-
* transitively, or via parameter defaults), returns proto's pre-erasure type
1261-
* with class-scope T-refs substituted. */
1262-
static zend_type zend_substitute_proto_type(
1259+
/* Internal: substitute proto's pre-erasure against ce's binding without
1260+
* applying the top-level type-parameter fallback. May return a type that
1261+
* still has TYPE_PARAMETER refs at the top — callers use this to detect
1262+
* the "would fall back" case so the child side can mirror it. Returns
1263+
* `fallback` (unchanged) when no substitution applies at all. */
1264+
static zend_type zend_substitute_proto_type_raw(
12631265
zend_type fallback,
12641266
const zend_type *pre_erasure,
12651267
const zend_function *proto,
@@ -1269,12 +1271,30 @@ static zend_type zend_substitute_proto_type(
12691271
return fallback;
12701272
}
12711273

1272-
/* Top-level type-param ref of function-scope origin can't be bound by
1273-
* inheritance. Compound types may still contain class-scope refs deep
1274-
* inside; the leaf walker filters origin per-leaf. */
1274+
/* Top-level type-param ref of function-scope origin: dereference to the
1275+
* param's bound's pre-erasure. This lets `set<U : T>(U $x)` substitute
1276+
* T → ce's binding in U's bound, so the inheritance check compares the
1277+
* substituted bound rather than the erased mixed. */
12751278
if (ZEND_TYPE_HAS_TYPE_PARAMETER(*pre_erasure)
12761279
&& ZEND_TYPE_TYPE_PARAMETER(*pre_erasure)->origin != ZEND_GENERIC_ORIGIN_CLASS_LIKE) {
1277-
return fallback;
1280+
const zend_type_parameter_ref *ref = ZEND_TYPE_TYPE_PARAMETER(*pre_erasure);
1281+
if (!ZEND_USER_CODE(proto->common.type)) return fallback;
1282+
const zend_op_array *op = &proto->op_array;
1283+
if (!op->generic_parameters || ref->index >= op->generic_parameters->count) {
1284+
return fallback;
1285+
}
1286+
const zend_generic_parameter *p = &op->generic_parameters->parameters[ref->index];
1287+
const zend_type *bound_pre = ZEND_TYPE_IS_SET(p->bound_pre_erasure)
1288+
? &p->bound_pre_erasure
1289+
: (ZEND_TYPE_IS_SET(p->bound) ? &p->bound : NULL);
1290+
if (!bound_pre || !zend_type_contains_type_parameter(*bound_pre)) {
1291+
return fallback;
1292+
}
1293+
/* Recurse with the bound as the pre-erasure. The recursive call may
1294+
* also need to deref (chained method-level bounds), but loop detection
1295+
* isn't needed for inheritance because the user-visible bound is a
1296+
* finite expression. */
1297+
return zend_substitute_proto_type_raw(fallback, bound_pre, proto, ce);
12781298
}
12791299
if (!zend_type_contains_type_parameter(*pre_erasure)) {
12801300
return fallback;
@@ -1298,49 +1318,96 @@ static zend_type zend_substitute_proto_type(
12981318
&& !zend_get_target_default_args(proto_scope, args, cap, &arity)) {
12991319
result = fallback;
13001320
} else {
1301-
zend_type substituted = zend_substitute_leaf_type_param(*pre_erasure, args, arity);
1302-
/* If the result is *still* a bare type-parameter (couldn't be ground),
1303-
* fall back to the erased form so the inheritance check stays symmetric
1304-
* with the other side's mixed. Compound types containing leftover
1305-
* type-param leaves are kept — the structural comparison below handles
1306-
* them by identity. */
1307-
result = ZEND_TYPE_HAS_TYPE_PARAMETER(substituted) ? fallback : substituted;
1321+
result = zend_substitute_leaf_type_param(*pre_erasure, args, arity);
13081322
}
13091323

13101324
free_alloca(args, use_heap);
13111325
return result;
13121326
}
13131327

1328+
/* If ce supplies type arguments to proto's declaring scope (directly,
1329+
* transitively, or via parameter defaults), returns proto's pre-erasure type
1330+
* with class-scope T-refs substituted. Applies a top-level fallback when the
1331+
* substituted result is still a bare TYPE_PARAMETER (couldn't be ground). */
1332+
static zend_type zend_substitute_proto_type(
1333+
zend_type fallback,
1334+
const zend_type *pre_erasure,
1335+
const zend_function *proto,
1336+
zend_class_entry *ce)
1337+
{
1338+
zend_type result = zend_substitute_proto_type_raw(fallback, pre_erasure, proto, ce);
1339+
/* If the result is *still* a bare type-parameter (couldn't be ground),
1340+
* fall back to the erased form. Compound types containing leftover
1341+
* type-param leaves are kept — the structural comparison handles them
1342+
* by identity. */
1343+
return ZEND_TYPE_HAS_TYPE_PARAMETER(result) ? fallback : result;
1344+
}
1345+
13141346
/* Returns the type the inheritance check should use for the child (fe) side.
13151347
* When the child has a pre-erasure stash and that stash carries class-scope
13161348
* shape the arg_info erased away (e.g. `Tl|Tr` collapsed to mixed because
13171349
* both members are unbounded class-scope type parameters), prefer the
13181350
* pre-erasure so the check sees the same structure that's substituted in
13191351
* for the parent side. Function-scope refs aren't bound by inheritance —
1320-
* they erase to their bound and we keep using the erased form. */
1352+
* they erase to their bound and we keep using the erased form.
1353+
*
1354+
* `proto_substituted_had_type_param` tells us whether the parent's
1355+
* substitution would have fallen back to its erased form (its substituted
1356+
* result was a bare TYPE_PARAMETER). When that's true and the child's
1357+
* pre-erasure is also a bare TYPE_PARAMETER, the child falls back too —
1358+
* keeping both sides at the erased mixed so the comparison stays symmetric.
1359+
* Otherwise the child uses its pre-erasure so structural compounds (unions
1360+
* like `Tl|Tr`, named-with-args like `I<T>`) line up with the parent's
1361+
* substituted form. */
13211362
static zend_type zend_resolve_fe_type(
13221363
zend_type fallback,
13231364
const zend_type *pre_erasure,
13241365
const zend_function *fe,
1325-
zend_class_entry *ce)
1366+
zend_class_entry *ce,
1367+
bool proto_substituted_had_type_param)
13261368
{
13271369
if (!pre_erasure || !ZEND_TYPE_IS_SET(*pre_erasure)) {
13281370
return fallback;
13291371
}
1372+
/* Function-scope TYPE_PARAMETER ref: deref to the param's bound's
1373+
* pre-erasure so its class-scope content can still drive the comparison.
1374+
* Mirrors the symmetric deref in zend_substitute_proto_type_raw. */
1375+
if (ZEND_TYPE_HAS_TYPE_PARAMETER(*pre_erasure)
1376+
&& ZEND_TYPE_TYPE_PARAMETER(*pre_erasure)->origin != ZEND_GENERIC_ORIGIN_CLASS_LIKE) {
1377+
const zend_type_parameter_ref *ref = ZEND_TYPE_TYPE_PARAMETER(*pre_erasure);
1378+
if (!ZEND_USER_CODE(fe->common.type)) return fallback;
1379+
const zend_op_array *op = &fe->op_array;
1380+
if (!op->generic_parameters || ref->index >= op->generic_parameters->count) {
1381+
return fallback;
1382+
}
1383+
const zend_generic_parameter *p = &op->generic_parameters->parameters[ref->index];
1384+
const zend_type *bound_pre = ZEND_TYPE_IS_SET(p->bound_pre_erasure)
1385+
? &p->bound_pre_erasure
1386+
: (ZEND_TYPE_IS_SET(p->bound) ? &p->bound : NULL);
1387+
if (!bound_pre) return fallback;
1388+
return zend_resolve_fe_type(fallback, bound_pre, fe, ce, proto_substituted_had_type_param);
1389+
}
13301390
if (!zend_type_contains_class_scope_type_parameter(*pre_erasure)) {
1391+
/* Pre-erasure has structure but no class-scope refs to substitute.
1392+
* If proto fell back (its substituted result was still a bare type
1393+
* param), fall back here too for symmetric mixed-vs-mixed compare;
1394+
* otherwise use the pre-erasure structure directly so unions like
1395+
* `string|int` compare properly. */
1396+
if (proto_substituted_had_type_param
1397+
&& ZEND_TYPE_HAS_TYPE_PARAMETER(*pre_erasure)) {
1398+
return fallback;
1399+
}
1400+
return *pre_erasure;
1401+
}
1402+
/* Mirror the parent's fall-back: if proto fell back AND fe's pre-erasure
1403+
* is itself a bare TYPE_PARAMETER, fall back to the erased form too. */
1404+
if (proto_substituted_had_type_param
1405+
&& ZEND_TYPE_HAS_TYPE_PARAMETER(*pre_erasure)) {
13311406
return fallback;
13321407
}
13331408
/* Immediate-child case: the pre-erasure type-param refs are already in
1334-
* fe_scope, which is the same scope the comparison runs in. Mirror the
1335-
* parent-side logic in zend_substitute_proto_type — if the pre-erasure
1336-
* is a bare top-level TYPE_PARAMETER, fall back to the erased form so
1337-
* both sides of the check see the same width (both substitute to the
1338-
* erased bound; the parent already falls back here, so the child must
1339-
* too or the comparison sees `T` vs `mixed` and spuriously rejects). */
1409+
* fe_scope, which is the same scope the comparison runs in. */
13401410
if (ce == fe->common.scope) {
1341-
if (ZEND_TYPE_HAS_TYPE_PARAMETER(*pre_erasure)) {
1342-
return fallback;
1343-
}
13441411
return *pre_erasure;
13451412
}
13461413
/* Transitive: substitute the child's class-scope refs against ce's
@@ -1453,17 +1520,17 @@ static inheritance_status zend_do_perform_implementation_check(
14531520
}
14541521

14551522
uint32_t proto_param_idx = i < proto_num_args ? i : proto_num_args - 1;
1456-
zend_type proto_type = zend_substitute_proto_type(
1523+
zend_type proto_raw = zend_substitute_proto_type_raw(
14571524
proto_arg_info->type,
14581525
zend_get_param_pre_erasure(proto, proto_param_idx),
1459-
proto,
1460-
ce
1461-
);
1526+
proto, ce);
1527+
bool proto_fell_back = ZEND_TYPE_HAS_TYPE_PARAMETER(proto_raw);
1528+
zend_type proto_type = proto_fell_back ? proto_arg_info->type : proto_raw;
14621529
uint32_t fe_param_idx = i < fe_num_args ? i : fe_num_args - 1;
14631530
zend_type fe_type = zend_resolve_fe_type(
14641531
fe_arg_info->type,
14651532
zend_get_param_pre_erasure(fe, fe_param_idx),
1466-
fe, ce);
1533+
fe, ce, proto_fell_back);
14671534
local_status = zend_do_perform_arg_type_hint_check(
14681535
fe_scope, fe_type, proto_scope, proto_type);
14691536

@@ -1495,14 +1562,17 @@ static inheritance_status zend_do_perform_implementation_check(
14951562
return status;
14961563
}
14971564

1498-
zend_type proto_return_type = zend_substitute_proto_type(
1565+
zend_type proto_raw_ret = zend_substitute_proto_type_raw(
14991566
proto->common.arg_info[-1].type,
15001567
zend_get_return_pre_erasure(proto),
15011568
proto, ce);
1569+
bool proto_ret_fell_back = ZEND_TYPE_HAS_TYPE_PARAMETER(proto_raw_ret);
1570+
zend_type proto_return_type = proto_ret_fell_back
1571+
? proto->common.arg_info[-1].type : proto_raw_ret;
15021572
zend_type fe_return_type = zend_resolve_fe_type(
15031573
fe->common.arg_info[-1].type,
15041574
zend_get_return_pre_erasure(fe),
1505-
fe, ce);
1575+
fe, ce, proto_ret_fell_back);
15061576
local_status = zend_perform_covariant_type_check(
15071577
fe_scope, fe_return_type, proto_scope, proto_return_type);
15081578

0 commit comments

Comments
 (0)