gron Mon, 20 Dec 2010 00:52:40 +0000
Revision: http://svn.php.net/viewvc?view=revision&revision=306476
Log:
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.
Changed paths:
A php/php-src/trunk/Zend/tests/traits/property001.phpt
A php/php-src/trunk/Zend/tests/traits/property002.phpt
A php/php-src/trunk/Zend/tests/traits/property003.phpt
A php/php-src/trunk/Zend/tests/traits/property004.phpt
A php/php-src/trunk/Zend/tests/traits/property005.phpt
U php/php-src/trunk/Zend/zend_compile.c
Added: php/php-src/trunk/Zend/tests/traits/property001.phpt
===================================================================
--- php/php-src/trunk/Zend/tests/traits/property001.phpt (rev 0)
+++ php/php-src/trunk/Zend/tests/traits/property001.phpt 2010-12-20 00:52:40 UTC (rev 306476)
@@ -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--
+<?php
+error_reporting(E_ALL);
+
+trait THello1 {
+ private $foo;
+}
+
+trait THello2 {
+ private $foo;
+}
+
+echo "PRE-CLASS-GUARD-TraitsTest\n";
+
+class TraitsTest {
+ use THello1;
+ use THello2;
+}
+
+error_reporting(E_ALL | E_STRICT);
+
+echo "PRE-CLASS-GUARD-TraitsTest2\n";
+
+class TraitsTest2 {
+ use THello1;
+ use THello2;
+}
+
+var_dump(property_exists('TraitsTest', 'foo'));
+var_dump(property_exists('TraitsTest2', 'foo'));
+?>
+--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
Added: php/php-src/trunk/Zend/tests/traits/property002.phpt
===================================================================
--- php/php-src/trunk/Zend/tests/traits/property002.phpt (rev 0)
+++ php/php-src/trunk/Zend/tests/traits/property002.phpt 2010-12-20 00:52:40 UTC (rev 306476)
@@ -0,0 +1,32 @@
+--TEST--
+Non-conflicting properties should work just fine.
+--FILE--
+<?php
+error_reporting(E_ALL);
+
+trait THello1 {
+ public $hello = "hello";
+}
+
+trait THello2 {
+ private $world = "World!";
+}
+
+class TraitsTest {
+ use THello1;
+ use THello2;
+ function test() {
+ echo $this->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
Added: php/php-src/trunk/Zend/tests/traits/property003.phpt
===================================================================
--- php/php-src/trunk/Zend/tests/traits/property003.phpt (rev 0)
+++ php/php-src/trunk/Zend/tests/traits/property003.phpt 2010-12-20 00:52:40 UTC (rev 306476)
@@ -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--
+<?php
+error_reporting(E_ALL);
+
+trait THello1 {
+ public $hello;
+}
+
+trait THello2 {
+ private $hello;
+}
+
+echo "PRE-CLASS-GUARD\n";
+
+class TraitsTest {
+ use THello1;
+ use THello2;
+}
+
+echo "POST-CLASS-GUARD\n";
+
+$t = new TraitsTest;
+$t->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
Added: php/php-src/trunk/Zend/tests/traits/property004.phpt
===================================================================
--- php/php-src/trunk/Zend/tests/traits/property004.phpt (rev 0)
+++ php/php-src/trunk/Zend/tests/traits/property004.phpt 2010-12-20 00:52:40 UTC (rev 306476)
@@ -0,0 +1,30 @@
+--TEST--
+Conflicting properties with different initial values are considered incompatible.
+--FILE--
+<?php
+error_reporting(E_ALL);
+
+trait THello1 {
+ public $hello = "foo";
+}
+
+trait THello2 {
+ private $hello = "bar";
+}
+
+echo "PRE-CLASS-GUARD\n";
+
+class TraitsTest {
+ use THello1;
+ use THello2;
+ public function getHello() {
+ return $this->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
Added: php/php-src/trunk/Zend/tests/traits/property005.phpt
===================================================================
--- php/php-src/trunk/Zend/tests/traits/property005.phpt (rev 0)
+++ php/php-src/trunk/Zend/tests/traits/property005.phpt 2010-12-20 00:52:40 UTC (rev 306476)
@@ -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--
+<?php
+error_reporting(E_ALL | E_STRICT);
+
+class Base {
+ private $hello;
+}
+
+trait THello1 {
+ private $hello;
+}
+
+echo "PRE-CLASS-GUARD\n";
+class NoticeForBase extends Base {
+ use THello1;
+}
+echo "POST-CLASS-GUARD\n";
+
+// now the same with a class that defines the property itself
+
+class Notice extends Base {
+ use THello1;
+ private $hello;
+}
+echo "POST-CLASS-GUARD2\n";
+
+// now we do the test for a fatal error
+
+class TraitsTest {
+ use THello1;
+ public $hello;
+}
+
+echo "POST-CLASS-GUARD\n";
+
+$t = new TraitsTest;
+$t->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
Modified: php/php-src/trunk/Zend/zend_compile.c
===================================================================
--- php/php-src/trunk/Zend/zend_compile.c 2010-12-19 23:47:00 UTC (rev 306475)
+++ php/php-src/trunk/Zend/zend_compile.c 2010-12-20 00:52:40 UTC (rev 306476)
@@ -3804,59 +3804,49 @@
}
}
-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_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;
--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
