]> granicus.if.org Git - php/commitdiff
Fixed Bug #55554 (Legacy constructors not handled properly) [TRAITS] [DOC]
authorStefan Marr <gron@php.net>
Sun, 9 Oct 2011 11:13:27 +0000 (11:13 +0000)
committerStefan Marr <gron@php.net>
Sun, 9 Oct 2011 11:13:27 +0000 (11:13 +0000)
# 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.

Zend/tests/traits/bug55554a.phpt [new file with mode: 0644]
Zend/tests/traits/bug55554b.phpt [new file with mode: 0644]
Zend/tests/traits/bug55554c.phpt [new file with mode: 0644]
Zend/tests/traits/bug55554d.phpt [new file with mode: 0644]
Zend/tests/traits/bug55554e.phpt [new file with mode: 0644]
Zend/tests/traits/bug55554f.phpt [new file with mode: 0644]
Zend/tests/traits/bug55554g.phpt [new file with mode: 0644]
Zend/tests/traits/language009.phpt
Zend/zend_compile.c

diff --git a/Zend/tests/traits/bug55554a.phpt b/Zend/tests/traits/bug55554a.phpt
new file mode 100644 (file)
index 0000000..b92a81f
--- /dev/null
@@ -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 (file)
index 0000000..bf40e89
--- /dev/null
@@ -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 (file)
index 0000000..466b3f1
--- /dev/null
@@ -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 (file)
index 0000000..10f2e29
--- /dev/null
@@ -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 (file)
index 0000000..5db508f
--- /dev/null
@@ -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 (file)
index 0000000..34b327d
--- /dev/null
@@ -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 (file)
index 0000000..22d2696
--- /dev/null
@@ -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
index fdb8a63d2e9859d0f88b7820e389c655c0bf3446..e55c8d84aa9538be162927af726ffb317c9bf235 100644 (file)
@@ -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();
 
 ?>
index 018b62062679e1d093577dd1d37260dae9eee7a6..f8fe4ef17693ad6d2c3415fb96b329677b4fd09c 100644 (file)
@@ -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 {