Hi Marcus!
>> 1. What should happen when the argument is an object?
>
> Seems like an error message is missing there. It allows to take an instance
> of another ArrayObject/Iterator and use the array from that.
>
> In case any other Object is passed it is ignored. What do you feel?
Sounds alright to me, but it's inconsistent with __construct(), which
in addition to using the storage array from ArrayObject/Iterator
instances, can use the properties from any other kind of object. Also,
just to be clear, I should point out that it is not the current
behaviour. Passing any object to exchangeArray(), even an instance of
ArrayObject/Iterator, eradicates the container:
<?php
$ao1 = new ArrayObject(array('one'));
$ao2 = new ArrayObject(array('two'));
$ao1->exchangeArray($ao2);
var_dump($ao1);
?>
Output on PHP 5.3.0-dev (cli) (built: Jul 28 2008 00:21:59):
object(ArrayObject)#1 (0) {
}
>> Here's a suggested patch against 5_3 that implements this (and
>> includes some tests): http://pastebin.ca/1091668
>> Note that I'm not an internals or SPL expert, so let me know if I
>> missed something.
>
> Seems about right. Can you attach the patch as a text file (I cannot open
> the link).
Attached. I just naively moved some logic from __construct() into a
separate function and called it from both __construct() and
exchangeArray().
Thanks!
Robin
Index: spl_array.c
===================================================================
RCS file: /repository/php-src/ext/spl/spl_array.c,v
retrieving revision 1.71.2.17.2.13.2.19
diff -u -w -p -r1.71.2.17.2.13.2.19 spl_array.c
--- spl_array.c 26 Jul 2008 12:34:10 -0000 1.71.2.17.2.13.2.19
+++ spl_array.c 3 Aug 2008 19:42:50 -0000
@@ -892,6 +892,51 @@ static void spl_array_it_rewind(zend_obj
}
/* }}} */
+/* {{{ spl_array_set_array */
+static void spl_array_set_array(zval *object, spl_array_object *intern, zval
**array, long ar_flags, int just_array TSRMLS_DC) {
+
+ if (Z_TYPE_PP(array) == IS_ARRAY) {
+ SEPARATE_ZVAL_IF_NOT_REF(array);
+ }
+
+ if (Z_TYPE_PP(array) == IS_OBJECT && (Z_OBJ_HT_PP(array) ==
&spl_handler_ArrayObject || Z_OBJ_HT_PP(array) == &spl_handler_ArrayIterator)) {
+ zval_ptr_dtor(&intern->array);
+ if (just_array) {
+ spl_array_object *other =
(spl_array_object*)zend_object_store_get_object(*array TSRMLS_CC);
+ ar_flags = other->ar_flags & ~SPL_ARRAY_INT_MASK;
+ }
+ ar_flags |= SPL_ARRAY_USE_OTHER;
+ intern->array = *array;
+ } else {
+ if (Z_TYPE_PP(array) != IS_OBJECT && Z_TYPE_PP(array) !=
IS_ARRAY) {
+ php_set_error_handling(EH_NORMAL, NULL TSRMLS_CC);
+ zend_throw_exception(spl_ce_InvalidArgumentException,
"Passed variable is not an array or object, using empty array instead", 0
TSRMLS_CC);
+ return;
+ }
+ zval_ptr_dtor(&intern->array);
+ intern->array = *array;
+ }
+ if (object == *array) {
+ intern->ar_flags |= SPL_ARRAY_IS_SELF;
+ intern->ar_flags &= ~SPL_ARRAY_USE_OTHER;
+ } else {
+ intern->ar_flags &= ~SPL_ARRAY_IS_SELF;
+ }
+ intern->ar_flags |= ar_flags;
+ Z_ADDREF_P(intern->array);
+ if (Z_TYPE_PP(array) == IS_OBJECT) {
+ zend_object_get_properties_t handler = Z_OBJ_HANDLER_PP(array,
get_properties);
+ if ((handler != std_object_handlers.get_properties && handler
!= spl_array_get_properties)
+ || !spl_array_get_hash_table(intern, 0 TSRMLS_CC)) {
+ php_set_error_handling(EH_NORMAL, NULL TSRMLS_CC);
+
zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0 TSRMLS_CC,
"Overloaded object of type %s is not compatible with %s",
Z_OBJCE_PP(array)->name, intern->std.ce->name);
+ }
+ }
+
+ spl_array_rewind(intern TSRMLS_CC);
+}
+/* }}} */
+
/* iterator handler table */
zend_object_iterator_funcs spl_array_it_funcs = {
spl_array_it_dtor,
@@ -949,10 +994,6 @@ SPL_METHOD(Array, __construct)
return;
}
- if (Z_TYPE_PP(array) == IS_ARRAY) {
- SEPARATE_ZVAL_IF_NOT_REF(array);
- }
-
if (ZEND_NUM_ARGS() > 2) {
if (zend_lookup_class(class_name, class_name_len,
&pce_get_iterator TSRMLS_CC) == FAILURE) {
zend_throw_exception(spl_ce_InvalidArgumentException,
"A class that implements Iterator must be specified", 0 TSRMLS_CC);
@@ -964,43 +1005,7 @@ SPL_METHOD(Array, __construct)
ar_flags &= ~SPL_ARRAY_INT_MASK;
- if (Z_TYPE_PP(array) == IS_OBJECT && (Z_OBJ_HT_PP(array) ==
&spl_handler_ArrayObject || Z_OBJ_HT_PP(array) == &spl_handler_ArrayIterator)) {
- zval_ptr_dtor(&intern->array);
- if (ZEND_NUM_ARGS() == 1)
- {
- spl_array_object *other =
(spl_array_object*)zend_object_store_get_object(*array TSRMLS_CC);
- ar_flags = other->ar_flags & ~SPL_ARRAY_INT_MASK;
- }
- ar_flags |= SPL_ARRAY_USE_OTHER;
- intern->array = *array;
- } else {
- if (Z_TYPE_PP(array) != IS_OBJECT && Z_TYPE_PP(array) !=
IS_ARRAY) {
- php_set_error_handling(EH_NORMAL, NULL TSRMLS_CC);
- zend_throw_exception(spl_ce_InvalidArgumentException,
"Passed variable is not an array or object, using empty array instead", 0
TSRMLS_CC);
- return;
- }
- zval_ptr_dtor(&intern->array);
- intern->array = *array;
- }
- if (object == *array) {
- intern->ar_flags |= SPL_ARRAY_IS_SELF;
- intern->ar_flags &= ~SPL_ARRAY_USE_OTHER;
- } else {
- intern->ar_flags &= ~SPL_ARRAY_IS_SELF;
- }
- intern->ar_flags |= ar_flags;
- Z_ADDREF_P(intern->array);
- if (Z_TYPE_PP(array) == IS_OBJECT) {
- zend_object_get_properties_t handler = Z_OBJ_HANDLER_PP(array,
get_properties);
- if ((handler != std_object_handlers.get_properties && handler
!= spl_array_get_properties)
- || !spl_array_get_hash_table(intern, 0 TSRMLS_CC)) {
- php_set_error_handling(EH_NORMAL, NULL TSRMLS_CC);
-
zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0 TSRMLS_CC,
"Overloaded object of type %s is not compatible with %s",
Z_OBJCE_PP(array)->name, intern->std.ce->name);
- return;
- }
- }
-
- spl_array_rewind(intern TSRMLS_CC);
+ spl_array_set_array(object, intern, array, ar_flags, ZEND_NUM_ARGS() ==
1 TSRMLS_CC);
php_set_error_handling(EH_NORMAL, NULL TSRMLS_CC);
}
@@ -1081,31 +1086,9 @@ SPL_METHOD(Array, exchangeArray)
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "Z", &array) ==
FAILURE) {
return;
}
- if (Z_TYPE_PP(array) == IS_OBJECT && intern ==
(spl_array_object*)zend_object_store_get_object(object TSRMLS_CC)) {
- zval_ptr_dtor(&intern->array);
- array = &object;
- intern->array = object;
- } else if (Z_TYPE_PP(array) == IS_OBJECT && (Z_OBJ_HT_PP(array) ==
&spl_handler_ArrayObject || Z_OBJ_HT_PP(array) == &spl_handler_ArrayIterator)) {
- spl_array_object *other =
(spl_array_object*)zend_object_store_get_object(*array TSRMLS_CC);
- zval_ptr_dtor(&intern->array);
- intern->array = other->array;
- } else {
- if (Z_TYPE_PP(array) != IS_OBJECT && !HASH_OF(*array)) {
- zend_throw_exception(spl_ce_InvalidArgumentException,
"Passed variable is not an array or object, using empty array instead", 0
TSRMLS_CC);
- return;
- }
- zval_ptr_dtor(&intern->array);
- intern->array = *array;
- intern->ar_flags &= ~SPL_ARRAY_USE_OTHER;
- }
- if (object == *array) {
- intern->ar_flags |= SPL_ARRAY_IS_SELF;
- } else {
- intern->ar_flags &= ~SPL_ARRAY_IS_SELF;
- }
- Z_ADDREF_P(intern->array);
- spl_array_rewind(intern TSRMLS_CC);
+ spl_array_set_array(object, intern, array, 0L, 1 TSRMLS_CC);
+
}
/* }}} */
Index: tests/arrayObject_exchange_basic1.phpt
===================================================================
RCS file: tests/arrayObject_exchange_basic1.phpt
diff -N tests/arrayObject_exchange_basic1.phpt
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/arrayObject_exchange_basic1.phpt 3 Aug 2008 09:44:39 -0000
@@ -0,0 +1,40 @@
+--TEST--
+ArrayObject::exchange() and copy-on-write references
+--FILE--
+<?php
+$ao = new ArrayObject();
+$swapIn = array();
+$cowRef = $swapIn; // create a copy-on-write ref to $swapIn
+$ao->exchangeArray($swapIn);
+
+$ao['a'] = 'adding element to $ao';
+$swapIn['b'] = 'adding element to $swapIn';
+$ao['c'] = 'adding another element to $ao';
+
+echo "\n--> swapIn: ";
+var_dump($swapIn);
+
+echo "\n--> cowRef: ";
+var_dump($cowRef);
+
+echo "\n--> ao: ";
+var_dump($ao);
+?>
+--EXPECTF--
+--> swapIn: array(1) {
+ ["b"]=>
+ string(25) "adding element to $swapIn"
+}
+
+--> cowRef: array(0) {
+}
+
+--> ao: object(ArrayObject)#%d (1) {
+ ["storage":"ArrayObject":private]=>
+ array(2) {
+ ["a"]=>
+ string(21) "adding element to $ao"
+ ["c"]=>
+ string(29) "adding another element to $ao"
+ }
+}
Index: tests/arrayObject_exchange_basic2.phpt
===================================================================
RCS file: tests/arrayObject_exchange_basic2.phpt
diff -N tests/arrayObject_exchange_basic2.phpt
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/arrayObject_exchange_basic2.phpt 3 Aug 2008 09:48:20 -0000
@@ -0,0 +1,33 @@
+--TEST--
+ArrayObject::exchangeArray(object)
+--FILE--
+<?php
+echo "--> exchangeArray(array):\n";
+$ao = new ArrayObject();
+$ao->exchangeArray(array('original'));
+var_dump($ao);
+
+echo "\n--> exchangeArray(object):\n";
+$obj = new stdClass;
+$obj->newProp = 'changed';
+$ao->exchangeArray($obj);
+var_dump($ao);
+?>
+--EXPECTF--
+--> exchangeArray(array):
+object(ArrayObject)#%d (1) {
+ ["storage":"ArrayObject":private]=>
+ array(1) {
+ [0]=>
+ string(8) "original"
+ }
+}
+
+--> exchangeArray(object):
+object(ArrayObject)#%d (1) {
+ ["storage":"ArrayObject":private]=>
+ object(stdClass)#%d (1) {
+ ["newProp"]=>
+ string(7) "changed"
+ }
+}
\ No newline at end of file
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
