Fixed Bug #60717 (Order of traits in use statement can cause a fatal error)

# Compatibility is now correctly checked in both directions.
# Introduced helper method for the test.

diff --git a/NEWS b/NEWS
index 30e74a5695c6bd76f24f1674ce6c2877d31d95b0..2151b8e61defe9707933329ffb225f2e25ebf53d 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -14,6 +14,8 @@ PHP                                                                        NEWS
 - Core:
   . Fixed bug #60573 (type hinting with "self" keyword causes weird errors).
     (Laruence)
+  . Fixed bug #60717 (Order of traits in use statement can cause a fatal
+    error). (Stefan)
   . Fixed bug #60801 (strpbrk() mishandles NUL byte). (Adam)
   . Fixed bug #60978 (exit code incorrect). (Laruence)
   . Fixed bug #61000 (Exceeding max nesting level doesn't delete numerical 

diff --git a/Zend/tests/traits/bug60717.phpt b/Zend/tests/traits/bug60717.phpt
new file mode 100644 (file)
index 0000000..bf3adb1
--- /dev/null
+++ b/Zend/tests/traits/bug60717.phpt
@@ -0,0 +1,73 @@
+--TEST--
+Bug #60717 (Order of traits in use statement can cause unexpected unresolved abstract method)
+--FILE--
+<?php
+
+namespace HTML
+{
+       interface Helper
+       {
+               function text($text);
+               function attributes(array $attributes = null);
+               function textArea(array $attributes = null, $value);
+       }
+
+       trait TextUTF8
+       {
+               function text($text) {}
+       }
+
+       trait TextArea
+       {
+               function textArea(array $attributes = null, $value) {}
+               abstract function attributes(array $attributes = null);
+               abstract function text($text);
+       }
+
+       trait HTMLAttributes
+       {
+               function attributes(array $attributes = null) { }
+               abstract function text($text);
+       }
+
+       class HTMLHelper implements Helper
+       {
+               use TextArea, HTMLAttributes, TextUTF8;
+       }
+       
+       class HTMLHelper2 implements Helper
+       {
+               use TextArea, TextUTF8, HTMLAttributes;
+       }
+       
+       class HTMLHelper3 implements Helper
+       {
+               use HTMLAttributes, TextArea, TextUTF8;
+       }
+
+       class HTMLHelper4 implements Helper
+       {
+               use HTMLAttributes, TextUTF8, TextArea;
+       }
+       
+       class HTMLHelper5 implements Helper
+       {
+               use TextUTF8, TextArea, HTMLAttributes;
+       }
+       
+       class HTMLHelper6 implements Helper
+       {
+               use TextUTF8, HTMLAttributes, TextArea;
+       }       
+
+       $o = new HTMLHelper;
+    $o = new HTMLHelper2;
+    $o = new HTMLHelper3;
+    $o = new HTMLHelper4;
+    $o = new HTMLHelper5;
+    $o = new HTMLHelper6;
+    echo 'Done';
+}
+
+--EXPECT--
+Done

diff --git a/Zend/tests/traits/bugs/abstract-methods06.phpt b/Zend/tests/traits/bugs/abstract-methods06.phpt
index fdcd81696158c8866af028f3c41d8fe7bc88cb7f..28ed672725bb53de92271f71579e8d68c0dcbe78 100644 (file)
--- a/Zend/tests/traits/bugs/abstract-methods06.phpt
+++ b/Zend/tests/traits/bugs/abstract-methods06.phpt
@@ -23,4 +23,4 @@ class TraitsTest1 {
 
 ?>
 --EXPECTF--    
-Fatal error: Declaration of THelloB::hello() must be compatible with THelloA::hello($a) in %s on line %d
\ No newline at end of file
+Fatal error: Declaration of THelloA::hello($a) must be compatible with THelloB::hello() in %s on line %d
\ No newline at end of file

diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
index 4c2a740841ebd5ae2cfcabb06a946edc2c0d3fa8..a7b130d8fdb0315e3d3a3490bb00b4bf5b3b94db 100644 (file)
--- a/Zend/zend_compile.c
+++ b/Zend/zend_compile.c
@@ -3623,6 +3623,18 @@ ZEND_API void zend_do_implement_trait(zend_class_entry *ce, zend_class_entry *tr
 }
 /* }}} */
 
+static zend_bool zend_traits_method_compatibility_check(zend_function *fn, zend_function *other_fn TSRMLS_DC) /* {{{ */
+{
+       zend_uint    fn_flags = fn->common.scope->ce_flags;
+       zend_uint other_flags = other_fn->common.scope->ce_flags;
+       
+       return zend_do_perform_implementation_check(fn, other_fn TSRMLS_CC)
+               && zend_do_perform_implementation_check(other_fn, fn TSRMLS_CC)
+               && ((fn_flags & ZEND_ACC_FINAL) == (other_flags & ZEND_ACC_FINAL))   /* equal final qualifier */
+               && ((fn_flags & ZEND_ACC_STATIC)== (other_flags & ZEND_ACC_STATIC)); /* equal static qualifier */
+}
+/* }}} */
+
 static int zend_traits_merge_functions(zend_function *fn TSRMLS_DC, int num_args, va_list args, zend_hash_key *hash_key) /* {{{ */
 {
        size_t current;
@@ -3645,22 +3657,16 @@ static int zend_traits_merge_functions(zend_function *fn TSRMLS_DC, int num_args
                if (i == current) {
                        continue; /* just skip this, cause its the table this function is applied on */
                }
-
+               
                if (zend_hash_quick_find(function_tables[i], hash_key->arKey, hash_key->nKeyLength, hash_key->h, (void **)&other_trait_fn) == SUCCESS) {
                        /* 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 */
-                               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);
+                               /* In case both are abstract, just check prototype, but need to do that in both directions */
+                               if (!zend_traits_method_compatibility_check(fn, other_trait_fn TSRMLS_CC)) {
+                                       zend_error(E_COMPILE_ERROR, "Declaration of %s must be compatible with %s",
+                                                                                               zend_get_function_declaration(fn TSRMLS_CC),
+                                                                                               zend_get_function_declaration(other_trait_fn TSRMLS_CC));
                                }
                                
                                /* we can savely free and remove it from other table */
@@ -3672,7 +3678,11 @@ static int zend_traits_merge_functions(zend_function *fn TSRMLS_DC, int num_args
                                if (fn->common.fn_flags & ZEND_ACC_ABSTRACT) {
                                        /* 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);
+                                       if (!zend_traits_method_compatibility_check(fn, other_trait_fn TSRMLS_CC)) {
+                                               zend_error(E_COMPILE_ERROR, "Declaration of %s must be compatible with %s",
+                                                                                                       zend_get_function_declaration(fn TSRMLS_CC),
+                                                                                                       zend_get_function_declaration(other_trait_fn TSRMLS_CC));
+                                       }
                                        
                                        /* just mark as solved, will be added if its own trait is processed */
                                        abstract_solved = 1;