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
Fixes GH-16198
Close GH-15878

NEWS

@@ -41,6 +41,7 @@ PHP                                                                        NEWS
   . Added the (void) cast to indicate that not using a value is intentional.
     (timwolla)
   . Added get_error_handler(), get_exception_handler() functions. (Arnaud)
+  . Fixed bug GH-15753 and GH-16198 (Bind traits before parent class). (ilutov)
 
 - Curl:
   . Added curl_multi_get_handles(). (timwolla)

UPGRADING

@@ -41,6 +41,9 @@ PHP 8.5 UPGRADE NOTES
   . The tick handlers are now deactivated after all shutdown functions, destructors
     have run and the output handlers have been cleaned up.
     This is a consequence of fixing GH-18033.
+  . Traits are now bound before the parent class. This is a subtle behavioral
+    change, but should closer match user expectations, demonstrated by GH-15753
+    and GH-16198.
 
 - Intl:
   . The extension now requires at least ICU 57.1.

Zend/tests/traits/constant_015.phpt

@@ -1,5 +1,5 @@
 --TEST--
-The same name constant of a trait used in a class that inherits a constant defined in a parent can be defined only if they are compatible.
+Final class constant in parent may not be overridden by child through trait
 --FILE--
 <?php
 
@@ -15,18 +15,6 @@ class DerivedClass1 extends BaseClass1 {
     use TestTrait1;
 }
 
-trait TestTrait2 {
-    public final const Constant = 123;
-}
-
-class BaseClass2 {
-    public final const Constant = 456;
-}
-
-class DerivedClass2 extends BaseClass2 {
-    use TestTrait2;
-}
-
 ?>
 --EXPECTF--
-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
+Fatal error: DerivedClass1::Constant cannot override final constant BaseClass1::Constant in %s on line %d

Zend/tests/traits/gh15753.phpt

@@ -0,0 +1,23 @@
+--TEST--
+GH-15753: Trait can override property declared in parent class of used class
+--FILE--
+<?php
+
+class P {
+    public $prop = 1;
+}
+
+trait T {
+    public $prop = 2;
+}
+
+class C extends P {
+    use T;
+}
+
+$c = new C();
+var_dump($c->prop);
+
+?>
+--EXPECT--
+int(2)

Zend/tests/traits/gh16198.phpt

@@ -0,0 +1,36 @@
+--TEST--
+GH-16198: Incorrect trait constant conflict when declared via trait
+--FILE--
+<?php
+
+trait T1 {
+    final public const string C1 = 'T1';
+}
+
+interface I1 {
+    public const ?string C1 = null;
+    public const ?string C2 = null;
+}
+
+final class O1 implements I1 {
+    final public const string C2 = 'O1';
+}
+
+final class O2 implements I1 {
+    use T1;
+}
+
+abstract class A1 implements I1 {}
+
+final class O3 extends A1 {
+    final public const string C2 = 'O3';
+}
+
+final class O4 extends A1 {
+    use T1;
+}
+
+?>
+===DONE===
+--EXPECT--
+===DONE===

Zend/tests/traits/gh16198_2.phpt

@@ -0,0 +1,27 @@
+--TEST--
+GH-16198: Usage of super in cloned trait method
+--FILE--
+<?php
+
+trait T {
+    public function __destruct() {
+        parent::__destruct();
+    }
+}
+
+class P {
+    public function __destruct() {
+        var_dump(__METHOD__);
+    }
+}
+
+class C extends P {
+    use T;
+}
+
+$c = new C();
+unset($c);
+
+?>
+--EXPECT--
+string(13) "P::__destruct"

Zend/zend_inheritance.c

@@ -1135,7 +1135,12 @@ static inheritance_status do_inheritance_check_on_method(
 
 #define SEPARATE_METHOD() do { \
 			if ((flags & ZEND_INHERITANCE_LAZY_CHILD_CLONE) \
-			 && child_scope != ce && child->type == ZEND_USER_FUNCTION) { \
+			 && child_scope != ce \
+			 /* Trait methods have already been separated at this point. However, their */ \
+			 /* scope isn't fixed until after inheritance checks to preserve the scope */ \
+			 /* in error messages. Skip them here explicitly. */ \
+			 && !(child_scope->ce_flags & ZEND_ACC_TRAIT) \
+			 && 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)); \
@@ -2350,7 +2355,6 @@ 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
@@ -2384,8 +2388,6 @@ static void zend_add_trait_method(zend_class_entry *ce, zend_string *name, zend_
 				ZSTR_VAL(fn->common.scope->name), ZSTR_VAL(fn->common.function_name),
 				ZSTR_VAL(ce->name), ZSTR_VAL(name),
 				ZSTR_VAL(existing_fn->common.scope->name), ZSTR_VAL(existing_fn->common.function_name));
-		} else {
-			check_inheritance = true;
 		}
 	}
 
@@ -2405,19 +2407,6 @@ 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. */
-		uint32_t flags = ZEND_INHERITANCE_CHECK_PROTO | ZEND_INHERITANCE_CHECK_VISIBILITY;
-		if (!(existing_fn->common.scope->ce_flags & ZEND_ACC_TRAIT)) {
-			flags |= ZEND_INHERITANCE_SET_CHILD_CHANGED |ZEND_INHERITANCE_SET_CHILD_PROTO |
-				ZEND_INHERITANCE_RESET_CHILD_OVERRIDE;
-		}
-		do_inheritance_check_on_method(
-			fn, fixup_trait_scope(fn, ce), existing_fn, fixup_trait_scope(existing_fn, ce),
-			ce, NULL, flags);
-	}
 }
 /* }}} */
 
@@ -2695,7 +2684,7 @@ static void zend_traits_init_trait_structures(zend_class_entry *ce, zend_class_e
 }
 /* }}} */
 
-static void zend_do_traits_method_binding(zend_class_entry *ce, zend_class_entry **traits, HashTable **exclude_tables, zend_class_entry **aliases) /* {{{ */
+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) /* {{{ */
 {
 	uint32_t i;
 	zend_string *key;
@@ -2706,6 +2695,11 @@ static void zend_do_traits_method_binding(zend_class_entry *ce, zend_class_entry
 			if (traits[i]) {
 				/* copies functions, applies defined aliasing, and excludes unused trait methods */
 				ZEND_HASH_MAP_FOREACH_STR_KEY_PTR(&traits[i]->function_table, key, fn) {
+					bool is_abstract = (bool) (fn->common.fn_flags & ZEND_ACC_ABSTRACT);
+					*contains_abstract_methods |= is_abstract;
+					if (verify_abstract != is_abstract) {
+						continue;
+					}
 					zend_traits_copy_functions(key, fn, ce, exclude_tables[i], aliases);
 				} ZEND_HASH_FOREACH_END();
 
@@ -2720,15 +2714,16 @@ static void zend_do_traits_method_binding(zend_class_entry *ce, zend_class_entry
 		for (i = 0; i < ce->num_traits; i++) {
 			if (traits[i]) {
 				ZEND_HASH_MAP_FOREACH_STR_KEY_PTR(&traits[i]->function_table, key, fn) {
+					bool is_abstract = (bool) (fn->common.fn_flags & ZEND_ACC_ABSTRACT);
+					*contains_abstract_methods |= is_abstract;
+					if (verify_abstract != is_abstract) {
+						continue;
+					}
 					zend_traits_copy_functions(key, fn, ce, NULL, aliases);
 				} ZEND_HASH_FOREACH_END();
 			}
 		}
 	}
-
-	ZEND_HASH_MAP_FOREACH_PTR(&ce->function_table, fn) {
-		zend_fixup_trait_method(fn, ce);
-	} ZEND_HASH_FOREACH_END();
 }
 /* }}} */
 
@@ -3010,33 +3005,6 @@ static void zend_do_traits_property_binding(zend_class_entry *ce, zend_class_ent
 }
 /* }}} */
 
-static void zend_do_bind_traits(zend_class_entry *ce, zend_class_entry **traits) /* {{{ */
-{
-	HashTable **exclude_tables;
-	zend_class_entry **aliases;
-
-	ZEND_ASSERT(ce->num_traits > 0);
-
-	/* complete initialization of trait structures in ce */
-	zend_traits_init_trait_structures(ce, traits, &exclude_tables, &aliases);
-
-	/* first care about all methods to be flattened into the class */
-	zend_do_traits_method_binding(ce, traits, exclude_tables, aliases);
-
-	if (aliases) {
-		efree(aliases);
-	}
-
-	if (exclude_tables) {
-		efree(exclude_tables);
-	}
-
-	/* then flatten the constants and properties into it, to, mostly to notify developer about problems */
-	zend_do_traits_constant_binding(ce, traits);
-	zend_do_traits_property_binding(ce, traits);
-}
-/* }}} */
-
 #define MAX_ABSTRACT_INFO_CNT 3
 #define MAX_ABSTRACT_INFO_FMT "%s%s%s%s"
 #define DISPLAY_ABSTRACT_FN(idx) \
@@ -3649,6 +3617,15 @@ ZEND_API zend_class_entry *zend_do_link_class(zend_class_entry *ce, zend_string
 		zend_link_hooked_object_iter(ce);
 #endif
 
+		HashTable **trait_exclude_tables;
+		zend_class_entry **trait_aliases;
+		bool trait_contains_abstract_methods = false;
+		if (ce->num_traits) {
+			zend_traits_init_trait_structures(ce, traits_and_interfaces, &trait_exclude_tables, &trait_aliases);
+			zend_do_traits_method_binding(ce, traits_and_interfaces, trait_exclude_tables, trait_aliases, false, &trait_contains_abstract_methods);
+			zend_do_traits_constant_binding(ce, traits_and_interfaces);
+			zend_do_traits_property_binding(ce, traits_and_interfaces);
+		}
 		if (parent) {
 			if (!(parent->ce_flags & ZEND_ACC_LINKED)) {
 				add_dependency_obligation(ce, parent);
@@ -3656,7 +3633,29 @@ ZEND_API zend_class_entry *zend_do_link_class(zend_class_entry *ce, zend_string
 			zend_do_inheritance(ce, parent);
 		}
 		if (ce->num_traits) {
-			zend_do_bind_traits(ce, traits_and_interfaces);
+			if (trait_contains_abstract_methods) {
+				zend_do_traits_method_binding(ce, traits_and_interfaces, trait_exclude_tables, trait_aliases, true, &trait_contains_abstract_methods);
+			}
+
+			if (trait_exclude_tables) {
+				for (i = 0; i < ce->num_traits; i++) {
+					if (traits_and_interfaces[i]) {
+						if (trait_exclude_tables[i]) {
+							zend_hash_destroy(trait_exclude_tables[i]);
+							FREE_HASHTABLE(trait_exclude_tables[i]);
+						}
+					}
+				}
+				efree(trait_exclude_tables);
+			}
+			if (trait_aliases) {
+				efree(trait_aliases);
+			}
+
+			zend_function *fn;
+			ZEND_HASH_MAP_FOREACH_PTR(&ce->function_table, fn) {
+				zend_fixup_trait_method(fn, ce);
+			} ZEND_HASH_FOREACH_END();
 		}
 		if (ce->num_interfaces) {
 			/* Also copy the parent interfaces here, so we don't need to reallocate later. */