diff --git a/Zend/tests/gh14009_001.phpt b/Zend/tests/gh14009_001.phpt new file mode 100644 index 000000000000..81325e814c27 --- /dev/null +++ b/Zend/tests/gh14009_001.phpt @@ -0,0 +1,41 @@ +--TEST-- +GH-14009: Traits inherit prototype +--FILE-- +common(); + } +} + +class B extends P { + protected function common() { + echo __METHOD__, "\n"; + } +} + +trait T { + protected function common() { + echo __METHOD__, "\n"; + } +} + +class C extends P { + use T; +} + +$a = new A(); +$a->test(new B()); +$a->test(new C()); + +?> +--EXPECT-- +B::common +T::common diff --git a/Zend/tests/gh14009_002.phpt b/Zend/tests/gh14009_002.phpt new file mode 100644 index 000000000000..86047e020205 --- /dev/null +++ b/Zend/tests/gh14009_002.phpt @@ -0,0 +1,22 @@ +--TEST-- +GH-14009: Traits inherit prototype +--FILE-- + +--EXPECTF-- +Fatal error: Declaration of P::common() must be compatible with T::common(int $param) in %s on line %d diff --git a/Zend/tests/gh14009_003.phpt b/Zend/tests/gh14009_003.phpt new file mode 100644 index 000000000000..71ee5baa360a --- /dev/null +++ b/Zend/tests/gh14009_003.phpt @@ -0,0 +1,19 @@ +--TEST-- +GH-14009: Traits inherit prototype +--FILE-- + +--EXPECT-- diff --git a/Zend/tests/gh14009_004.phpt b/Zend/tests/gh14009_004.phpt new file mode 100644 index 000000000000..01bad46fedcb --- /dev/null +++ b/Zend/tests/gh14009_004.phpt @@ -0,0 +1,37 @@ +--TEST-- +GH-14009: Traits inherit prototype +--FILE-- +test(); + } +} + +D::callTest(new C()); + +?> +--EXPECT-- +B::test diff --git a/Zend/tests/interface_constructor_prototype_001.phpt b/Zend/tests/interface_constructor_prototype_001.phpt new file mode 100644 index 000000000000..67341367ac58 --- /dev/null +++ b/Zend/tests/interface_constructor_prototype_001.phpt @@ -0,0 +1,19 @@ +--TEST-- +Interfaces don't set prototypes to their parent method +--FILE-- + +--EXPECTF-- +Fatal error: Declaration of Test::__construct(int $param) must be compatible with B::__construct(int|float $param) in %s on line %d diff --git a/Zend/tests/interface_constructor_prototype_002.phpt b/Zend/tests/interface_constructor_prototype_002.phpt new file mode 100644 index 000000000000..76398d2d215b --- /dev/null +++ b/Zend/tests/interface_constructor_prototype_002.phpt @@ -0,0 +1,26 @@ +--TEST-- +Interfaces don't set prototypes to their parent method +--XFAIL-- +X::__constructor()'s prototype is set to B::__construct(). Y::__construct() then +uses prototype to verify LSP, but misses A::__construct() which has a stricter +signature. +--FILE-- + +--EXPECTF-- +Fatal error: Declaration of Y::__construct(int $param) must be compatible with A::__construct(int|float $param) in %s on line %d diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index cec0ffdb1853..695d31be0fbf 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -1075,26 +1075,45 @@ static void perform_delayable_implementation_check( } } -static zend_always_inline inheritance_status do_inheritance_check_on_method_ex( + +#define ZEND_INHERITANCE_LAZY_CHILD_CLONE (1<<0) +#define ZEND_INHERITANCE_CHECK_SILENT (1<<1) /* don't throw errors */ +#define ZEND_INHERITANCE_CHECK_PROTO (1<<2) /* check method prototype (it might be already checked before) */ +#define ZEND_INHERITANCE_CHECK_VISIBILITY (1<<3) +#define ZEND_INHERITANCE_SET_CHILD_CHANGED (1<<4) +#define ZEND_INHERITANCE_SET_CHILD_PROTO (1<<5) + +static inheritance_status do_inheritance_check_on_method( zend_function *child, zend_class_entry *child_scope, zend_function *parent, zend_class_entry *parent_scope, - zend_class_entry *ce, zval *child_zv, - bool check_visibility, bool check_only, bool checked) /* {{{ */ + zend_class_entry *ce, zval *child_zv, uint32_t flags) /* {{{ */ { uint32_t child_flags; uint32_t parent_flags = parent->common.fn_flags; zend_function *proto; - if (UNEXPECTED((parent_flags & ZEND_ACC_PRIVATE) && !(parent_flags & ZEND_ACC_ABSTRACT) && !(parent_flags & ZEND_ACC_CTOR))) { - if (!check_only) { +#define SEPARATE_METHOD() do { \ + if ((flags & ZEND_INHERITANCE_LAZY_CHILD_CLONE) \ + && child_scope != ce && child->type == ZEND_USER_FUNCTION) { \ + /* op_array wasn't duplicated yet */ \ + zend_function *new_function = zend_arena_alloc(&CG(arena), sizeof(zend_op_array)); \ + memcpy(new_function, child, sizeof(zend_op_array)); \ + Z_PTR_P(child_zv) = child = new_function; \ + flags &= ~ZEND_INHERITANCE_LAZY_CHILD_CLONE; \ + } \ + } while(0) + + if (UNEXPECTED((parent_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_ABSTRACT|ZEND_ACC_CTOR)) == ZEND_ACC_PRIVATE)) { + if (flags & ZEND_INHERITANCE_SET_CHILD_CHANGED) { + SEPARATE_METHOD(); child->common.fn_flags |= ZEND_ACC_CHANGED; } /* The parent method is private and not an abstract so we don't need to check any inheritance rules */ return INHERITANCE_SUCCESS; } - if (!checked && UNEXPECTED(parent_flags & ZEND_ACC_FINAL)) { - if (check_only) { + if ((flags & ZEND_INHERITANCE_CHECK_PROTO) && UNEXPECTED(parent_flags & ZEND_ACC_FINAL)) { + if (flags & ZEND_INHERITANCE_CHECK_SILENT) { return INHERITANCE_ERROR; } zend_error_at_noreturn(E_COMPILE_ERROR, func_filename(child), func_lineno(child), @@ -1105,8 +1124,9 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex( child_flags = child->common.fn_flags; /* You cannot change from static to non static and vice versa. */ - if (!checked && UNEXPECTED((child_flags & ZEND_ACC_STATIC) != (parent_flags & ZEND_ACC_STATIC))) { - if (check_only) { + if ((flags & ZEND_INHERITANCE_CHECK_PROTO) + && UNEXPECTED((child_flags & ZEND_ACC_STATIC) != (parent_flags & ZEND_ACC_STATIC))) { + if (flags & ZEND_INHERITANCE_CHECK_SILENT) { return INHERITANCE_ERROR; } if (child_flags & ZEND_ACC_STATIC) { @@ -1121,8 +1141,9 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex( } /* Disallow making an inherited method abstract. */ - if (!checked && UNEXPECTED((child_flags & ZEND_ACC_ABSTRACT) > (parent_flags & ZEND_ACC_ABSTRACT))) { - if (check_only) { + if ((flags & ZEND_INHERITANCE_CHECK_PROTO) + && UNEXPECTED((child_flags & ZEND_ACC_ABSTRACT) > (parent_flags & ZEND_ACC_ABSTRACT))) { + if (flags & ZEND_INHERITANCE_CHECK_SILENT) { return INHERITANCE_ERROR; } zend_error_at_noreturn(E_COMPILE_ERROR, func_filename(child), func_lineno(child), @@ -1130,7 +1151,9 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex( ZEND_FN_SCOPE_NAME(parent), ZSTR_VAL(child->common.function_name), ZEND_FN_SCOPE_NAME(child)); } - if (!check_only && (parent_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_CHANGED))) { + if ((flags & ZEND_INHERITANCE_SET_CHILD_CHANGED) + && (parent_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_CHANGED))) { + SEPARATE_METHOD(); child->common.fn_flags |= ZEND_ACC_CHANGED; } @@ -1146,27 +1169,16 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex( parent = proto; } - if (!check_only && child->common.prototype != proto && child_zv) { - do { - if (child->common.scope != ce && child->type == ZEND_USER_FUNCTION) { - if (ce->ce_flags & ZEND_ACC_INTERFACE) { - /* Few parent interfaces contain the same method */ - break; - } else { - /* op_array wasn't duplicated yet */ - zend_function *new_function = zend_arena_alloc(&CG(arena), sizeof(zend_op_array)); - memcpy(new_function, child, sizeof(zend_op_array)); - Z_PTR_P(child_zv) = child = new_function; - } - } - child->common.prototype = proto; - } while (0); + if ((flags & ZEND_INHERITANCE_SET_CHILD_PROTO) + && child->common.prototype != proto) { + SEPARATE_METHOD(); + child->common.prototype = proto; } /* Prevent derived classes from restricting access that was available in parent classes (except deriving from non-abstract ctors) */ - if (!checked && check_visibility + if ((flags & ZEND_INHERITANCE_CHECK_VISIBILITY) && (child_flags & ZEND_ACC_PPP_MASK) > (parent_flags & ZEND_ACC_PPP_MASK)) { - if (check_only) { + if (flags & ZEND_INHERITANCE_CHECK_SILENT) { return INHERITANCE_ERROR; } zend_error_at_noreturn(E_COMPILE_ERROR, func_filename(child), func_lineno(child), @@ -1174,25 +1186,20 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex( ZEND_FN_SCOPE_NAME(child), ZSTR_VAL(child->common.function_name), zend_visibility_string(parent_flags), ZEND_FN_SCOPE_NAME(parent), (parent_flags&ZEND_ACC_PUBLIC) ? "" : " or weaker"); } - if (!checked) { - if (check_only) { + if (flags & ZEND_INHERITANCE_CHECK_PROTO) { + if (flags & ZEND_INHERITANCE_CHECK_SILENT) { return zend_do_perform_implementation_check(child, child_scope, parent, parent_scope); } perform_delayable_implementation_check(ce, child, child_scope, parent, parent_scope); } + +#undef SEPARATE_METHOD + return INHERITANCE_SUCCESS; } /* }}} */ -static zend_never_inline void do_inheritance_check_on_method( - zend_function *child, zend_class_entry *child_scope, - zend_function *parent, zend_class_entry *parent_scope, - zend_class_entry *ce, zval *child_zv, bool check_visibility) -{ - 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, bool is_interface, bool checked) /* {{{ */ +static void do_inherit_method(zend_string *key, zend_function *parent, zend_class_entry *ce, bool is_interface, uint32_t flags) /* {{{ */ { zval *child = zend_hash_find_known_hash(&ce->function_table, key); @@ -1204,15 +1211,8 @@ static zend_always_inline void do_inherit_method(zend_string *key, zend_function return; } - if (checked) { - do_inheritance_check_on_method_ex( - func, func->common.scope, parent, parent->common.scope, ce, child, - /* check_visibility */ 1, 0, checked); - } else { - do_inheritance_check_on_method( - func, func->common.scope, parent, parent->common.scope, ce, child, - /* check_visibility */ 1); - } + do_inheritance_check_on_method( + func, func->common.scope, parent, parent->common.scope, ce, child, flags); } else { if (is_interface || (parent->common.fn_flags & (ZEND_ACC_ABSTRACT))) { @@ -1618,16 +1618,15 @@ ZEND_API void zend_do_inheritance_ex(zend_class_entry *ce, zend_class_entry *par zend_hash_extend(&ce->function_table, zend_hash_num_elements(&ce->function_table) + zend_hash_num_elements(&parent_ce->function_table), 0); + uint32_t flags = + ZEND_INHERITANCE_LAZY_CHILD_CLONE | ZEND_INHERITANCE_SET_CHILD_CHANGED | ZEND_INHERITANCE_SET_CHILD_PROTO; - if (checked) { - ZEND_HASH_MAP_FOREACH_STR_KEY_PTR(&parent_ce->function_table, key, func) { - do_inherit_method(key, func, ce, 0, 1); - } ZEND_HASH_FOREACH_END(); - } else { - ZEND_HASH_MAP_FOREACH_STR_KEY_PTR(&parent_ce->function_table, key, func) { - do_inherit_method(key, func, ce, 0, 0); - } ZEND_HASH_FOREACH_END(); + if (!checked) { + flags |= ZEND_INHERITANCE_CHECK_PROTO | ZEND_INHERITANCE_CHECK_VISIBILITY; } + ZEND_HASH_MAP_FOREACH_STR_KEY_PTR(&parent_ce->function_table, key, func) { + do_inherit_method(key, func, ce, 0, flags); + } ZEND_HASH_FOREACH_END(); } do_inherit_parent_constructor(ce); @@ -1733,13 +1732,20 @@ static void do_interface_implementation(zend_class_entry *ce, zend_class_entry * zend_function *func; zend_string *key; zend_class_constant *c; + uint32_t flags = ZEND_INHERITANCE_CHECK_PROTO | ZEND_INHERITANCE_CHECK_VISIBILITY; + + if (!(ce->ce_flags & ZEND_ACC_INTERFACE)) { + /* We are not setting the prototype of overridden interface methods because of abstract + * constructors. See Zend/tests/interface_constructor_prototype_001.phpt. */ + flags |= ZEND_INHERITANCE_LAZY_CHILD_CLONE | ZEND_INHERITANCE_SET_CHILD_PROTO; + } ZEND_HASH_MAP_FOREACH_STR_KEY_PTR(&iface->constants_table, key, c) { do_inherit_iface_constant(key, c, ce, iface); } ZEND_HASH_FOREACH_END(); ZEND_HASH_MAP_FOREACH_STR_KEY_PTR(&iface->function_table, key, func) { - do_inherit_method(key, func, ce, 1, 0); + do_inherit_method(key, func, ce, 1, flags); } ZEND_HASH_FOREACH_END(); do_implement_interface(ce, iface); @@ -1868,6 +1874,7 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_ { zend_function *existing_fn = NULL; zend_function *new_fn; + bool check_inheritance = false; if ((existing_fn = zend_hash_find_ptr(&ce->function_table, key)) != NULL) { /* if it is the same function with the same visibility and has not been assigned a class scope yet, regardless @@ -1887,7 +1894,7 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_ */ do_inheritance_check_on_method( existing_fn, fixup_trait_scope(existing_fn, ce), fn, fixup_trait_scope(fn, ce), - ce, NULL, /* check_visibility */ 0); + ce, NULL, ZEND_INHERITANCE_CHECK_PROTO); return; } @@ -1902,11 +1909,7 @@ 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. */ - do_inheritance_check_on_method( - fn, fixup_trait_scope(fn, ce), existing_fn, fixup_trait_scope(existing_fn, ce), - ce, NULL, /* check_visibility */ 1); + check_inheritance = true; } } @@ -1926,6 +1929,15 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_ function_add_ref(new_fn); fn = zend_hash_update_ptr(&ce->function_table, key, new_fn); zend_add_magic_method(ce, fn, key); + + if (check_inheritance) { + /* Inherited members are overridden by members inserted by traits. + * Check whether the trait method fulfills the inheritance requirements. */ + do_inheritance_check_on_method( + fn, fixup_trait_scope(fn, ce), existing_fn, fixup_trait_scope(existing_fn, ce), + ce, NULL, + ZEND_INHERITANCE_CHECK_PROTO | ZEND_INHERITANCE_CHECK_VISIBILITY | ZEND_INHERITANCE_SET_CHILD_PROTO); + } } /* }}} */ @@ -3103,10 +3115,11 @@ static inheritance_status zend_can_early_bind(zend_class_entry *ce, zend_class_e if (zv) { zend_function *child_func = Z_FUNC_P(zv); inheritance_status status = - do_inheritance_check_on_method_ex( + do_inheritance_check_on_method( child_func, child_func->common.scope, parent_func, parent_func->common.scope, - ce, NULL, /* check_visibility */ 1, 1, 0); + ce, NULL, + ZEND_INHERITANCE_CHECK_SILENT | ZEND_INHERITANCE_CHECK_PROTO | ZEND_INHERITANCE_CHECK_VISIBILITY); if (UNEXPECTED(status == INHERITANCE_WARNING)) { overall_status = INHERITANCE_WARNING; } else if (UNEXPECTED(status != INHERITANCE_SUCCESS)) {