Separate out trait constants inheritance and simplify outputting error message

Author: sj-i

Zend/tests/traits/constant_005.phpt

@@ -19,4 +19,4 @@ echo "POST-CLASS-GUARD\n";
 --EXPECTF--
 PRE-CLASS-GUARD
 
-Fatal error: ComposingClass and TestTrait define the same constant (Constant). However, the definition differs and is considered incompatible. Class was composed in %s on line %d
+Fatal error: ComposingClass and TestTrait define the same constant (Constant) in the composition of ComposingClass. However, the definition differs and is considered incompatible. Class was composed in %s on line %d

Zend/tests/traits/constant_006.phpt

@@ -19,4 +19,4 @@ echo "POST-CLASS-GUARD\n";
 --EXPECTF--
 PRE-CLASS-GUARD
 
-Fatal error: ComposingClass and TestTrait define the same constant (Constant). However, the definition differs and is considered incompatible. Class was composed in %s on line %d
+Fatal error: ComposingClass and TestTrait define the same constant (Constant) in the composition of ComposingClass. However, the definition differs and is considered incompatible. Class was composed in %s on line %d

Zend/tests/traits/constant_007.phpt

@@ -19,4 +19,4 @@ echo "POST-CLASS-GUARD\n";
 --EXPECTF--
 PRE-CLASS-GUARD
 
-Fatal error: ComposingClass and TestTrait define the same constant (Constant). However, the definition differs and is considered incompatible. Class was composed in %s on line %d
+Fatal error: ComposingClass and TestTrait define the same constant (Constant) in the composition of ComposingClass. However, the definition differs and is considered incompatible. Class was composed in %s on line %d

Zend/tests/traits/constant_010.phpt

@@ -19,4 +19,4 @@ echo "POST-CLASS-GUARD\n";
 --EXPECTF--
 PRE-CLASS-GUARD
 
-Fatal error: ComposingClass and TestTrait define the same constant (Constant). However, the definition differs and is considered incompatible. Class was composed in %s on line %d
+Fatal error: ComposingClass and TestTrait define the same constant (Constant) in the composition of ComposingClass. However, the definition differs and is considered incompatible. Class was composed in %s on line %d

Zend/tests/traits/constant_011.phpt

@@ -13,4 +13,4 @@ trait Trait2 {
 }
 ?>
 --EXPECTF--
-Fatal error: Trait2 and Trait1 define the same constant (Constant). However, the definition differs and is considered incompatible. Class was composed in %s on line %d
+Fatal error: Trait2 and Trait1 define the same constant (Constant) in the composition of Trait2. However, the definition differs and is considered incompatible. Class was composed in %s on line %d

Zend/tests/traits/constant_012.phpt

@@ -13,4 +13,4 @@ trait Trait2 {
 }
 ?>
 --EXPECTF--
-Fatal error: Trait2 and Trait1 define the same constant (Constant). However, the definition differs and is considered incompatible. Class was composed in %s on line %d
+Fatal error: Trait2 and Trait1 define the same constant (Constant) in the composition of Trait2. However, the definition differs and is considered incompatible. Class was composed in %s on line %d

Zend/tests/traits/constant_013.phpt

@@ -13,4 +13,4 @@ trait Trait2 {
 }
 ?>
 --EXPECTF--
-Fatal error: Trait2 and Trait1 define the same constant (Constant). However, the definition differs and is considered incompatible. Class was composed in %s on line %d
+Fatal error: Trait2 and Trait1 define the same constant (Constant) in the composition of Trait2. However, the definition differs and is considered incompatible. Class was composed in %s on line %d

Zend/tests/traits/constant_014.phpt

@@ -18,4 +18,4 @@ class DerivedClass extends ComposingClass {
 
 ?>
 --EXPECTF--
-Fatal error: DerivedClass::Constant cannot override final constant TestTrait::Constant in %s on line %d
+Fatal error: DerivedClass::Constant cannot override final constant ComposingClass::Constant in %s on line %d

Zend/zend_inheritance.c

@@ -1601,63 +1601,34 @@ static bool do_inherit_constant_check(
 
 	zend_class_constant *old_constant = Z_PTR_P(zv);
 
-	if (parent_constant->ce->ce_flags & ZEND_ACC_TRAIT) {
-		bool compatible = false;
-		uint32_t flags_mask = ZEND_ACC_PPP_MASK | ZEND_ACC_FINAL;
-
-		if (parent_constant->ce == old_constant->ce) {
-			return true;
-		}
-		if ((ZEND_CLASS_CONST_FLAGS(parent_constant) & flags_mask) == (ZEND_CLASS_CONST_FLAGS(old_constant) & flags_mask)) {
-			compatible = check_trait_property_or_constant_value_compatibility(ce, &parent_constant->value, &old_constant->value);
-		}
-		if (!compatible) {
-			if (old_constant->ce != ce) {
-				zend_error_noreturn(E_COMPILE_ERROR,
-			    "%s and %s define the same constant (%s) in the composition of %s. However, the definition differs and is considered incompatible. Class was composed",
-					ZSTR_VAL(old_constant->ce->name),
-					ZSTR_VAL(parent_constant->ce->name),
-					ZSTR_VAL(name),
-					ZSTR_VAL(ce->name));
-			} else {
-				zend_error_noreturn(E_COMPILE_ERROR,
-				    "%s and %s define the same constant (%s). However, the definition differs and is considered incompatible. Class was composed",
-					ZSTR_VAL(old_constant->ce->name),
-					ZSTR_VAL(parent_constant->ce->name),
-					ZSTR_VAL(name));
-			}
-		}
-		return compatible;
-	} else {
-		if (parent_constant->ce != old_constant->ce && (ZEND_CLASS_CONST_FLAGS(parent_constant) & ZEND_ACC_FINAL)) {
-			zend_error_noreturn(E_COMPILE_ERROR, "%s::%s cannot override final constant %s::%s",
-				ZSTR_VAL(old_constant->ce->name), ZSTR_VAL(name),
-				ZSTR_VAL(parent_constant->ce->name), ZSTR_VAL(name)
-			);
-		}
+	if (parent_constant->ce != old_constant->ce && (ZEND_CLASS_CONST_FLAGS(parent_constant) & ZEND_ACC_FINAL)) {
+		zend_error_noreturn(E_COMPILE_ERROR, "%s::%s cannot override final constant %s::%s",
+			ZSTR_VAL(old_constant->ce->name), ZSTR_VAL(name),
+			ZSTR_VAL(parent_constant->ce->name), ZSTR_VAL(name)
+		);
+	}
 
-		if (old_constant->ce != parent_constant->ce && old_constant->ce != ce) {
-			zend_error_noreturn(E_COMPILE_ERROR,
-			    "%s %s inherits both %s::%s and %s::%s, which is ambiguous",
-				zend_get_object_type_uc(ce),
-				ZSTR_VAL(ce->name),
-				ZSTR_VAL(old_constant->ce->name), ZSTR_VAL(name),
-				ZSTR_VAL(parent_constant->ce->name), ZSTR_VAL(name));
-		}
+	if (old_constant->ce != parent_constant->ce && old_constant->ce != ce) {
+		zend_error_noreturn(E_COMPILE_ERROR,
+			"%s %s inherits both %s::%s and %s::%s, which is ambiguous",
+			zend_get_object_type_uc(ce),
+			ZSTR_VAL(ce->name),
+			ZSTR_VAL(old_constant->ce->name), ZSTR_VAL(name),
+			ZSTR_VAL(parent_constant->ce->name), ZSTR_VAL(name));
 	}
 
 	return false;
 }
 /* }}} */
 
-static void do_inherit_trait_or_interface_constant(zend_string *name, zend_class_constant *c, zend_class_entry *ce, zend_class_entry *trait_or_interface) /* {{{ */
+static void do_inherit_interface_constant(zend_string *name, zend_class_constant *c, zend_class_entry *ce, zend_class_entry *iface) /* {{{ */
 {
 	if (do_inherit_constant_check(ce, c, name)) {
 		zend_class_constant *ct;
 		if (Z_TYPE(c->value) == IS_CONSTANT_AST) {
 			ce->ce_flags &= ~ZEND_ACC_CONSTANTS_UPDATED;
 			ce->ce_flags |= ZEND_ACC_HAS_AST_CONSTANTS;
-			if (trait_or_interface->ce_flags & ZEND_ACC_IMMUTABLE) {
+			if (iface->ce_flags & ZEND_ACC_IMMUTABLE) {
 				ct = zend_arena_alloc(&CG(arena), sizeof(zend_class_constant));
 				memcpy(ct, c, sizeof(zend_class_constant));
 				c = ct;
@@ -1668,19 +1639,6 @@ static void do_inherit_trait_or_interface_constant(zend_string *name, zend_class
 			ct = pemalloc(sizeof(zend_class_constant), 1);
 			memcpy(ct, c, sizeof(zend_class_constant));
 			c = ct;
-		} else if (trait_or_interface->ce_flags & ZEND_ACC_TRAIT) {
-			ct = emalloc(sizeof(zend_class_constant));
-			memcpy(ct, c, sizeof(zend_class_constant));
-			c = ct;
-		}
-
-		if (trait_or_interface->ce_flags & ZEND_ACC_TRAIT) {
-			c->ce = ce;
-			Z_TRY_ADDREF(c->value);
-			c->doc_comment = c->doc_comment ? zend_string_copy(c->doc_comment) : NULL;
-			if (c->attributes && (!(GC_FLAGS(c->attributes) & IS_ARRAY_IMMUTABLE))) {
-				GC_ADDREF(c->attributes);
-			}
 		}
 
 		zend_hash_update_ptr(&ce->constants_table, name, c);
@@ -1695,7 +1653,7 @@ static void do_interface_implementation(zend_class_entry *ce, zend_class_entry *
 	zend_class_constant *c;
 
 	ZEND_HASH_MAP_FOREACH_STR_KEY_PTR(&iface->constants_table, key, c) {
-        do_inherit_trait_or_interface_constant(key, c, ce, iface);
+		do_inherit_interface_constant(key, c, ce, iface);
 	} ZEND_HASH_FOREACH_END();
 
 	ZEND_HASH_MAP_FOREACH_STR_KEY_PTR(&iface->function_table, key, func) {
@@ -2179,7 +2137,7 @@ static void zend_do_traits_method_binding(zend_class_entry *ce, zend_class_entry
 }
 /* }}} */
 
-static zend_class_entry* find_first_definition(zend_class_entry *ce, zend_class_entry **traits, size_t current_trait, zend_string *prop_name, zend_class_entry *colliding_ce) /* {{{ */
+static zend_class_entry* find_first_property_definition(zend_class_entry *ce, zend_class_entry **traits, size_t current_trait, zend_string *prop_name, zend_class_entry *colliding_ce) /* {{{ */
 {
 	size_t i;
 
@@ -2196,6 +2154,85 @@ static zend_class_entry* find_first_definition(zend_class_entry *ce, zend_class_
 }
 /* }}} */
 
+static zend_class_entry* find_first_constant_definition(zend_class_entry *ce, zend_class_entry **traits, size_t current_trait, zend_string *constant_name, zend_class_entry *colliding_ce) /* {{{ */
+{
+	size_t i;
+
+	if (colliding_ce == ce) {
+		for (i = 0; i < current_trait; i++) {
+			if (traits[i]
+				&& zend_hash_exists(&traits[i]->constants_table, constant_name)) {
+				return traits[i];
+			}
+		}
+	}
+
+	return colliding_ce;
+}
+/* }}} */
+
+static bool do_trait_constant_check(
+	zend_class_entry *ce, zend_class_constant *trait_constant, zend_string *name, zend_class_entry **traits, size_t current_trait
+) {
+	bool compatible = false;
+	uint32_t flags_mask = ZEND_ACC_PPP_MASK | ZEND_ACC_FINAL;
+
+	zval *zv = zend_hash_find_known_hash(&ce->constants_table, name);
+	if (zv == NULL) {
+		return true;
+	}
+
+	zend_class_constant *old_constant = Z_PTR_P(zv);
+
+	if (trait_constant->ce == old_constant->ce) {
+		return true;
+	}
+	if ((ZEND_CLASS_CONST_FLAGS(trait_constant) & flags_mask) == (ZEND_CLASS_CONST_FLAGS(old_constant) & flags_mask)) {
+		compatible = check_trait_property_or_constant_value_compatibility(ce, &trait_constant->value, &old_constant->value);
+	}
+	if (!compatible) {
+		zend_error_noreturn(E_COMPILE_ERROR,
+			"%s and %s define the same constant (%s) in the composition of %s. However, the definition differs and is considered incompatible. Class was composed",
+			ZSTR_VAL(find_first_constant_definition(ce, traits, current_trait, name, old_constant->ce)->name),
+			ZSTR_VAL(trait_constant->ce->name),
+			ZSTR_VAL(name),
+			ZSTR_VAL(ce->name));
+	}
+	return compatible;
+}
+/* }}} */
+
+static void do_inherit_trait_constant(zend_string *name, zend_class_constant *c, zend_class_entry *ce, zend_class_entry **traits, size_t current_trait) /* {{{ */
+{
+	if (do_trait_constant_check(ce, c, name, traits, current_trait)) {
+		zend_class_constant *ct;
+		if (Z_TYPE(c->value) == IS_CONSTANT_AST) {
+			ce->ce_flags &= ~ZEND_ACC_CONSTANTS_UPDATED;
+			ce->ce_flags |= ZEND_ACC_HAS_AST_CONSTANTS;
+			if (traits[current_trait]->ce_flags & ZEND_ACC_IMMUTABLE) {
+				ct = zend_arena_alloc(&CG(arena), sizeof(zend_class_constant));
+				memcpy(ct, c, sizeof(zend_class_constant));
+				c = ct;
+				Z_CONSTANT_FLAGS(c->value) |= CONST_OWNED;
+			}
+		}
+
+		ct = emalloc(sizeof(zend_class_constant));
+		memcpy(ct, c, sizeof(zend_class_constant));
+		c = ct;
+
+		c->ce = ce;
+		Z_TRY_ADDREF(c->value);
+		c->doc_comment = c->doc_comment ? zend_string_copy(c->doc_comment) : NULL;
+		if (c->attributes && (!(GC_FLAGS(c->attributes) & IS_ARRAY_IMMUTABLE))) {
+			GC_ADDREF(c->attributes);
+		}
+
+		zend_hash_update_ptr(&ce->constants_table, name, c);
+	}
+}
+/* }}} */
+
 static void zend_do_traits_property_and_constant_binding(zend_class_entry *ce, zend_class_entry **traits) /* {{{ */
 {
 	size_t i;
@@ -2218,7 +2255,7 @@ static void zend_do_traits_property_and_constant_binding(zend_class_entry *ce, z
 			continue;
 		}
 		ZEND_HASH_MAP_FOREACH_STR_KEY_PTR(&traits[i]->constants_table, constant_name, constant) {
-			do_inherit_trait_or_interface_constant(constant_name, constant, ce, traits[i]);
+			do_inherit_trait_constant(constant_name, constant, ce, traits, i);
 		} ZEND_HASH_FOREACH_END();
 
 		ZEND_HASH_MAP_FOREACH_STR_KEY_PTR(&traits[i]->properties_info, prop_name, property_info) {
@@ -2254,7 +2291,7 @@ static void zend_do_traits_property_and_constant_binding(zend_class_entry *ce, z
 					if (!compatible) {
 						zend_error_noreturn(E_COMPILE_ERROR,
 							   "%s and %s define the same property ($%s) in the composition of %s. However, the definition differs and is considered incompatible. Class was composed",
-								ZSTR_VAL(find_first_definition(ce, traits, i, prop_name, colliding_prop->ce)->name),
+								ZSTR_VAL(find_first_property_definition(ce, traits, i, prop_name, colliding_prop->ce)->name),
 								ZSTR_VAL(property_info->ce->name),
 								ZSTR_VAL(prop_name),
 								ZSTR_VAL(ce->name));