gron Sat, 05 Nov 2011 01:46:40 +0000
Revision: http://svn.php.net/viewvc?view=revision&revision=318793
Log:
Fixed Bug #60217 (Requiring the same method from different traits)
- also added test to check for inconsistent abstract method definitions, they
need to be compatible
Bug: https://bugs.php.net/60217 (Assigned) imposing requirements in traits
Changed paths:
A php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60217a.phpt
A php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60217b.phpt
A php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60217c.phpt
U php/php-src/branches/PHP_5_4/Zend/zend_compile.c
A php/php-src/trunk/Zend/tests/traits/bug60217a.phpt
A php/php-src/trunk/Zend/tests/traits/bug60217b.phpt
A php/php-src/trunk/Zend/tests/traits/bug60217c.phpt
U php/php-src/trunk/Zend/zend_compile.c
Added: php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60217a.phpt
===================================================================
--- php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60217a.phpt (rev 0)
+++ php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60217a.phpt 2011-11-05 01:46:40 UTC (rev 318793)
@@ -0,0 +1,26 @@
+--TEST--
+Bug #60217 (Requiring the same method from different traits.)
+--FILE--
+<?php
+
+trait T1 {
+ public abstract function foo();
+}
+
+trait T2 {
+ public abstract function foo();
+}
+
+class C {
+ use T1, T2;
+
+ public function foo() {
+ echo "C::foo() works.\n";
+ }
+}
+
+$o = new C;
+$o->foo();
+
+--EXPECTF--
+C::foo() works.
Added: php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60217b.phpt
===================================================================
--- php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60217b.phpt (rev 0)
+++ php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60217b.phpt 2011-11-05 01:46:40 UTC (rev 318793)
@@ -0,0 +1,26 @@
+--TEST--
+Bug #60217 (Requiring the same method from different traits and abstract methods have to be compatible)
+--FILE--
+<?php
+
+trait TBroken1 {
+ public abstract function foo($a);
+}
+
+trait TBroken2 {
+ public abstract function foo($a, $b = 0);
+}
+
+class CBroken {
+ use TBroken1, TBroken2;
+
+ public function foo($a) {
+ echo 'FOO';
+ }
+}
+
+$o = new CBroken;
+$o->foo(1);
+
+--EXPECTF--
+Fatal error: Declaration of TBroken1::foo($a) must be compatible with TBroken2::foo($a, $b = 0) in %s on line %d
Added: php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60217c.phpt
===================================================================
--- php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60217c.phpt (rev 0)
+++ php/php-src/branches/PHP_5_4/Zend/tests/traits/bug60217c.phpt 2011-11-05 01:46:40 UTC (rev 318793)
@@ -0,0 +1,26 @@
+--TEST--
+Bug #60217 (Requiring the same method from different traits and abstract methods have to be compatible, in both directions.)
+--FILE--
+<?php
+
+trait TBroken1 {
+ public abstract function foo($a, $b = 0);
+}
+
+trait TBroken2 {
+ public abstract function foo($a);
+}
+
+class CBroken {
+ use TBroken1, TBroken2;
+
+ public function foo($a) {
+ echo 'FOO';
+ }
+}
+
+$o = new CBroken;
+$o->foo(1);
+
+--EXPECTF--
+Fatal error: Declaration of TBroken1::foo($a, $b = 0) must be compatible with TBroken2::foo($a) in %s on line %d
Modified: php/php-src/branches/PHP_5_4/Zend/zend_compile.c
===================================================================
--- php/php-src/branches/PHP_5_4/Zend/zend_compile.c 2011-11-04 22:58:44 UTC (rev 318792)
+++ php/php-src/branches/PHP_5_4/Zend/zend_compile.c 2011-11-05 01:46:40 UTC (rev 318793)
@@ -3617,7 +3617,19 @@
/* if it is an abstract method, there is no collision */
if (other_trait_fn->common.fn_flags & ZEND_ACC_ABSTRACT) {
/* Make sure they are compatible */
- do_inheritance_check_on_method(fn, other_trait_fn TSRMLS_CC);
+ if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) {
+ /* In case both are abstract, just check prototype, but need to do that in both directions */
+ if ( !zend_do_perform_implementation_check(fn, other_trait_fn TSRMLS_CC)
+ || !zend_do_perform_implementation_check(other_trait_fn, fn TSRMLS_CC)) {
+ zend_error(E_COMPILE_ERROR, "Declaration of %s must be compatible with %s", //ZEND_FN_SCOPE_NAME(fn), fn->common.function_name, //::%s()
+ zend_get_function_declaration(fn TSRMLS_CC),
+ zend_get_function_declaration(other_trait_fn TSRMLS_CC));
+ }
+ }
+ else {
+ /* otherwise, do the full check */
+ do_inheritance_check_on_method(fn, other_trait_fn TSRMLS_CC);
+ }
/* we can savely free and remove it from other table */
zend_function_dtor(other_trait_fn);
@@ -3626,7 +3638,8 @@
/* if it is not an abstract method, there is still no collision */
/* if fn is an abstract method */
if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) {
- /* Make sure they are compatible */
+ /* Make sure they are compatible.
+ Here, we already know other_trait_fn cannot be abstract, full check ok. */
do_inheritance_check_on_method(other_trait_fn, fn TSRMLS_CC);
/* just mark as solved, will be added if its own trait is processed */
@@ -3843,39 +3856,39 @@
zend_function* existing_fn = NULL;
zend_function fn_copy, *fn_copy_p;
zend_function* prototype = NULL; /* is used to determine the prototype according to the inheritance chain */
-
+
if (zend_hash_quick_find(&ce->function_table, hash_key->arKey, hash_key->nKeyLength, hash_key->h, (void**) &existing_fn) == FAILURE) {
add = 1; /* not found */
} else if (existing_fn->common.scope != ce) {
add = 1; /* or inherited from other class or interface */
}
-
+
if (add) {
zend_function* parent_function;
if (ce->parent && zend_hash_quick_find(&ce->parent->function_table, hash_key->arKey, hash_key->nKeyLength, hash_key->h, (void**) &parent_function) != FAILURE) {
prototype = parent_function; /* ->common.fn_flags |= ZEND_ACC_ABSTRACT; */
/* we got that method in the parent class, and are going to override it,
- except, if the trait is just asking to have an abstract method implemented. */
+ except, if the trait is just asking to have an abstract method implemented. */
if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) {
/* then we clean up an skip this method */
zend_function_dtor(fn);
return ZEND_HASH_APPLY_REMOVE;
}
}
-
+
fn->common.scope = ce;
fn->common.prototype = prototype;
-
+
if (prototype
- && (prototype->common.fn_flags & ZEND_ACC_IMPLEMENTED_ABSTRACT
- || prototype->common.fn_flags & ZEND_ACC_ABSTRACT)) {
- fn->common.fn_flags |= ZEND_ACC_IMPLEMENTED_ABSTRACT;
- } else if (fn->common.fn_flags & ZEND_ACC_IMPLEMENTED_ABSTRACT) {
- /* remove ZEND_ACC_IMPLEMENTED_ABSTRACT flag, think it shouldn't be copied to class */
- fn->common.fn_flags = fn->common.fn_flags - ZEND_ACC_IMPLEMENTED_ABSTRACT;
- }
-
+ && (prototype->common.fn_flags & ZEND_ACC_IMPLEMENTED_ABSTRACT
+ || prototype->common.fn_flags & ZEND_ACC_ABSTRACT)) {
+ fn->common.fn_flags |= ZEND_ACC_IMPLEMENTED_ABSTRACT;
+ } else if (fn->common.fn_flags & ZEND_ACC_IMPLEMENTED_ABSTRACT) {
+ /* remove ZEND_ACC_IMPLEMENTED_ABSTRACT flag, think it shouldn't be copied to class */
+ fn->common.fn_flags = fn->common.fn_flags - ZEND_ACC_IMPLEMENTED_ABSTRACT;
+ }
+
/* check whether the trait method fullfills the inheritance requirements */
if (prototype) {
do_inheritance_check_on_method(fn, prototype TSRMLS_CC);
@@ -3906,18 +3919,18 @@
}
fn_copy = *fn;
zend_traits_duplicate_function(&fn_copy, ce, estrdup(fn->common.function_name) TSRMLS_CC);
-
+
if (zend_hash_quick_update(&ce->function_table, hash_key->arKey, hash_key->nKeyLength, hash_key->h, &fn_copy, sizeof(zend_function), (void**)&fn_copy_p)==FAILURE) {
zend_error(E_COMPILE_ERROR, "Trait method %s has not been applied, because failure occured during updating class method table", hash_key->arKey);
}
-
+
zend_add_magic_methods(ce, hash_key->arKey, hash_key->nKeyLength, fn_copy_p TSRMLS_CC);
-
+
zend_function_dtor(fn);
} else {
zend_function_dtor(fn);
}
-
+
return ZEND_HASH_APPLY_REMOVE;
}
/* }}} */
Added: php/php-src/trunk/Zend/tests/traits/bug60217a.phpt
===================================================================
--- php/php-src/trunk/Zend/tests/traits/bug60217a.phpt (rev 0)
+++ php/php-src/trunk/Zend/tests/traits/bug60217a.phpt 2011-11-05 01:46:40 UTC (rev 318793)
@@ -0,0 +1,26 @@
+--TEST--
+Bug #60217 (Requiring the same method from different traits.)
+--FILE--
+<?php
+
+trait T1 {
+ public abstract function foo();
+}
+
+trait T2 {
+ public abstract function foo();
+}
+
+class C {
+ use T1, T2;
+
+ public function foo() {
+ echo "C::foo() works.\n";
+ }
+}
+
+$o = new C;
+$o->foo();
+
+--EXPECTF--
+C::foo() works.
Added: php/php-src/trunk/Zend/tests/traits/bug60217b.phpt
===================================================================
--- php/php-src/trunk/Zend/tests/traits/bug60217b.phpt (rev 0)
+++ php/php-src/trunk/Zend/tests/traits/bug60217b.phpt 2011-11-05 01:46:40 UTC (rev 318793)
@@ -0,0 +1,26 @@
+--TEST--
+Bug #60217 (Requiring the same method from different traits and abstract methods have to be compatible)
+--FILE--
+<?php
+
+trait TBroken1 {
+ public abstract function foo($a);
+}
+
+trait TBroken2 {
+ public abstract function foo($a, $b = 0);
+}
+
+class CBroken {
+ use TBroken1, TBroken2;
+
+ public function foo($a) {
+ echo 'FOO';
+ }
+}
+
+$o = new CBroken;
+$o->foo(1);
+
+--EXPECTF--
+Fatal error: Declaration of TBroken1::foo($a) must be compatible with TBroken2::foo($a, $b = 0) in %s on line %d
Added: php/php-src/trunk/Zend/tests/traits/bug60217c.phpt
===================================================================
--- php/php-src/trunk/Zend/tests/traits/bug60217c.phpt (rev 0)
+++ php/php-src/trunk/Zend/tests/traits/bug60217c.phpt 2011-11-05 01:46:40 UTC (rev 318793)
@@ -0,0 +1,26 @@
+--TEST--
+Bug #60217 (Requiring the same method from different traits and abstract methods have to be compatible, in both directions.)
+--FILE--
+<?php
+
+trait TBroken1 {
+ public abstract function foo($a, $b = 0);
+}
+
+trait TBroken2 {
+ public abstract function foo($a);
+}
+
+class CBroken {
+ use TBroken1, TBroken2;
+
+ public function foo($a) {
+ echo 'FOO';
+ }
+}
+
+$o = new CBroken;
+$o->foo(1);
+
+--EXPECTF--
+Fatal error: Declaration of TBroken1::foo($a, $b = 0) must be compatible with TBroken2::foo($a) in %s on line %d
Modified: php/php-src/trunk/Zend/zend_compile.c
===================================================================
--- php/php-src/trunk/Zend/zend_compile.c 2011-11-04 22:58:44 UTC (rev 318792)
+++ php/php-src/trunk/Zend/zend_compile.c 2011-11-05 01:46:40 UTC (rev 318793)
@@ -3617,7 +3617,19 @@
/* if it is an abstract method, there is no collision */
if (other_trait_fn->common.fn_flags & ZEND_ACC_ABSTRACT) {
/* Make sure they are compatible */
- do_inheritance_check_on_method(fn, other_trait_fn TSRMLS_CC);
+ if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) {
+ /* In case both are abstract, just check prototype, but need to do that in both directions */
+ if ( !zend_do_perform_implementation_check(fn, other_trait_fn TSRMLS_CC)
+ || !zend_do_perform_implementation_check(other_trait_fn, fn TSRMLS_CC)) {
+ zend_error(E_COMPILE_ERROR, "Declaration of %s must be compatible with %s", //ZEND_FN_SCOPE_NAME(fn), fn->common.function_name, //::%s()
+ zend_get_function_declaration(fn TSRMLS_CC),
+ zend_get_function_declaration(other_trait_fn TSRMLS_CC));
+ }
+ }
+ else {
+ /* otherwise, do the full check */
+ do_inheritance_check_on_method(fn, other_trait_fn TSRMLS_CC);
+ }
/* we can savely free and remove it from other table */
zend_function_dtor(other_trait_fn);
@@ -3626,7 +3638,8 @@
/* if it is not an abstract method, there is still no collision */
/* if fn is an abstract method */
if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) {
- /* Make sure they are compatible */
+ /* Make sure they are compatible.
+ Here, we already know other_trait_fn cannot be abstract, full check ok. */
do_inheritance_check_on_method(other_trait_fn, fn TSRMLS_CC);
/* just mark as solved, will be added if its own trait is processed */
@@ -3843,39 +3856,39 @@
zend_function* existing_fn = NULL;
zend_function fn_copy, *fn_copy_p;
zend_function* prototype = NULL; /* is used to determine the prototype according to the inheritance chain */
-
+
if (zend_hash_quick_find(&ce->function_table, hash_key->arKey, hash_key->nKeyLength, hash_key->h, (void**) &existing_fn) == FAILURE) {
add = 1; /* not found */
} else if (existing_fn->common.scope != ce) {
add = 1; /* or inherited from other class or interface */
}
-
+
if (add) {
zend_function* parent_function;
if (ce->parent && zend_hash_quick_find(&ce->parent->function_table, hash_key->arKey, hash_key->nKeyLength, hash_key->h, (void**) &parent_function) != FAILURE) {
prototype = parent_function; /* ->common.fn_flags |= ZEND_ACC_ABSTRACT; */
/* we got that method in the parent class, and are going to override it,
- except, if the trait is just asking to have an abstract method implemented. */
+ except, if the trait is just asking to have an abstract method implemented. */
if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) {
/* then we clean up an skip this method */
zend_function_dtor(fn);
return ZEND_HASH_APPLY_REMOVE;
}
}
-
+
fn->common.scope = ce;
fn->common.prototype = prototype;
-
+
if (prototype
- && (prototype->common.fn_flags & ZEND_ACC_IMPLEMENTED_ABSTRACT
- || prototype->common.fn_flags & ZEND_ACC_ABSTRACT)) {
- fn->common.fn_flags |= ZEND_ACC_IMPLEMENTED_ABSTRACT;
- } else if (fn->common.fn_flags & ZEND_ACC_IMPLEMENTED_ABSTRACT) {
- /* remove ZEND_ACC_IMPLEMENTED_ABSTRACT flag, think it shouldn't be copied to class */
- fn->common.fn_flags = fn->common.fn_flags - ZEND_ACC_IMPLEMENTED_ABSTRACT;
- }
-
+ && (prototype->common.fn_flags & ZEND_ACC_IMPLEMENTED_ABSTRACT
+ || prototype->common.fn_flags & ZEND_ACC_ABSTRACT)) {
+ fn->common.fn_flags |= ZEND_ACC_IMPLEMENTED_ABSTRACT;
+ } else if (fn->common.fn_flags & ZEND_ACC_IMPLEMENTED_ABSTRACT) {
+ /* remove ZEND_ACC_IMPLEMENTED_ABSTRACT flag, think it shouldn't be copied to class */
+ fn->common.fn_flags = fn->common.fn_flags - ZEND_ACC_IMPLEMENTED_ABSTRACT;
+ }
+
/* check whether the trait method fullfills the inheritance requirements */
if (prototype) {
do_inheritance_check_on_method(fn, prototype TSRMLS_CC);
@@ -3906,18 +3919,18 @@
}
fn_copy = *fn;
zend_traits_duplicate_function(&fn_copy, ce, estrdup(fn->common.function_name) TSRMLS_CC);
-
+
if (zend_hash_quick_update(&ce->function_table, hash_key->arKey, hash_key->nKeyLength, hash_key->h, &fn_copy, sizeof(zend_function), (void**)&fn_copy_p)==FAILURE) {
zend_error(E_COMPILE_ERROR, "Trait method %s has not been applied, because failure occured during updating class method table", hash_key->arKey);
}
-
+
zend_add_magic_methods(ce, hash_key->arKey, hash_key->nKeyLength, fn_copy_p TSRMLS_CC);
-
+
zend_function_dtor(fn);
} else {
zend_function_dtor(fn);
}
-
+
return ZEND_HASH_APPLY_REMOVE;
}
/* }}} */
--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
