Skip to content

Commit

Permalink
Merge branch 'PHP-8.3'
Browse files Browse the repository at this point in the history
* PHP-8.3:
  Fix #[Override] on traits overriding a parent method without a matching interface (#12205)
  • Loading branch information
TimWolla committed Sep 15, 2023
2 parents 7aea6dd + d344fe0 commit 7c4db15
Show file tree
Hide file tree
Showing 7 changed files with 179 additions and 10 deletions.
42 changes: 42 additions & 0 deletions Zend/tests/attributes/override/gh12189.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
--TEST--
#[Override] attribute in trait does not check for parent class implementations
--FILE--
<?php

class A {
public function foo(): void {}
}

interface I {
public function foo(): void;
}

trait T {
#[\Override]
public function foo(): void {
echo 'foo';
}
}

// Works fine
class B implements I {
use T;
}

// Works fine ("copied and pasted into the target class")
class C extends A {
#[\Override]
public function foo(): void {
echo 'foo';
}
}

// Does not work
class D extends A {
use T;
}
echo "Done";

?>
--EXPECT--
Done
24 changes: 24 additions & 0 deletions Zend/tests/attributes/override/gh12189_2.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
--TEST--
#[Override] attribute in trait does not check for parent class implementations (Variant with private parent method)
--FILE--
<?php

class A {
private function foo(): void {}
}

trait T {
#[\Override]
public function foo(): void {
echo 'foo';
}
}

class D extends A {
use T;
}
echo "Done";

?>
--EXPECTF--
Fatal error: D::foo() has #[\Override] attribute, but no matching parent method exists in %s on line %d
24 changes: 24 additions & 0 deletions Zend/tests/attributes/override/gh12189_3.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
--TEST--
#[Override] attribute in trait does not check for parent class implementations (Variant with protected parent method)
--FILE--
<?php

class A {
protected function foo(): void {}
}

trait T {
#[\Override]
public function foo(): void {
echo 'foo';
}
}

class D extends A {
use T;
}
echo "Done";

?>
--EXPECTF--
Done
24 changes: 24 additions & 0 deletions Zend/tests/attributes/override/gh12189_4.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
--TEST--
#[Override] attribute in trait does not check for parent class implementations (Variant with __construct)
--FILE--
<?php

class A {
public function __construct() {}
}

trait T {
#[\Override]
public function __construct() {
echo 'foo';
}
}

class D extends A {
use T;
}
echo "Done";

?>
--EXPECTF--
Fatal error: D::__construct() has #[\Override] attribute, but no matching parent method exists in %s on line %d
24 changes: 24 additions & 0 deletions Zend/tests/attributes/override/gh12189_5.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
--TEST--
#[Override] attribute in trait does not check for parent class implementations (Variant with abstract __construct)
--FILE--
<?php

abstract class A {
abstract public function __construct();
}

trait T {
#[\Override]
public function __construct() {
echo 'foo';
}
}

class D extends A {
use T;
}
echo "Done";

?>
--EXPECT--
Done
25 changes: 25 additions & 0 deletions Zend/tests/attributes/override/gh12189_6.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
--TEST--
#[Override]: Inheritance check of inherited trait method against interface
--FILE--
<?php

trait T1 {
public abstract function test();
}

trait T2 {
public function test() {}
}

class A {
use T2;
}

class B extends A {
use T1;
}

?>
===DONE===
--EXPECT--
===DONE===
26 changes: 16 additions & 10 deletions Zend/zend_inheritance.c
Original file line number Diff line number Diff line change
Expand Up @@ -1081,12 +1081,13 @@ static void perform_delayable_implementation_check(
/**
* @param check_only Set to false to throw compile errors on incompatible methods, or true to return INHERITANCE_ERROR.
* @param checked Whether the compatibility check has already succeeded in zend_can_early_bind().
* @param force_mutable Whether we know that child may be modified, i.e. doesn't live in shm.
*/
static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(
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) /* {{{ */
bool check_visibility, bool check_only, bool checked, bool force_mutable) /* {{{ */
{
uint32_t child_flags;
uint32_t parent_flags = parent->common.fn_flags;
Expand Down Expand Up @@ -1188,7 +1189,7 @@ static zend_always_inline inheritance_status do_inheritance_check_on_method_ex(
perform_delayable_implementation_check(ce, child, child_scope, parent, parent_scope);
}

if (!check_only && child->common.scope == ce) {
if (!check_only && (child->common.scope == ce || force_mutable)) {
child->common.fn_flags &= ~ZEND_ACC_OVERRIDE;
}

Expand All @@ -1201,7 +1202,7 @@ static zend_never_inline void do_inheritance_check_on_method(
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);
do_inheritance_check_on_method_ex(child, child_scope, parent, parent_scope, ce, child_zv, check_visibility, 0, 0, /* force_mutable */ false);
}

static zend_always_inline void do_inherit_method(zend_string *key, zend_function *parent, zend_class_entry *ce, bool is_interface, bool checked) /* {{{ */
Expand All @@ -1219,7 +1220,7 @@ static zend_always_inline void do_inherit_method(zend_string *key, zend_function
if (checked) {
do_inheritance_check_on_method_ex(
func, func->common.scope, parent, parent->common.scope, ce, child,
/* check_visibility */ 1, 0, checked);
/* check_visibility */ 1, 0, checked, /* force_mutable */ false);
} else {
do_inheritance_check_on_method(
func, func->common.scope, parent, parent->common.scope, ce, child,
Expand Down Expand Up @@ -1946,6 +1947,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
Expand Down Expand Up @@ -1980,11 +1982,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;
}
}

Expand All @@ -2004,6 +2002,14 @@ 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_ex(
fn, fixup_trait_scope(fn, ce), existing_fn, fixup_trait_scope(existing_fn, ce),
ce, NULL, /* check_visibility */ 1, false, false, /* force_mutable */ true);
}
}
/* }}} */

Expand Down Expand Up @@ -3239,7 +3245,7 @@ static inheritance_status zend_can_early_bind(zend_class_entry *ce, zend_class_e
do_inheritance_check_on_method_ex(
child_func, child_func->common.scope,
parent_func, parent_func->common.scope,
ce, NULL, /* check_visibility */ 1, 1, 0);
ce, NULL, /* check_visibility */ 1, 1, 0, /* force_mutable */ false);
if (UNEXPECTED(status == INHERITANCE_WARNING)) {
overall_status = INHERITANCE_WARNING;
} else if (UNEXPECTED(status != INHERITANCE_SUCCESS)) {
Expand Down

0 comments on commit 7c4db15

Please sign in to comment.