From: Stefan Marr Date: Mon, 20 Dec 2010 00:52:40 +0000 (+0000) Subject: Added strict handling of properties in traits. X-Git-Tag: php-5.4.0alpha1~191^2~482 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7357867bacdf1c10f6abe14e875ada7911b9629f;p=php Added strict handling of properties in traits. # This is the first attempt to implement the properties as discussed on the mailinglist. # RFC is not updated to reflect this changes, yet. --- diff --git a/Zend/tests/traits/property001.phpt b/Zend/tests/traits/property001.phpt new file mode 100644 index 0000000000..7efd02ecc7 --- /dev/null +++ b/Zend/tests/traits/property001.phpt @@ -0,0 +1,40 @@ +--TEST-- +Potentially conflicting properties should result in a strict notice. Property use is discorage for traits that are supposed to enable maintainable code reuse. Accessor methods are the language supported idiom for this. +--FILE-- + +--EXPECTF-- +PRE-CLASS-GUARD-TraitsTest +PRE-CLASS-GUARD-TraitsTest2 + +Strict Standards: THello1 and THello2 define the same property ($foo) in the composition of TraitsTest2. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %s on line %d +bool(true) +bool(true) \ No newline at end of file diff --git a/Zend/tests/traits/property002.phpt b/Zend/tests/traits/property002.phpt new file mode 100644 index 0000000000..27361e04d2 --- /dev/null +++ b/Zend/tests/traits/property002.phpt @@ -0,0 +1,32 @@ +--TEST-- +Non-conflicting properties should work just fine. +--FILE-- +hello . ' ' . $this->world; + } +} + +var_dump(property_exists('TraitsTest', 'hello')); +var_dump(property_exists('TraitsTest', 'world')); + +$t = new TraitsTest; +$t->test(); +?> +--EXPECTF-- +bool(true) +bool(true) +hello World! \ No newline at end of file diff --git a/Zend/tests/traits/property003.phpt b/Zend/tests/traits/property003.phpt new file mode 100644 index 0000000000..b4f0105d20 --- /dev/null +++ b/Zend/tests/traits/property003.phpt @@ -0,0 +1,30 @@ +--TEST-- +Conflicting properties with different visibility modifiers should result in a fatal error, since this indicates that the code is incompatible. +--FILE-- +hello = "foo"; +?> +--EXPECTF-- +PRE-CLASS-GUARD + +Fatal error: THello1 and THello2 define the same property ($hello) in the composition of TraitsTest. However, the definition differs and is considered incompatible. Class was composed in %s on line %d \ No newline at end of file diff --git a/Zend/tests/traits/property004.phpt b/Zend/tests/traits/property004.phpt new file mode 100644 index 0000000000..393b492b7f --- /dev/null +++ b/Zend/tests/traits/property004.phpt @@ -0,0 +1,30 @@ +--TEST-- +Conflicting properties with different initial values are considered incompatible. +--FILE-- +hello; + } +} + +$t = new TraitsTest; +?> +--EXPECTF-- +PRE-CLASS-GUARD + +Fatal error: THello1 and THello2 define the same property ($hello) in the composition of TraitsTest. However, the definition differs and is considered incompatible. Class was composed in %s on line %d \ No newline at end of file diff --git a/Zend/tests/traits/property005.phpt b/Zend/tests/traits/property005.phpt new file mode 100644 index 0000000000..3c80a1f26a --- /dev/null +++ b/Zend/tests/traits/property005.phpt @@ -0,0 +1,50 @@ +--TEST-- +The same rules are applied for properties that are defined in the class hierarchy. Thus, if the properties are compatible, a notice is issued, if not a fatal error occures. +--FILE-- +hello = "foo"; +?> +--EXPECTF-- +PRE-CLASS-GUARD + +Strict Standards: Base and THello1 define the same property ($hello) in the composition of NoticeForBase. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %s on line %d +POST-CLASS-GUARD + +Strict Standards: Notice and THello1 define the same property ($hello) in the composition of Notice. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed in %s on line %d +POST-CLASS-GUARD2 + +Fatal error: TraitsTest and THello1 define the same property ($hello) in the composition of TraitsTest. However, the definition differs and is considered incompatible. Class was composed in %s on line %d diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 9b928437b0..d189d60899 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -3804,59 +3804,49 @@ static void zend_traits_compile_exclude_table(HashTable* exclude_table, zend_tra } } -ZEND_API void zend_do_bind_traits(zend_class_entry *ce TSRMLS_DC) /* {{{ */ +static void zend_do_traits_method_binding(zend_class_entry *ce TSRMLS_DC) /* {{{ */ { HashTable** function_tables; HashTable* resulting_table; HashTable exclude_table; size_t i; - if (ce->num_traits <= 0) { - return; - } - -/* zend_error(E_NOTICE, "Do bind Traits on %v with %d traits.\n Class has already %d methods.\n", - ce->name.s, ce->num_traits, ce->function_table.nNumOfElements); */ - - /* complete initialization of trait strutures in ce */ - zend_traits_init_trait_structures(ce TSRMLS_CC); - /* prepare copies of trait function tables for combination */ function_tables = malloc(sizeof(HashTable*) * ce->num_traits); resulting_table = (HashTable *) malloc(sizeof(HashTable)); zend_hash_init_ex(resulting_table, 10, /* TODO: revisit this start size, may be its not optimal */ - /* NULL, ZEND_FUNCTION_DTOR, 0, 0); */ - NULL, NULL, 0, 0); - + /* NULL, ZEND_FUNCTION_DTOR, 0, 0); */ + NULL, NULL, 0, 0); + for (i = 0; i < ce->num_traits; i++) { function_tables[i] = (HashTable *) malloc(sizeof(HashTable)); - zend_hash_init_ex(function_tables[i], ce->traits[i]->function_table.nNumOfElements, + zend_hash_init_ex(function_tables[i], ce->traits[i]->function_table.nNumOfElements, /* NULL, ZEND_FUNCTION_DTOR, 0, 0); */ NULL, NULL, 0, 0); - + zend_hash_init_ex(&exclude_table, 2, /* TODO: revisit this start size, may be its not optimal */ - NULL, NULL, 0, 0); + NULL, NULL, 0, 0); zend_traits_compile_exclude_table(&exclude_table, ce->trait_precedences, ce->traits[i]); - + /* copies functions, applies defined aliasing, and excludes unused trait methods */ zend_traits_copy_trait_function_table(function_tables[i], &ce->traits[i]->function_table, ce->trait_aliases, &exclude_table TSRMLS_CC); zend_hash_graceful_destroy(&exclude_table); } - + /* now merge trait methods */ for (i = 0; i < ce->num_traits; i++) { zend_hash_apply_with_arguments(function_tables[i] TSRMLS_CC, (apply_func_args_t)zend_traits_merge_functions, 5, /* 5 is number of args for apply_func */ - i, ce->num_traits, resulting_table, function_tables, ce); + i, ce->num_traits, resulting_table, function_tables, ce); } - - /* now the resulting_table contains all trait methods we would have to - add to the class - in the following step the methods are inserted into the method table - if there is already a method with the same name it is replaced iff ce != fn.scope - --> all inherited methods are overridden, methods defined in the class are leaved - untouched */ + + /* now the resulting_table contains all trait methods we would have to + add to the class + in the following step the methods are inserted into the method table + if there is already a method with the same name it is replaced iff ce != fn.scope + --> all inherited methods are overridden, methods defined in the class are left + untouched */ zend_hash_apply_with_arguments(resulting_table TSRMLS_CC, (apply_func_args_t)zend_traits_merge_functions_to_class, 1, ce); - + /* free temporary function tables */ for (i = 0; i < ce->num_traits; i++) { /* zend_hash_destroy(function_tables[i]); */ @@ -3869,8 +3859,145 @@ ZEND_API void zend_do_bind_traits(zend_class_entry *ce TSRMLS_DC) /* {{{ */ /* zend_hash_destroy(resulting_table); */ zend_hash_graceful_destroy(resulting_table); free(resulting_table); +} + +static zend_class_entry* find_first_definition(zend_class_entry *ce, size_t current_trait, char* prop_name, int prop_name_length, zend_class_entry *coliding_ce) /* {{{ */ +{ + size_t i; + zend_property_info *coliding_prop; + for (i = 0; (i < current_trait) && (i < ce->num_traits); i++) { + if (zend_hash_find(&ce->traits[i]->properties_info, prop_name, prop_name_length+1, (void **) &coliding_prop) == SUCCESS) { + return ce->traits[i]; + } + } + + return coliding_ce; +} + +static void zend_do_traits_property_binding(zend_class_entry *ce TSRMLS_DC) /* {{{ */ +{ + //HashTable* resulting_table; + size_t i; + zend_property_info *property_info; + zend_property_info *coliding_prop; + zval compare_result; + char* prop_name; + int prop_name_length; + + char* class_name_unused; + bool prop_found; + bool not_compatible; + zval* prop_value; + + + /* in the following steps the properties are inserted into the property table + for that, a very strict approach is applied: + - check for compatibility, if not compatible with any property in class -> fatal + - if compatible, then strict notice + */ + + + for (i = 0; i < ce->num_traits; i++) { + for (zend_hash_internal_pointer_reset(&ce->traits[i]->properties_info); + zend_hash_get_current_data(&ce->traits[i]->properties_info, (void *) &property_info) == SUCCESS; + zend_hash_move_forward(&ce->traits[i]->properties_info)) { + /* property_info now contains the property */ + + /* first get the unmangeld name if necessary, + then check whether the property is already there */ + if ((property_info->flags & ZEND_ACC_PPP_MASK) == ZEND_ACC_PUBLIC) { + prop_name = property_info->name; + prop_name_length = property_info->name_length; + prop_found = zend_hash_quick_find(&ce->properties_info, + property_info->name, property_info->name_length+1, + property_info->h, (void **) &coliding_prop) == SUCCESS; + } + else { + /* for private and protected we need to unmangle the names */ + zend_unmangle_property_name(property_info->name, property_info->name_length, + &class_name_unused, &prop_name); + prop_name_length = strlen(prop_name); + prop_found = zend_hash_find(&ce->properties_info, prop_name, prop_name_length+1, (void **) &coliding_prop) == SUCCESS; + } + + /* next: check for conflicts with current class */ + if (prop_found) { + if (coliding_prop->flags & ZEND_ACC_SHADOW) { + /* this one is inherited, lets look it up in its own class */ + zend_hash_find(&coliding_prop->ce->properties_info, prop_name, prop_name_length+1, (void **) &coliding_prop); + } + if ((coliding_prop->flags & ZEND_ACC_PPP_MASK) == (property_info->flags & ZEND_ACC_PPP_MASK)) { + /* flags are identical, now the value needs to be checked */ + if (property_info->flags & ZEND_ACC_STATIC) { + not_compatible = compare_function(&compare_result, + ce->default_static_members_table[coliding_prop->offset], + ce->traits[i]->default_static_members_table[property_info->offset] TSRMLS_CC) == FAILURE; + } + else { + not_compatible = compare_function(&compare_result, + ce->default_properties_table[coliding_prop->offset], + ce->traits[i]->default_properties_table[property_info->offset] TSRMLS_CC) == FAILURE; + } + } + else { + /* the flags are not identical, thus, we assume properties are not compatible */ + not_compatible = true; + } + + if (not_compatible) { + zend_error(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", + find_first_definition(ce, i, prop_name, prop_name_length, coliding_prop->ce)->name, + property_info->ce->name, + prop_name, + ce->name); + } + else { + zend_error(E_STRICT, + "%s and %s define the same property ($%s) in the composition of %s. This might be incompatible, to improve maintainability consider using accessor methods in traits instead. Class was composed", + find_first_definition(ce, i, prop_name, prop_name_length, coliding_prop->ce)->name, + property_info->ce->name, + prop_name, + ce->name); + } + } + + /* property not found, so lets add it */ + if (property_info->flags & ZEND_ACC_STATIC) { + prop_value = ce->traits[i]->default_static_members_table[property_info->offset]; + } + else { + prop_value = ce->traits[i]->default_properties_table[property_info->offset]; + } + + zend_declare_property_ex(ce, prop_name, prop_name_length, + prop_value, property_info->flags, + property_info->doc_comment, property_info->doc_comment_len TSRMLS_CC); + } + } +} + + +ZEND_API void zend_do_bind_traits(zend_class_entry *ce TSRMLS_DC) /* {{{ */ +{ + + if (ce->num_traits <= 0) { + return; + } + + /* complete initialization of trait strutures in ce */ + zend_traits_init_trait_structures(ce TSRMLS_CC); + + /* first care about all methods to be flattened into the class */ + zend_do_traits_method_binding(ce TSRMLS_CC); + + /* then flatten the properties into it, to, mostly to notfiy developer about problems */ + zend_do_traits_property_binding(ce TSRMLS_CC); + + /* verify that all abstract methods from traits have been implemented */ zend_verify_abstract_class(ce TSRMLS_CC); + /* now everything should be fine and an added ZEND_ACC_IMPLICIT_ABSTRACT_CLASS should be removed */ if (ce->ce_flags & ZEND_ACC_IMPLICIT_ABSTRACT_CLASS) { ce->ce_flags -= ZEND_ACC_IMPLICIT_ABSTRACT_CLASS;