Skip to content

Commit cf7c682

Browse files
committed
Fix treatment of "self" when validating against trait method
If we're validating a class method against a trait method, we need to treat "self" in the trait method as the class where the method is used. To achieve this, we need to thread the proto scope through all methods, so it can be provided separately from proto.common->scope.
1 parent 13df3fc commit cf7c682

File tree

4 files changed

+131
-35
lines changed

4 files changed

+131
-35
lines changed
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
--TEST--
2+
Abstract method in trait using "self" (invalid)
3+
--FILE--
4+
<?php
5+
6+
trait T {
7+
abstract private function method(self $x): self;
8+
}
9+
10+
class C {
11+
use T;
12+
13+
private function method(int $x): int { }
14+
}
15+
16+
?>
17+
===DONE===
18+
--EXPECTF--
19+
Fatal error: Declaration of C::method(int $x): int must be compatible with T::method(C $x): C in %s on line %d
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--TEST--
2+
Abstract method in trait using "self"
3+
--FILE--
4+
<?php
5+
6+
trait T {
7+
abstract private function method(self $x): self;
8+
}
9+
10+
class C {
11+
use T;
12+
13+
private function method(self $x): self {
14+
return $this;
15+
}
16+
}
17+
18+
?>
19+
===DONE===
20+
--EXPECT--
21+
===DONE===
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
Abstract method in trait using "self" (delayed obligation)
3+
--FILE--
4+
<?php
5+
6+
spl_autoload_register(function($class) {
7+
if ($class == T::class) {
8+
trait T {
9+
abstract private function method($x): self;
10+
}
11+
} else if ($class == C::class) {
12+
class C {
13+
use T;
14+
15+
private function method($x): D {
16+
return new D;
17+
}
18+
}
19+
} else if ($class == D::class) {
20+
class D extends C {}
21+
}
22+
});
23+
24+
new C;
25+
26+
?>
27+
===DONE===
28+
--EXPECT--
29+
===DONE===

Zend/zend_inheritance.c

Lines changed: 62 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@
2929

3030
static void add_dependency_obligation(zend_class_entry *ce, zend_class_entry *dependency_ce);
3131
static void add_compatibility_obligation(
32-
zend_class_entry *ce, const zend_function *child_fn, const zend_function *parent_fn);
32+
zend_class_entry *ce, const zend_function *child_fn, const zend_function *parent_fn,
33+
zend_class_entry *parent_scope);
3334
static void add_property_compatibility_obligation(
3435
zend_class_entry *ce, const zend_property_info *child_prop,
3536
const zend_property_info *parent_prop);
@@ -496,8 +497,8 @@ static inheritance_status zend_perform_covariant_type_check(
496497
/* }}} */
497498

498499
static inheritance_status zend_do_perform_arg_type_hint_check(
499-
const zend_function *fe, zend_arg_info *fe_arg_info,
500-
const zend_function *proto, zend_arg_info *proto_arg_info) /* {{{ */
500+
zend_class_entry *fe_scope, zend_arg_info *fe_arg_info,
501+
zend_class_entry *proto_scope, zend_arg_info *proto_arg_info) /* {{{ */
501502
{
502503
if (!ZEND_TYPE_IS_SET(fe_arg_info->type)) {
503504
/* Child with no type is always compatible */
@@ -512,12 +513,16 @@ static inheritance_status zend_do_perform_arg_type_hint_check(
512513
/* Contravariant type check is performed as a covariant type check with swapped
513514
* argument order. */
514515
return zend_perform_covariant_type_check(
515-
proto->common.scope, proto_arg_info->type, fe->common.scope, fe_arg_info->type);
516+
proto_scope, proto_arg_info->type, fe_scope, fe_arg_info->type);
516517
}
517518
/* }}} */
518519

520+
/* For abstract trait methods, proto_scope will differ from proto->common.scope,
521+
* as self will refer to the self of the class the trait is used in, not the trait
522+
* the method was declared in. */
519523
static inheritance_status zend_do_perform_implementation_check(
520-
const zend_function *fe, const zend_function *proto) /* {{{ */
524+
const zend_function *fe, const zend_function *proto,
525+
zend_class_entry *proto_scope) /* {{{ */
521526
{
522527
uint32_t i, num_args, proto_num_args, fe_num_args;
523528
inheritance_status status, local_status;
@@ -578,7 +583,8 @@ static inheritance_status zend_do_perform_implementation_check(
578583
return INHERITANCE_ERROR;
579584
}
580585

581-
local_status = zend_do_perform_arg_type_hint_check(fe, fe_arg_info, proto, proto_arg_info);
586+
local_status = zend_do_perform_arg_type_hint_check(
587+
fe->common.scope, fe_arg_info, proto_scope, proto_arg_info);
582588

583589
if (UNEXPECTED(local_status != INHERITANCE_SUCCESS)) {
584590
if (UNEXPECTED(local_status == INHERITANCE_ERROR)) {
@@ -604,7 +610,7 @@ static inheritance_status zend_do_perform_implementation_check(
604610

605611
local_status = zend_perform_covariant_type_check(
606612
fe->common.scope, fe->common.arg_info[-1].type,
607-
proto->common.scope, proto->common.arg_info[-1].type);
613+
proto_scope, proto->common.arg_info[-1].type);
608614

609615
if (UNEXPECTED(local_status != INHERITANCE_SUCCESS)) {
610616
if (UNEXPECTED(local_status == INHERITANCE_ERROR)) {
@@ -619,10 +625,11 @@ static inheritance_status zend_do_perform_implementation_check(
619625
}
620626
/* }}} */
621627

622-
static ZEND_COLD void zend_append_type_hint(smart_str *str, const zend_function *fptr, zend_arg_info *arg_info, int return_hint) /* {{{ */
628+
static ZEND_COLD void zend_append_type_hint(
629+
smart_str *str, zend_class_entry *scope, zend_arg_info *arg_info, int return_hint) /* {{{ */
623630
{
624631
if (ZEND_TYPE_IS_SET(arg_info->type)) {
625-
zend_string *type_str = zend_type_to_string_resolved(arg_info->type, fptr->common.scope);
632+
zend_string *type_str = zend_type_to_string_resolved(arg_info->type, scope);
626633
smart_str_append(str, type_str);
627634
zend_string_release(type_str);
628635
if (!return_hint) {
@@ -632,7 +639,8 @@ static ZEND_COLD void zend_append_type_hint(smart_str *str, const zend_function
632639
}
633640
/* }}} */
634641

635-
static ZEND_COLD zend_string *zend_get_function_declaration(const zend_function *fptr) /* {{{ */
642+
static ZEND_COLD zend_string *zend_get_function_declaration(
643+
const zend_function *fptr, zend_class_entry *scope) /* {{{ */
636644
{
637645
smart_str str = {0};
638646

@@ -659,7 +667,7 @@ static ZEND_COLD zend_string *zend_get_function_declaration(const zend_function
659667
num_args++;
660668
}
661669
for (i = 0; i < num_args;) {
662-
zend_append_type_hint(&str, fptr, arg_info, 0);
670+
zend_append_type_hint(&str, scope, arg_info, 0);
663671

664672
if (ZEND_ARG_SEND_MODE(arg_info)) {
665673
smart_str_appendc(&str, '&');
@@ -752,7 +760,7 @@ static ZEND_COLD zend_string *zend_get_function_declaration(const zend_function
752760

753761
if (fptr->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) {
754762
smart_str_appends(&str, ": ");
755-
zend_append_type_hint(&str, fptr, fptr->common.arg_info - 1, 1);
763+
zend_append_type_hint(&str, scope, fptr->common.arg_info - 1, 1);
756764
}
757765
smart_str_0(&str);
758766

@@ -765,10 +773,10 @@ static zend_always_inline uint32_t func_lineno(const zend_function *fn) {
765773
}
766774

767775
static void ZEND_COLD emit_incompatible_method_error(
768-
const zend_function *child, const zend_function *parent,
776+
const zend_function *child, const zend_function *parent, zend_class_entry *parent_scope,
769777
inheritance_status status) {
770-
zend_string *parent_prototype = zend_get_function_declaration(parent);
771-
zend_string *child_prototype = zend_get_function_declaration(child);
778+
zend_string *parent_prototype = zend_get_function_declaration(parent, parent_scope);
779+
zend_string *child_prototype = zend_get_function_declaration(child, child->common.scope);
772780
if (status == INHERITANCE_UNRESOLVED) {
773781
/* Fetch the first unresolved class from registered autoloads */
774782
zend_string *unresolved_class = NULL;
@@ -791,20 +799,23 @@ static void ZEND_COLD emit_incompatible_method_error(
791799

792800
static void perform_delayable_implementation_check(
793801
zend_class_entry *ce, const zend_function *fe,
794-
const zend_function *proto)
802+
const zend_function *proto, zend_class_entry *proto_scope)
795803
{
796-
inheritance_status status = zend_do_perform_implementation_check(fe, proto);
804+
inheritance_status status = zend_do_perform_implementation_check(fe, proto, proto_scope);
797805
if (UNEXPECTED(status != INHERITANCE_SUCCESS)) {
798806
if (EXPECTED(status == INHERITANCE_UNRESOLVED)) {
799-
add_compatibility_obligation(ce, fe, proto);
807+
add_compatibility_obligation(ce, fe, proto, proto_scope);
800808
} else {
801809
ZEND_ASSERT(status == INHERITANCE_ERROR);
802-
emit_incompatible_method_error(fe, proto, status);
810+
emit_incompatible_method_error(fe, proto, proto_scope, status);
803811
}
804812
}
805813
}
806814

807-
static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(zend_function *child, zend_function *parent, zend_class_entry *ce, zval *child_zv, zend_bool check_visibility, zend_bool check_only, zend_bool checked) /* {{{ */
815+
static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(
816+
zend_function *child, zend_function *parent,
817+
zend_class_entry *ce, zend_class_entry *parent_scope, zval *child_zv,
818+
zend_bool check_visibility, zend_bool check_only, zend_bool checked) /* {{{ */
808819
{
809820
uint32_t child_flags;
810821
uint32_t parent_flags = parent->common.fn_flags;
@@ -899,17 +910,20 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(z
899910

900911
if (!checked) {
901912
if (check_only) {
902-
return zend_do_perform_implementation_check(child, parent);
913+
return zend_do_perform_implementation_check(child, parent, parent_scope);
903914
}
904-
perform_delayable_implementation_check(ce, child, parent);
915+
perform_delayable_implementation_check(ce, child, parent, parent_scope);
905916
}
906917
return INHERITANCE_SUCCESS;
907918
}
908919
/* }}} */
909920

910-
static zend_never_inline void do_inheritance_check_on_method(zend_function *child, zend_function *parent, zend_class_entry *ce, zval *child_zv, zend_bool check_visibility) /* {{{ */
921+
static zend_never_inline void do_inheritance_check_on_method(
922+
zend_function *child, zend_function *parent,
923+
zend_class_entry *ce, zend_class_entry *parent_scope,
924+
zval *child_zv, zend_bool check_visibility) /* {{{ */
911925
{
912-
do_inheritance_check_on_method_ex(child, parent, ce, child_zv, check_visibility, 0, 0);
926+
do_inheritance_check_on_method_ex(child, parent, ce, parent_scope, child_zv, check_visibility, 0, 0);
913927
}
914928
/* }}} */
915929

@@ -927,9 +941,11 @@ static zend_always_inline void do_inherit_method(zend_string *key, zend_function
927941

928942
if (checked) {
929943
do_inheritance_check_on_method_ex(
930-
func, parent, ce, child, /* check_visibility */ 1, 0, checked);
944+
func, parent, ce, parent->common.scope, child,
945+
/* check_visibility */ 1, 0, checked);
931946
} else {
932-
do_inheritance_check_on_method(func, parent, ce, child, /* check_visibility */ 1);
947+
do_inheritance_check_on_method(
948+
func, parent, ce, parent->common.scope, child, /* check_visibility */ 1);
933949
}
934950
} else {
935951

@@ -1589,8 +1605,13 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_
15891605
/* "abstract private" methods in traits were not available prior to PHP 8.
15901606
* As such, "abstract protected" was sometimes used to indicate trait requirements,
15911607
* even though the "implementing" method was private. Do not check visibility
1592-
* requirements to maintain backwards-compatibility with such usage. */
1593-
do_inheritance_check_on_method(existing_fn, fn, ce, NULL, /* check_visibility */ 0);
1608+
* requirements to maintain backwards-compatibility with such usage.
1609+
*
1610+
* The parent_scope passed here is not fn->common.scope, because we want "self" to be
1611+
* resolved against the using class, not the declaring trait.
1612+
*/
1613+
do_inheritance_check_on_method(
1614+
existing_fn, fn, ce, /* parent_scope */ ce, NULL, /* check_visibility */ 0);
15941615
return;
15951616
}
15961617

@@ -1607,7 +1628,8 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_
16071628
} else {
16081629
/* inherited members are overridden by members inserted by traits */
16091630
/* check whether the trait method fulfills the inheritance requirements */
1610-
do_inheritance_check_on_method(fn, existing_fn, ce, NULL, /* check_visibility */ 1);
1631+
do_inheritance_check_on_method(
1632+
fn, existing_fn, ce, existing_fn->common.scope, NULL, /* check_visibility */ 1);
16111633
}
16121634
}
16131635

@@ -2182,6 +2204,7 @@ typedef struct {
21822204
* so use copies of functions here as well. */
21832205
zend_function parent_fn;
21842206
zend_function child_fn;
2207+
zend_class_entry *parent_scope;
21852208
};
21862209
struct {
21872210
const zend_property_info *parent_prop;
@@ -2229,7 +2252,8 @@ static void add_dependency_obligation(zend_class_entry *ce, zend_class_entry *de
22292252
}
22302253

22312254
static void add_compatibility_obligation(
2232-
zend_class_entry *ce, const zend_function *child_fn, const zend_function *parent_fn) {
2255+
zend_class_entry *ce, const zend_function *child_fn, const zend_function *parent_fn,
2256+
zend_class_entry *parent_scope) {
22332257
HashTable *obligations = get_or_init_obligations_for_class(ce);
22342258
variance_obligation *obligation = emalloc(sizeof(variance_obligation));
22352259
obligation->type = OBLIGATION_COMPATIBILITY;
@@ -2244,6 +2268,7 @@ static void add_compatibility_obligation(
22442268
} else {
22452269
memcpy(&obligation->parent_fn, parent_fn, sizeof(zend_op_array));
22462270
}
2271+
obligation->parent_scope = parent_scope;
22472272
zend_hash_next_index_insert_ptr(obligations, obligation);
22482273
}
22492274

@@ -2272,13 +2297,14 @@ static int check_variance_obligation(zval *zv) {
22722297
}
22732298
} else if (obligation->type == OBLIGATION_COMPATIBILITY) {
22742299
inheritance_status status = zend_do_perform_implementation_check(
2275-
&obligation->child_fn, &obligation->parent_fn);
2300+
&obligation->child_fn, &obligation->parent_fn, obligation->parent_scope);
22762301
if (UNEXPECTED(status != INHERITANCE_SUCCESS)) {
22772302
if (EXPECTED(status == INHERITANCE_UNRESOLVED)) {
22782303
return ZEND_HASH_APPLY_KEEP;
22792304
}
22802305
ZEND_ASSERT(status == INHERITANCE_ERROR);
2281-
emit_incompatible_method_error(&obligation->child_fn, &obligation->parent_fn, status);
2306+
emit_incompatible_method_error(
2307+
&obligation->child_fn, &obligation->parent_fn, obligation->parent_scope, status);
22822308
}
22832309
/* Either the compatibility check was successful or only threw a warning. */
22842310
} else {
@@ -2345,10 +2371,10 @@ static void report_variance_errors(zend_class_entry *ce) {
23452371
/* Just used to populate the delayed_autoloads table,
23462372
* which will be used when printing the "unresolved" error. */
23472373
inheritance_status status = zend_do_perform_implementation_check(
2348-
&obligation->child_fn, &obligation->parent_fn);
2374+
&obligation->child_fn, &obligation->parent_fn, obligation->parent_scope);
23492375
ZEND_ASSERT(status == INHERITANCE_UNRESOLVED);
23502376
emit_incompatible_method_error(
2351-
&obligation->child_fn, &obligation->parent_fn, status);
2377+
&obligation->child_fn, &obligation->parent_fn, obligation->parent_scope, status);
23522378
} else if (obligation->type == OBLIGATION_PROPERTY_COMPATIBILITY) {
23532379
emit_incompatible_property_error(obligation->child_prop, obligation->parent_prop);
23542380
} else {
@@ -2475,7 +2501,8 @@ static inheritance_status zend_can_early_bind(zend_class_entry *ce, zend_class_e
24752501
zend_function *child_func = Z_FUNC_P(zv);
24762502
inheritance_status status =
24772503
do_inheritance_check_on_method_ex(
2478-
child_func, parent_func, ce, NULL, /* check_visibility */ 1, 1, 0);
2504+
child_func, parent_func, ce, parent_func->common.scope, NULL,
2505+
/* check_visibility */ 1, 1, 0);
24792506

24802507
if (UNEXPECTED(status != INHERITANCE_SUCCESS)) {
24812508
return status;

0 commit comments

Comments
 (0)