Fixed Bug #55554 (Legacy constructors not handled properly) [TRAITS] [DOC]

# The handling of legacy constructors defined by traits was corrected.
# They are now properly registered and used on instantiation.
# The situation for conflicting legacy and __construct constructors is
# mostly identical. If they are defined in the class, they override conflicts
# and do not collide. However, in case different styles are mixed, between
# class and trait definition, we assume a programmer's mistake and report
# a collision.
#
# BTW: +1 for all the fixed tests! `make test` is fun again.

9 files changed, 292 insertions, 21 deletions

diff --git a/Zend/tests/traits/bug55554a.phpt b/Zend/tests/traits/bug55554a.phpt
new file mode 100644
index 0000000000..b92a81f54f
--- /dev/null
+++ b/Zend/tests/traits/bug55554a.phpt
@@ -0,0 +1,34 @@
+--TEST--
+Bug #55137 (Legacy constructor not registered for class)
+--FILE--
+<?php
+
+// All constructors should be registered as such
+
+trait TConstructor {
+    public function constructor() {
+        echo "ctor executed\n";
+    }
+}
+
+class NewConstructor {
+	use TConstructor {
+	    constructor as __construct;
+	}
+}
+
+class LegacyConstructor {
+    use TConstructor {
+        constructor as LegacyConstructor;
+    }
+}
+
+echo "New constructor: ";
+$o = new NewConstructor;
+
+echo "Legacy constructor: ";
+$o = new LegacyConstructor;
+
+--EXPECT--
+New constructor: ctor executed
+Legacy constructor: ctor executed
diff --git a/Zend/tests/traits/bug55554b.phpt b/Zend/tests/traits/bug55554b.phpt
new file mode 100644
index 0000000000..bf40e89a08
--- /dev/null
+++ b/Zend/tests/traits/bug55554b.phpt
@@ -0,0 +1,56 @@
+--TEST--
+Bug #55137 (Legacy constructor not registered for class)
+--FILE--
+<?php
+
+trait TConstructor {
+    public function foo() {
+        echo "foo executed\n";
+    }
+    public function bar() {
+        echo "bar executed\n";
+    }
+}
+
+class OverridingIsSilent1 {
+    use TConstructor {
+	    foo as __construct;
+	}
+	
+	public function __construct() {
+	    echo "OverridingIsSilent1 __construct\n";
+	}
+}
+
+$o = new OverridingIsSilent1;
+
+class OverridingIsSilent2 {
+    use TConstructor {
+	    foo as OverridingIsSilent2;
+	}
+	
+	public function OverridingIsSilent2() {
+	    echo "OverridingIsSilent2 OverridingIsSilent2\n";
+	}
+}
+
+$o = new OverridingIsSilent2;
+
+class ReportCollision {
+	use TConstructor {
+	    bar as ReportCollision;
+	    foo as __construct;
+	}
+}
+
+
+echo "ReportCollision: ";
+$o = new ReportCollision;
+
+
+--EXPECTF--
+OverridingIsSilent1 __construct
+OverridingIsSilent2 OverridingIsSilent2
+
+Fatal error: ReportCollision has colliding constructor definitions coming from traits in %s on line %d
+
diff --git a/Zend/tests/traits/bug55554c.phpt b/Zend/tests/traits/bug55554c.phpt
new file mode 100644
index 0000000000..466b3f1c08
--- /dev/null
+++ b/Zend/tests/traits/bug55554c.phpt
@@ -0,0 +1,46 @@
+--TEST--
+Bug #55137 (Legacy constructor not registered for class)
+--FILE--
+<?php
+
+// Test that the behavior is consistent with the existing handling of new
+// and legacy constructors.
+// Here, the traits conflicts are overridden by local definitions,
+// and the two constructor definitions do not directly collide in that case.
+
+trait TC1 {
+    public function __construct() {
+        echo "TC1 executed\n";
+    }
+    public function ReportCollision() {
+        echo "TC1 executed\n";
+    }
+}
+
+trait TC2 {
+    public function __construct() {
+        echo "TC2 executed\n";
+    }
+    public function ReportCollision() {
+        echo "TC1 executed\n";
+    }
+}
+
+class ReportCollision {
+	use TC1, TC2;
+	
+	public function __construct() {
+        echo "New constructor executed\n";
+    }
+    public function ReportCollision() {
+        echo "Legacy constructor executed\n";
+    }
+}
+
+
+echo "ReportCollision: ";
+$o = new ReportCollision;
+
+
+--EXPECTF--
+ReportCollision: New constructor executed
diff --git a/Zend/tests/traits/bug55554d.phpt b/Zend/tests/traits/bug55554d.phpt
new file mode 100644
index 0000000000..10f2e2952d
--- /dev/null
+++ b/Zend/tests/traits/bug55554d.phpt
@@ -0,0 +1,32 @@
+--TEST--
+Bug #55137 (Legacy constructor not registered for class)
+--FILE--
+<?php
+
+// Test mixed constructors from different traits, we are more strict about
+// these cases, since that can lead to un-expected behavior.
+// It is not consistent with the normal constructor handling, but
+// here we have a chance to be more strict for the new traits.
+
+trait TNew {
+    public function __construct() {
+        echo "TNew executed\n";
+    }
+}
+
+trait TLegacy {
+    public function ReportCollision() {
+        echo "ReportCollision executed\n";
+    }
+}
+
+class ReportCollision {
+    use TNew, TLegacy;
+}
+
+$o = new ReportCollision;
+
+--EXPECTF--
+
+Fatal error: ReportCollision has colliding constructor definitions coming from traits in %s on line %d
+
diff --git a/Zend/tests/traits/bug55554e.phpt b/Zend/tests/traits/bug55554e.phpt
new file mode 100644
index 0000000000..5db508fc85
--- /dev/null
+++ b/Zend/tests/traits/bug55554e.phpt
@@ -0,0 +1,30 @@
+--TEST--
+Bug #55137 (Legacy constructor not registered for class)
+--FILE--
+<?php
+
+// Ensuring that the collision still occurs as expected.
+
+trait TC1 {
+    public function ReportCollision() {
+        echo "TC1 executed\n";
+    }
+}
+
+trait TC2 {
+    public function ReportCollision() {
+        echo "TC1 executed\n";
+    }
+}
+
+class ReportCollision {
+	use TC1, TC2;
+}
+
+
+echo "ReportCollision: ";
+$o = new ReportCollision;
+
+
+--EXPECTF--
+Fatal error: Trait method ReportCollision has not been applied, because there are collisions with other trait methods on ReportCollision in %s on line %d
\ No newline at end of file
diff --git a/Zend/tests/traits/bug55554f.phpt b/Zend/tests/traits/bug55554f.phpt
new file mode 100644
index 0000000000..34b327d305
--- /dev/null
+++ b/Zend/tests/traits/bug55554f.phpt
@@ -0,0 +1,29 @@
+--TEST--
+Bug #55137 (Legacy constructor not registered for class)
+--FILE--
+<?php
+
+// Ensuring that inconsistent constructor use results in an error to avoid
+// problems creeping in.
+
+trait TNew {
+    public function __construct() {
+        echo "TNew executed\n";
+    }
+}
+
+class ReportCollision {
+    use TNew;
+	
+	public function ReportCollision() {
+	    echo "ReportCollision executed\n";
+	}
+}
+
+
+echo "ReportCollision: ";
+$o = new ReportCollision;
+
+
+--EXPECTF--
+Fatal error: ReportCollision has colliding constructor definitions coming from traits in %s on line %d
\ No newline at end of file
diff --git a/Zend/tests/traits/bug55554g.phpt b/Zend/tests/traits/bug55554g.phpt
new file mode 100644
index 0000000000..22d269603e
--- /dev/null
+++ b/Zend/tests/traits/bug55554g.phpt
@@ -0,0 +1,29 @@
+--TEST--
+Bug #55137 (Legacy constructor not registered for class)
+--FILE--
+<?php
+
+// Ensuring that inconsistent constructor use results in an error to avoid
+// problems creeping in.
+
+trait TLegacy {
+    public function ReportCollision() {
+        echo "TLegacy executed\n";
+    }
+}
+
+class ReportCollision {
+    use TLegacy;
+	
+	public function __construct() {
+	    echo "ReportCollision executed\n";
+	}
+}
+
+
+echo "ReportCollision: ";
+$o = new ReportCollision;
+
+
+--EXPECTF--
+Fatal error: ReportCollision has colliding constructor definitions coming from traits in %s on line %d
\ No newline at end of file
diff --git a/Zend/tests/traits/language009.phpt b/Zend/tests/traits/language009.phpt
index fdb8a63d2e..e55c8d84aa 100644
--- a/Zend/tests/traits/language009.phpt
+++ b/Zend/tests/traits/language009.phpt
@@ -22,13 +22,13 @@ trait C {
    }
 }
 
-class Foo {
+class MyClass {
     use C, A, B {
 		B::foo insteadof A, C; 
 	}
 }
 
-$t = new Foo;
+$t = new MyClass;
 $t->foo();
 
 ?>
diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index 018b620626..f8fe4ef176 100644
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -3654,21 +3654,6 @@ static int zend_traits_merge_functions(zend_function *fn TSRMLS_DC, int num_args
 }
 /* }}} */
 
-#define IS_EQUAL(mname, mname_len, str) \
-      strncmp(mname, str, mname_len)
-
-#define _ADD_MAGIC_METHOD(ce, mname, mname_len, fe) { \
-	if (     !IS_EQUAL(mname, mname_len, ZEND_CLONE_FUNC_NAME))       { (ce)->clone       = (fe); (fe)->common.fn_flags |= ZEND_ACC_CLONE; } \
-	else if (!IS_EQUAL(mname, mname_len, ZEND_CONSTRUCTOR_FUNC_NAME)) { (ce)->constructor = (fe); (fe)->common.fn_flags |= ZEND_ACC_CTOR; } \
-	else if (!IS_EQUAL(mname, mname_len, ZEND_DESTRUCTOR_FUNC_NAME))  { (ce)->destructor  = (fe); (fe)->common.fn_flags |= ZEND_ACC_DTOR; } \
-	else if (!IS_EQUAL(mname, mname_len, ZEND_GET_FUNC_NAME))        (ce)->__get          = (fe); \
-	else if (!IS_EQUAL(mname, mname_len, ZEND_SET_FUNC_NAME))        (ce)->__set          = (fe); \
-	else if (!IS_EQUAL(mname, mname_len, ZEND_CALL_FUNC_NAME))       (ce)->__call         = (fe); \
-	else if (!IS_EQUAL(mname, mname_len, ZEND_UNSET_FUNC_NAME))      (ce)->__unset        = (fe); \
-	else if (!IS_EQUAL(mname, mname_len, ZEND_ISSET_FUNC_NAME))      (ce)->__isset        = (fe); \
-	else if (!IS_EQUAL(mname, mname_len, ZEND_CALLSTATIC_FUNC_NAME)) (ce)->__callstatic   = (fe); \
-	else if (!IS_EQUAL(mname, mname_len, ZEND_TOSTRING_FUNC_NAME))   (ce)->__tostring     = (fe); \
-}
 
 /* {{{ Originates from php_runkit_function_copy_ctor
 	Duplicate structures in an op_array where necessary to make an outright duplicate */
@@ -3790,7 +3775,39 @@ static void zend_traits_duplicate_function(zend_function *fe, zend_class_entry *
 	fe->op_array.brk_cont_array = (zend_brk_cont_element*)estrndup((char*)fe->op_array.brk_cont_array, sizeof(zend_brk_cont_element) * fe->op_array.last_brk_cont);
   
 }
-/* }}}} */
+/* }}} */
+
+static void zend_add_magic_methods(zend_class_entry* ce, const char* mname, uint mname_len, zend_function* fe TSRMLS_DC) {
+	if (     !strncmp(mname, ZEND_CLONE_FUNC_NAME,       mname_len)) {	ce->clone = fe; fe->common.fn_flags |= ZEND_ACC_CLONE; }
+	else if (!strncmp(mname, ZEND_CONSTRUCTOR_FUNC_NAME, mname_len)) {
+		if (ce->constructor) {
+			zend_error(E_COMPILE_ERROR, "%s has colliding constructor definitions coming from traits", ce->name);
+		}
+		ce->constructor = fe; fe->common.fn_flags |= ZEND_ACC_CTOR; 
+	}
+	else if (!strncmp(mname, ZEND_DESTRUCTOR_FUNC_NAME,  mname_len)) {	ce->destructor  = fe; fe->common.fn_flags |= ZEND_ACC_DTOR; }
+	else if (!strncmp(mname, ZEND_GET_FUNC_NAME,         mname_len)) ce->__get       = fe;
+	else if (!strncmp(mname, ZEND_SET_FUNC_NAME,         mname_len)) ce->__set       = fe;
+	else if (!strncmp(mname, ZEND_CALL_FUNC_NAME,        mname_len)) ce->__call      = fe;
+	else if (!strncmp(mname, ZEND_UNSET_FUNC_NAME,       mname_len)) ce->__unset     = fe;
+	else if (!strncmp(mname, ZEND_ISSET_FUNC_NAME,       mname_len)) ce->__isset     = fe;
+	else if (!strncmp(mname, ZEND_CALLSTATIC_FUNC_NAME,  mname_len)) ce->__callstatic= fe;
+	else if (!strncmp(mname, ZEND_TOSTRING_FUNC_NAME,    mname_len)) ce->__tostring	 = fe;
+	else if (ce->name_length + 1 == mname_len) {
+		char *lowercase_name = emalloc(ce->name_length + 1);
+		zend_str_tolower_copy(lowercase_name, ce->name, ce->name_length);
+		lowercase_name = (char*)zend_new_interned_string(lowercase_name, ce->name_length + 1, 1 TSRMLS_CC);
+		if (!memcmp(mname, lowercase_name, mname_len)) {
+			if (ce->constructor) {
+				zend_error(E_COMPILE_ERROR, "%s has colliding constructor definitions coming from traits", ce->name);
+			}
+			ce->constructor = fe;
+			fe->common.fn_flags |= ZEND_ACC_CTOR;
+		}
+		str_efree(lowercase_name);
+	}
+}
+
 
 static int zend_traits_merge_functions_to_class(zend_function *fn TSRMLS_DC, int num_args, va_list args, zend_hash_key *hash_key) /* {{{ */
 {
@@ -3858,9 +3875,7 @@ static int zend_traits_merge_functions_to_class(zend_function *fn TSRMLS_DC, int
 			zend_error(E_COMPILE_ERROR, "Trait method %s has not been applied, because failure occured during updating class method table", hash_key->arKey);
 		}
 
-		_ADD_MAGIC_METHOD(ce, hash_key->arKey, hash_key->nKeyLength, fn_copy_p);
-		/* it could be necessary to update child classes as well */
-		/* zend_hash_apply_with_arguments(EG(class_table) TSRMLS_CC, (apply_func_args_t)php_runkit_update_children_methods, 5, dce, dce, &dfe, dfunc, dfunc_len); */
+		zend_add_magic_methods(ce, hash_key->arKey, hash_key->nKeyLength, fn_copy_p TSRMLS_CC);
 
 		zend_function_dtor(fn);
 	} else {