Skip to content

Commit 86c7536

Browse files
committed
Bind traits before parent class
This more accurately matches the "copy & paste" semantics described in the documentation. Abstract trait methods diverge from this behavior, given that a parent method can satisfy trait methods used in the child. In that case, the method is not copied, but the check is performed after the parent has been bound. Fixes GH-15753
1 parent 99f72fa commit 86c7536

File tree

6 files changed

+148
-48
lines changed

6 files changed

+148
-48
lines changed

Zend/tests/gh16198.phpt

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
--TEST--
2+
GH-16198: Incorrect trait constant conflict when declared via trait
3+
--FILE--
4+
<?php
5+
6+
trait T1
7+
{
8+
final public const string C1 = 'T1';
9+
}
10+
11+
interface I1
12+
{
13+
public const ?string C1 = null;
14+
public const ?string C2 = null;
15+
}
16+
17+
final class O1 implements I1
18+
{
19+
final public const string C2 = 'O1';
20+
}
21+
22+
final class O2 implements I1
23+
{
24+
use T1;
25+
}
26+
27+
abstract class A1 implements I1
28+
{
29+
}
30+
31+
final class O3 extends A1
32+
{
33+
final public const string C2 = 'O3';
34+
}
35+
36+
final class O4 extends A1
37+
{
38+
use T1;
39+
}
40+
41+
?>
42+
===DONE===
43+
--EXPECT--
44+
===DONE===

Zend/tests/gh16198_2.phpt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
GH-16198: Usage of super in cloned trait method
3+
--FILE--
4+
<?php
5+
6+
trait T {
7+
public function __destruct() {
8+
parent::__destruct();
9+
}
10+
}
11+
12+
class P {
13+
public function __destruct() {
14+
var_dump(__METHOD__);
15+
}
16+
}
17+
18+
class C extends P {
19+
use T;
20+
}
21+
22+
$c = new C();
23+
unset($c);
24+
25+
?>
26+
--EXPECT--
27+
string(13) "P::__destruct"

Zend/tests/traits/constant_015.phpt

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,6 @@ class DerivedClass1 extends BaseClass1 {
1515
use TestTrait1;
1616
}
1717

18-
trait TestTrait2 {
19-
public final const Constant = 123;
20-
}
21-
22-
class BaseClass2 {
23-
public final const Constant = 456;
24-
}
25-
26-
class DerivedClass2 extends BaseClass2 {
27-
use TestTrait2;
28-
}
29-
3018
?>
3119
--EXPECTF--
32-
Fatal error: BaseClass2 and TestTrait2 define the same constant (Constant) in the composition of DerivedClass2. However, the definition differs and is considered incompatible. Class was composed in %s on line %d
20+
Fatal error: DerivedClass1::Constant cannot override final constant BaseClass1::Constant in %s on line %d

Zend/tests/traits/error_001.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,4 @@ class A extends foo {
2525

2626
?>
2727
--EXPECTF--
28-
Fatal error: Class A cannot extend trait foo in %s on line %d
28+
Fatal error: Required Trait foo2 wasn't added to A in %s on line %d

Zend/tests/traits/gh15753.phpt

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
--TEST--
2+
GH-15753: Overriding readonly properties from traits don't allow assignment from the child
3+
--FILE--
4+
<?php
5+
6+
class P {
7+
protected readonly int $prop;
8+
}
9+
10+
trait T {
11+
protected readonly int $prop;
12+
}
13+
14+
class C extends P {
15+
use T;
16+
17+
public function __construct() {
18+
$this->prop = 20;
19+
}
20+
}
21+
22+
$c = new C();
23+
var_dump($c);
24+
25+
?>
26+
--EXPECT--
27+
object(C)#1 (1) {
28+
["prop":protected]=>
29+
int(20)
30+
}

Zend/zend_inheritance.c

Lines changed: 45 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1135,7 +1135,10 @@ static inheritance_status do_inheritance_check_on_method(
11351135

11361136
#define SEPARATE_METHOD() do { \
11371137
if ((flags & ZEND_INHERITANCE_LAZY_CHILD_CLONE) \
1138-
&& child_scope != ce && child->type == ZEND_USER_FUNCTION) { \
1138+
&& child_scope != ce \
1139+
/* Trait scopes are fixed after inheritance. However, they are always duplicated. */ \
1140+
&& !(child_scope->ce_flags & ZEND_ACC_TRAIT) \
1141+
&& child->type == ZEND_USER_FUNCTION) { \
11391142
/* op_array wasn't duplicated yet */ \
11401143
zend_function *new_function = zend_arena_alloc(&CG(arena), sizeof(zend_op_array)); \
11411144
memcpy(new_function, child, sizeof(zend_op_array)); \
@@ -2695,7 +2698,7 @@ static void zend_traits_init_trait_structures(zend_class_entry *ce, zend_class_e
26952698
}
26962699
/* }}} */
26972700

2698-
static void zend_do_traits_method_binding(zend_class_entry *ce, zend_class_entry **traits, HashTable **exclude_tables, zend_class_entry **aliases) /* {{{ */
2701+
static void zend_do_traits_method_binding(zend_class_entry *ce, zend_class_entry **traits, HashTable **exclude_tables, zend_class_entry **aliases, bool verify_abstract, bool *contains_abstract_methods) /* {{{ */
26992702
{
27002703
uint32_t i;
27012704
zend_string *key;
@@ -2706,6 +2709,10 @@ static void zend_do_traits_method_binding(zend_class_entry *ce, zend_class_entry
27062709
if (traits[i]) {
27072710
/* copies functions, applies defined aliasing, and excludes unused trait methods */
27082711
ZEND_HASH_MAP_FOREACH_STR_KEY_PTR(&traits[i]->function_table, key, fn) {
2712+
if (verify_abstract != (bool) (fn->common.fn_flags & ZEND_ACC_ABSTRACT)) {
2713+
*contains_abstract_methods = true;
2714+
continue;
2715+
}
27092716
zend_traits_copy_functions(key, fn, ce, exclude_tables[i], aliases);
27102717
} ZEND_HASH_FOREACH_END();
27112718

@@ -2720,15 +2727,15 @@ static void zend_do_traits_method_binding(zend_class_entry *ce, zend_class_entry
27202727
for (i = 0; i < ce->num_traits; i++) {
27212728
if (traits[i]) {
27222729
ZEND_HASH_MAP_FOREACH_STR_KEY_PTR(&traits[i]->function_table, key, fn) {
2730+
if (verify_abstract != (bool) (fn->common.fn_flags & ZEND_ACC_ABSTRACT)) {
2731+
*contains_abstract_methods = true;
2732+
continue;
2733+
}
27232734
zend_traits_copy_functions(key, fn, ce, NULL, aliases);
27242735
} ZEND_HASH_FOREACH_END();
27252736
}
27262737
}
27272738
}
2728-
2729-
ZEND_HASH_MAP_FOREACH_PTR(&ce->function_table, fn) {
2730-
zend_fixup_trait_method(fn, ce);
2731-
} ZEND_HASH_FOREACH_END();
27322739
}
27332740
/* }}} */
27342741

@@ -3010,33 +3017,6 @@ static void zend_do_traits_property_binding(zend_class_entry *ce, zend_class_ent
30103017
}
30113018
/* }}} */
30123019

3013-
static void zend_do_bind_traits(zend_class_entry *ce, zend_class_entry **traits) /* {{{ */
3014-
{
3015-
HashTable **exclude_tables;
3016-
zend_class_entry **aliases;
3017-
3018-
ZEND_ASSERT(ce->num_traits > 0);
3019-
3020-
/* complete initialization of trait structures in ce */
3021-
zend_traits_init_trait_structures(ce, traits, &exclude_tables, &aliases);
3022-
3023-
/* first care about all methods to be flattened into the class */
3024-
zend_do_traits_method_binding(ce, traits, exclude_tables, aliases);
3025-
3026-
if (aliases) {
3027-
efree(aliases);
3028-
}
3029-
3030-
if (exclude_tables) {
3031-
efree(exclude_tables);
3032-
}
3033-
3034-
/* then flatten the constants and properties into it, to, mostly to notify developer about problems */
3035-
zend_do_traits_constant_binding(ce, traits);
3036-
zend_do_traits_property_binding(ce, traits);
3037-
}
3038-
/* }}} */
3039-
30403020
#define MAX_ABSTRACT_INFO_CNT 3
30413021
#define MAX_ABSTRACT_INFO_FMT "%s%s%s%s"
30423022
#define DISPLAY_ABSTRACT_FN(idx) \
@@ -3649,14 +3629,45 @@ ZEND_API zend_class_entry *zend_do_link_class(zend_class_entry *ce, zend_string
36493629
zend_link_hooked_object_iter(ce);
36503630
#endif
36513631

3632+
HashTable **trait_exclude_tables;
3633+
zend_class_entry **trait_aliases;
3634+
bool trait_contains_abstract_methods = false;
3635+
if (ce->num_traits) {
3636+
zend_traits_init_trait_structures(ce, traits_and_interfaces, &trait_exclude_tables, &trait_aliases);
3637+
zend_do_traits_method_binding(ce, traits_and_interfaces, trait_exclude_tables, trait_aliases, false, &trait_contains_abstract_methods);
3638+
zend_do_traits_constant_binding(ce, traits_and_interfaces);
3639+
zend_do_traits_property_binding(ce, traits_and_interfaces);
3640+
}
36523641
if (parent) {
36533642
if (!(parent->ce_flags & ZEND_ACC_LINKED)) {
36543643
add_dependency_obligation(ce, parent);
36553644
}
36563645
zend_do_inheritance(ce, parent);
36573646
}
36583647
if (ce->num_traits) {
3659-
zend_do_bind_traits(ce, traits_and_interfaces);
3648+
if (trait_contains_abstract_methods) {
3649+
zend_do_traits_method_binding(ce, traits_and_interfaces, trait_exclude_tables, trait_aliases, true, &trait_contains_abstract_methods);
3650+
}
3651+
3652+
if (trait_exclude_tables) {
3653+
for (i = 0; i < ce->num_traits; i++) {
3654+
if (traits_and_interfaces[i]) {
3655+
if (trait_exclude_tables[i]) {
3656+
zend_hash_destroy(trait_exclude_tables[i]);
3657+
FREE_HASHTABLE(trait_exclude_tables[i]);
3658+
}
3659+
}
3660+
}
3661+
efree(trait_exclude_tables);
3662+
}
3663+
if (trait_aliases) {
3664+
efree(trait_aliases);
3665+
}
3666+
3667+
zend_function *fn;
3668+
ZEND_HASH_MAP_FOREACH_PTR(&ce->function_table, fn) {
3669+
zend_fixup_trait_method(fn, ce);
3670+
} ZEND_HASH_FOREACH_END();
36603671
}
36613672
if (ce->num_interfaces) {
36623673
/* Also copy the parent interfaces here, so we don't need to reallocate later. */

0 commit comments

Comments
 (0)