From a1f4eba0045e4155857578f3589e3dc48e99dcb4 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 17 Jan 2024 17:13:35 +0100 Subject: [PATCH 1/2] Fix GH-13177: PHP 8.3.2: final private constructor not allowed when used in trait zend_compile has an exception to this rule for constructors using `zend_is_constructor`, which compares the function name to `__construct`. Sadly, `zend_is_constructor` is not a public API, but we can just do the string compare ourselves. --- Zend/tests/traits/bugs/gh13177.phpt | 50 +++++++++++++++++++++++++++++ Zend/zend_inheritance.c | 2 +- 2 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 Zend/tests/traits/bugs/gh13177.phpt diff --git a/Zend/tests/traits/bugs/gh13177.phpt b/Zend/tests/traits/bugs/gh13177.phpt new file mode 100644 index 000000000000..43c61ec1f169 --- /dev/null +++ b/Zend/tests/traits/bugs/gh13177.phpt @@ -0,0 +1,50 @@ +--TEST-- +GH-13177 (PHP 8.3.2: final private constructor not allowed when used in trait) +--FILE-- +getMethod("__construct"), "\n"; +} + +class Foo4 extends Foo3 { + private function __construct() {} +} + +?> +--EXPECTF-- +Method [ final private method __construct ] { + @@ %sgh13177.php 4 - 4 +} + +Method [ final private method __construct ] { + @@ %sgh13177.php 4 - 4 +} + +Method [ final private method __construct ] { + @@ %sgh13177.php 4 - 4 +} + + +Fatal error: Cannot override final method Foo3::__construct() in %s on line %d diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index 6362090ada90..fa372dc5de2c 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -1949,7 +1949,7 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_ zend_function *new_fn; bool check_inheritance = false; - if ((fn->common.fn_flags & (ZEND_ACC_PRIVATE | ZEND_ACC_FINAL)) == (ZEND_ACC_PRIVATE | ZEND_ACC_FINAL)) { + if ((fn->common.fn_flags & (ZEND_ACC_PRIVATE | ZEND_ACC_FINAL)) == (ZEND_ACC_PRIVATE | ZEND_ACC_FINAL) && !zend_string_equals_literal_ci(name, ZEND_CONSTRUCTOR_FUNC_NAME)) { zend_error(E_COMPILE_WARNING, "Private methods cannot be final as they are never overridden by other classes"); } From 6dda94c97c374dd1d479658084910f9a0cf7c584 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Thu, 18 Jan 2024 20:22:53 +0100 Subject: [PATCH 2/2] Don't emit a warning when the definition already did --- Zend/tests/traits/bugs/gh13177.phpt | 17 +++++++++++++++-- Zend/zend_inheritance.c | 19 +++++++++++++++---- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/Zend/tests/traits/bugs/gh13177.phpt b/Zend/tests/traits/bugs/gh13177.phpt index 43c61ec1f169..42ef0ae9d60d 100644 --- a/Zend/tests/traits/bugs/gh13177.phpt +++ b/Zend/tests/traits/bugs/gh13177.phpt @@ -23,17 +23,26 @@ class Foo3 { } } -for ($i = 1; $i <= 3; $i++) { +trait TraitNonConstructor { + private final function test() {} +} + +class Foo4 { + use TraitNonConstructor { test as __construct; } +} + +for ($i = 1; $i <= 4; $i++) { $rc = new ReflectionClass("Foo$i"); echo $rc->getMethod("__construct"), "\n"; } -class Foo4 extends Foo3 { +class Foo5 extends Foo3 { private function __construct() {} } ?> --EXPECTF-- +Warning: Private methods cannot be final as they are never overridden by other classes in %s on line %d Method [ final private method __construct ] { @@ %sgh13177.php 4 - 4 } @@ -46,5 +55,9 @@ Method [ final private method __construct ] { @@ %sgh13177.php 4 - 4 } +Method [ final private method __construct ] { + @@ %sgh13177.php 24 - 24 +} + Fatal error: Cannot override final method Foo3::__construct() in %s on line %d diff --git a/Zend/zend_inheritance.c b/Zend/zend_inheritance.c index fa372dc5de2c..ea8cc219a23a 100644 --- a/Zend/zend_inheritance.c +++ b/Zend/zend_inheritance.c @@ -1949,10 +1949,6 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_ zend_function *new_fn; bool check_inheritance = false; - if ((fn->common.fn_flags & (ZEND_ACC_PRIVATE | ZEND_ACC_FINAL)) == (ZEND_ACC_PRIVATE | ZEND_ACC_FINAL) && !zend_string_equals_literal_ci(name, ZEND_CONSTRUCTOR_FUNC_NAME)) { - zend_error(E_COMPILE_WARNING, "Private methods cannot be final as they are never overridden by other classes"); - } - 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 * of where it is coming from there is no conflict and we do not need to add it again */ @@ -2033,6 +2029,17 @@ static void zend_fixup_trait_method(zend_function *fn, zend_class_entry *ce) /* } /* }}} */ +static void zend_traits_check_private_final_inheritance(uint32_t original_fn_flags, zend_function *fn_copy, zend_string *name) +{ + /* If the function was originally already private+final, then it will have already been warned about. + * If the function became private+final only after applying modifiers, we need to emit the same warning. */ + if ((original_fn_flags & (ZEND_ACC_PRIVATE | ZEND_ACC_FINAL)) != (ZEND_ACC_PRIVATE | ZEND_ACC_FINAL) + && (fn_copy->common.fn_flags & (ZEND_ACC_PRIVATE | ZEND_ACC_FINAL)) == (ZEND_ACC_PRIVATE | ZEND_ACC_FINAL) + && !zend_string_equals_literal_ci(name, ZEND_CONSTRUCTOR_FUNC_NAME)) { + zend_error(E_COMPILE_WARNING, "Private methods cannot be final as they are never overridden by other classes"); + } +} + static void zend_traits_copy_functions(zend_string *fnname, zend_function *fn, zend_class_entry *ce, HashTable *exclude_table, zend_class_entry **aliases) /* {{{ */ { zend_trait_alias *alias, **alias_ptr; @@ -2058,6 +2065,8 @@ static void zend_traits_copy_functions(zend_string *fnname, zend_function *fn, z fn_copy.common.fn_flags = alias->modifiers | fn->common.fn_flags; } + zend_traits_check_private_final_inheritance(fn->common.fn_flags, &fn_copy, alias->alias); + lcname = zend_string_tolower(alias->alias); zend_add_trait_method(ce, alias->alias, lcname, &fn_copy); zend_string_release_ex(lcname, 0); @@ -2095,6 +2104,8 @@ static void zend_traits_copy_functions(zend_string *fnname, zend_function *fn, z } } + zend_traits_check_private_final_inheritance(fn->common.fn_flags, &fn_copy, fnname); + zend_add_trait_method(ce, fn->common.function_name, fnname, &fn_copy); } }