Skip to content

Commit

Permalink
Fix GH-13177: PHP 8.3.2: final private constructor not allowed when u…
Browse files Browse the repository at this point in the history
…sed 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.

Closes GH-13179.
  • Loading branch information
nielsdos committed Jan 19, 2024
1 parent 8772810 commit c2c1710
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 4 deletions.
2 changes: 2 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ PHP NEWS
. Fixed bug GH-12349 (linking failure on ARM with mold). (Jan Palus)
. Fixed bug GH-13097 (Anonymous class reference in trigger_error / thrown
Exception). (nielsdos)
. Fixed bug GH-13177 (PHP 8.3.2: final private constructor not allowed
when used in trait). (nielsdos)

- Curl:
. Fix missing error check in curl_multi_init(). (divinity76)
Expand Down
63 changes: 63 additions & 0 deletions Zend/tests/traits/bugs/gh13177.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
--TEST--
GH-13177 (PHP 8.3.2: final private constructor not allowed when used in trait)
--FILE--
<?php

trait Bar {
final private function __construct() {}
}

final class Foo1 {
use Bar;
}

final class Foo2 {
use Bar {
__construct as final;
}
}

class Foo3 {
use Bar {
__construct as final;
}
}

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 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 [ <user, ctor> final private method __construct ] {
@@ %sgh13177.php 4 - 4
}

Method [ <user, ctor> final private method __construct ] {
@@ %sgh13177.php 4 - 4
}

Method [ <user, ctor> final private method __construct ] {
@@ %sgh13177.php 4 - 4
}

Method [ <user, ctor> final private method __construct ] {
@@ %sgh13177.php 24 - 24
}


Fatal error: Cannot override final method Foo3::__construct() in %s on line %d
19 changes: 15 additions & 4 deletions Zend/zend_inheritance.c
Original file line number Diff line number Diff line change
Expand Up @@ -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_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 */
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
}
Expand Down

0 comments on commit c2c1710

Please sign in to comment.