]> granicus.if.org Git - php/commitdiff
Fix bug #80055
authorNikita Popov <nikita.ppv@gmail.com>
Thu, 15 Oct 2020 10:46:07 +0000 (12:46 +0200)
committerNikita Popov <nikita.ppv@gmail.com>
Thu, 15 Oct 2020 12:24:25 +0000 (14:24 +0200)
We need to perform trait scope fixup for both methods involved
in the inheritance check. For that purpose we already need to
thread through a separate fn scope through the entire inheritance
checking machinery.

NEWS
Zend/tests/bug80055.phpt [new file with mode: 0644]
Zend/zend_inheritance.c

diff --git a/NEWS b/NEWS
index b6cf08b3f2b5f8200234f7e278007fede7cb56bb..481d67ddcbf47941ae10a71f6b2e60b952f4853c 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,10 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? ????, PHP 8.0.0RC3
 
+- Core:
+  . Fixed bug #8055 (Abstract trait methods returning "self" cannot be
+    fulfilled by traits). (Nikita)
+
 - IMAP:
   . Fixed bug #80239 (imap_rfc822_write_address() leaks memory). (cmb)
 
diff --git a/Zend/tests/bug80055.phpt b/Zend/tests/bug80055.phpt
new file mode 100644 (file)
index 0000000..f43ab49
--- /dev/null
@@ -0,0 +1,24 @@
+--TEST--
+Bug #80055: Abstract trait methods returning "self" cannot be fulfilled by traits
+--FILE--
+<?php
+
+trait AbstractTrait {
+    abstract public function selfReturner(): self;
+}
+
+trait ConcreteTrait {
+    public function selfReturner(): self {
+        return $this;
+    }
+}
+
+class Test {
+    use AbstractTrait;
+    use ConcreteTrait;
+}
+
+?>
+===DONE===
+--EXPECT--
+===DONE===
index cf4fdea45f2d49822f47be2afc28bb1a82cd43f2..191126eed79f08d93895c1339afb3e28e8ce0d77 100644 (file)
@@ -29,8 +29,8 @@
 
 static void add_dependency_obligation(zend_class_entry *ce, zend_class_entry *dependency_ce);
 static void add_compatibility_obligation(
-               zend_class_entry *ce, const zend_function *child_fn, const zend_function *parent_fn,
-               zend_class_entry *parent_scope);
+               zend_class_entry *ce, const zend_function *child_fn, zend_class_entry *child_scope,
+               const zend_function *parent_fn, zend_class_entry *parent_scope);
 static void add_property_compatibility_obligation(
                zend_class_entry *ce, const zend_property_info *child_prop,
                const zend_property_info *parent_prop);
@@ -522,12 +522,12 @@ static inheritance_status zend_do_perform_arg_type_hint_check(
 }
 /* }}} */
 
-/* For abstract trait methods, proto_scope will differ from proto->common.scope,
+/* For trait methods, fe_scope/proto_scope may differ from fe/proto->common.scope,
  * as self will refer to the self of the class the trait is used in, not the trait
  * the method was declared in. */
 static inheritance_status zend_do_perform_implementation_check(
-               const zend_function *fe, const zend_function *proto,
-               zend_class_entry *proto_scope) /* {{{ */
+               const zend_function *fe, zend_class_entry *fe_scope,
+               const zend_function *proto, zend_class_entry *proto_scope) /* {{{ */
 {
        uint32_t i, num_args, proto_num_args, fe_num_args;
        inheritance_status status, local_status;
@@ -589,7 +589,7 @@ static inheritance_status zend_do_perform_implementation_check(
                }
 
                local_status = zend_do_perform_arg_type_hint_check(
-                       fe->common.scope, fe_arg_info, proto_scope, proto_arg_info);
+                       fe_scope, fe_arg_info, proto_scope, proto_arg_info);
 
                if (UNEXPECTED(local_status != INHERITANCE_SUCCESS)) {
                        if (UNEXPECTED(local_status == INHERITANCE_ERROR)) {
@@ -614,7 +614,7 @@ static inheritance_status zend_do_perform_implementation_check(
                }
 
                local_status = zend_perform_covariant_type_check(
-                       fe->common.scope, fe->common.arg_info[-1].type,
+                       fe_scope, fe->common.arg_info[-1].type,
                        proto_scope, proto->common.arg_info[-1].type);
 
                if (UNEXPECTED(local_status != INHERITANCE_SUCCESS)) {
@@ -778,10 +778,11 @@ static zend_always_inline uint32_t func_lineno(const zend_function *fn) {
 }
 
 static void ZEND_COLD emit_incompatible_method_error(
-               const zend_function *child, const zend_function *parent, zend_class_entry *parent_scope,
+               const zend_function *child, zend_class_entry *child_scope,
+               const zend_function *parent, zend_class_entry *parent_scope,
                inheritance_status status) {
        zend_string *parent_prototype = zend_get_function_declaration(parent, parent_scope);
-       zend_string *child_prototype = zend_get_function_declaration(child, child->common.scope);
+       zend_string *child_prototype = zend_get_function_declaration(child, child_scope);
        if (status == INHERITANCE_UNRESOLVED) {
                /* Fetch the first unresolved class from registered autoloads */
                zend_string *unresolved_class = NULL;
@@ -803,23 +804,26 @@ static void ZEND_COLD emit_incompatible_method_error(
 }
 
 static void perform_delayable_implementation_check(
-               zend_class_entry *ce, const zend_function *fe,
+               zend_class_entry *ce,
+               const zend_function *fe, zend_class_entry *fe_scope,
                const zend_function *proto, zend_class_entry *proto_scope)
 {
-       inheritance_status status = zend_do_perform_implementation_check(fe, proto, proto_scope);
+       inheritance_status status =
+               zend_do_perform_implementation_check(fe, fe_scope, proto, proto_scope);
        if (UNEXPECTED(status != INHERITANCE_SUCCESS)) {
                if (EXPECTED(status == INHERITANCE_UNRESOLVED)) {
-                       add_compatibility_obligation(ce, fe, proto, proto_scope);
+                       add_compatibility_obligation(ce, fe, fe_scope, proto, proto_scope);
                } else {
                        ZEND_ASSERT(status == INHERITANCE_ERROR);
-                       emit_incompatible_method_error(fe, proto, proto_scope, status);
+                       emit_incompatible_method_error(fe, fe_scope, proto, proto_scope, status);
                }
        }
 }
 
 static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(
-               zend_function *child, zend_function *parent,
-               zend_class_entry *ce, zend_class_entry *parent_scope, zval *child_zv,
+               zend_function *child, zend_class_entry *child_scope,
+               zend_function *parent, zend_class_entry *parent_scope,
+               zend_class_entry *ce, zval *child_zv,
                zend_bool check_visibility, zend_bool check_only, zend_bool checked) /* {{{ */
 {
        uint32_t child_flags;
@@ -919,22 +923,21 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(
 
        if (!checked) {
                if (check_only) {
-                       return zend_do_perform_implementation_check(child, parent, parent_scope);
+                       return zend_do_perform_implementation_check(child, child_scope, parent, parent_scope);
                }
-               perform_delayable_implementation_check(ce, child, parent, parent_scope);
+               perform_delayable_implementation_check(ce, child, child_scope, parent, parent_scope);
        }
        return INHERITANCE_SUCCESS;
 }
 /* }}} */
 
 static zend_never_inline void do_inheritance_check_on_method(
-               zend_function *child, zend_function *parent,
-               zend_class_entry *ce, zend_class_entry *parent_scope,
-               zval *child_zv, zend_bool check_visibility) /* {{{ */
+               zend_function *child, zend_class_entry *child_scope,
+               zend_function *parent, zend_class_entry *parent_scope,
+               zend_class_entry *ce, zval *child_zv, zend_bool check_visibility)
 {
-       do_inheritance_check_on_method_ex(child, parent, ce, parent_scope, child_zv, check_visibility, 0, 0);
+       do_inheritance_check_on_method_ex(child, child_scope, parent, parent_scope, ce, child_zv, check_visibility, 0, 0);
 }
-/* }}} */
 
 static zend_always_inline void do_inherit_method(zend_string *key, zend_function *parent, zend_class_entry *ce, zend_bool is_interface, zend_bool checked) /* {{{ */
 {
@@ -950,11 +953,12 @@ static zend_always_inline void do_inherit_method(zend_string *key, zend_function
 
                if (checked) {
                        do_inheritance_check_on_method_ex(
-                               func, parent, ce, parent->common.scope, child,
+                               func, func->common.scope, parent, parent->common.scope, ce, child,
                                /* check_visibility */ 1, 0, checked);
                } else {
                        do_inheritance_check_on_method(
-                               func, parent, ce, parent->common.scope, child, /* check_visibility */ 1);
+                               func, func->common.scope, parent, parent->common.scope, ce, child,
+                               /* check_visibility */ 1);
                }
        } else {
 
@@ -1557,6 +1561,12 @@ static void zend_do_implement_interfaces(zend_class_entry *ce, zend_class_entry
 }
 /* }}} */
 
+static zend_class_entry *fixup_trait_scope(const zend_function *fn, zend_class_entry *ce)
+{
+       /* self in trait methods should be resolved to the using class, not the trait. */
+       return fn->common.scope->ce_flags & ZEND_ACC_TRAIT ? ce : fn->common.scope;
+}
+
 static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_string *key, zend_function *fn) /* {{{ */
 {
        zend_function *existing_fn = NULL;
@@ -1577,12 +1587,10 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_
                         * As such, "abstract protected" was sometimes used to indicate trait requirements,
                         * even though the "implementing" method was private. Do not check visibility
                         * requirements to maintain backwards-compatibility with such usage.
-                        *
-                        * The parent_scope passed here is not fn->common.scope, because we want "self" to be
-                        * resolved against the using class, not the declaring trait.
                         */
                        do_inheritance_check_on_method(
-                               existing_fn, fn, ce, /* parent_scope */ ce, NULL, /* check_visibility */ 0);
+                               existing_fn, fixup_trait_scope(existing_fn, ce), fn, fixup_trait_scope(fn, ce),
+                               ce, NULL, /* check_visibility */ 0);
                        return;
                }
 
@@ -1597,10 +1605,11 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_
                                ZSTR_VAL(ce->name), ZSTR_VAL(name),
                                ZSTR_VAL(existing_fn->common.scope->name), ZSTR_VAL(existing_fn->common.function_name));
                } else {
-                       /* inherited members are overridden by members inserted by traits */
-                       /* check whether the trait method fulfills the inheritance requirements */
+                       /* Inherited members are overridden by members inserted by traits.
+                        * Check whether the trait method fulfills the inheritance requirements. */
                        do_inheritance_check_on_method(
-                               fn, existing_fn, ce, existing_fn->common.scope, NULL, /* check_visibility */ 1);
+                               fn, fixup_trait_scope(fn, ce), existing_fn, fixup_trait_scope(existing_fn, ce),
+                               ce, NULL, /* check_visibility */ 1);
                }
        }
 
@@ -2186,6 +2195,7 @@ typedef struct {
                         * so use copies of functions here as well. */
                        zend_function parent_fn;
                        zend_function child_fn;
+                       zend_class_entry *child_scope;
                        zend_class_entry *parent_scope;
                };
                struct {
@@ -2234,8 +2244,9 @@ static void add_dependency_obligation(zend_class_entry *ce, zend_class_entry *de
 }
 
 static void add_compatibility_obligation(
-               zend_class_entry *ce, const zend_function *child_fn, const zend_function *parent_fn,
-               zend_class_entry *parent_scope) {
+               zend_class_entry *ce,
+               const zend_function *child_fn, zend_class_entry *child_scope,
+               const zend_function *parent_fn, zend_class_entry *parent_scope) {
        HashTable *obligations = get_or_init_obligations_for_class(ce);
        variance_obligation *obligation = emalloc(sizeof(variance_obligation));
        obligation->type = OBLIGATION_COMPATIBILITY;
@@ -2250,6 +2261,7 @@ static void add_compatibility_obligation(
        } else {
                memcpy(&obligation->parent_fn, parent_fn, sizeof(zend_op_array));
        }
+       obligation->child_scope = child_scope;
        obligation->parent_scope = parent_scope;
        zend_hash_next_index_insert_ptr(obligations, obligation);
 }
@@ -2279,14 +2291,16 @@ static int check_variance_obligation(zval *zv) {
                }
        } else if (obligation->type == OBLIGATION_COMPATIBILITY) {
                inheritance_status status = zend_do_perform_implementation_check(
-                       &obligation->child_fn, &obligation->parent_fn, obligation->parent_scope);
+                       &obligation->child_fn, obligation->child_scope,
+                       &obligation->parent_fn, obligation->parent_scope);
                if (UNEXPECTED(status != INHERITANCE_SUCCESS)) {
                        if (EXPECTED(status == INHERITANCE_UNRESOLVED)) {
                                return ZEND_HASH_APPLY_KEEP;
                        }
                        ZEND_ASSERT(status == INHERITANCE_ERROR);
                        emit_incompatible_method_error(
-                               &obligation->child_fn, &obligation->parent_fn, obligation->parent_scope, status);
+                               &obligation->child_fn, obligation->child_scope,
+                               &obligation->parent_fn, obligation->parent_scope, status);
                }
                /* Either the compatibility check was successful or only threw a warning. */
        } else {
@@ -2353,10 +2367,12 @@ static void report_variance_errors(zend_class_entry *ce) {
                        /* Just used to populate the delayed_autoloads table,
                         * which will be used when printing the "unresolved" error. */
                        inheritance_status status = zend_do_perform_implementation_check(
-                               &obligation->child_fn, &obligation->parent_fn, obligation->parent_scope);
+                               &obligation->child_fn, obligation->child_scope,
+                               &obligation->parent_fn, obligation->parent_scope);
                        ZEND_ASSERT(status == INHERITANCE_UNRESOLVED);
                        emit_incompatible_method_error(
-                               &obligation->child_fn, &obligation->parent_fn, obligation->parent_scope, status);
+                               &obligation->child_fn, obligation->child_scope,
+                               &obligation->parent_fn, obligation->parent_scope, status);
                } else if (obligation->type == OBLIGATION_PROPERTY_COMPATIBILITY) {
                        emit_incompatible_property_error(obligation->child_prop, obligation->parent_prop);
                } else {
@@ -2482,8 +2498,9 @@ static inheritance_status zend_can_early_bind(zend_class_entry *ce, zend_class_e
                        zend_function *child_func = Z_FUNC_P(zv);
                        inheritance_status status =
                                do_inheritance_check_on_method_ex(
-                                       child_func, parent_func, ce, parent_func->common.scope, NULL,
-                                       /* check_visibility */ 1, 1, 0);
+                                       child_func, child_func->common.scope,
+                                       parent_func, parent_func->common.scope,
+                                       ce, NULL, /* check_visibility */ 1, 1, 0);
 
                        if (UNEXPECTED(status != INHERITANCE_SUCCESS)) {
                                return status;