Fix more issues for reference values (#5613) * Fix more issues for reference values * Revert change in gdb test * Add more tests
diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c index 610d3eb..0c5b68c 100644 --- a/php/ext/google/protobuf/storage.c +++ b/php/ext/google/protobuf/storage.c
@@ -161,6 +161,11 @@ bool native_slot_set_by_array(upb_fieldtype_t type, const zend_class_entry* klass, void* memory, zval* value TSRMLS_DC) { +#if PHP_MAJOR_VERSION >= 7 + if (Z_ISREF_P(value)) { + ZVAL_DEREF(value); + } +#endif switch (type) { case UPB_TYPE_STRING: case UPB_TYPE_BYTES: { @@ -186,12 +191,6 @@ break; } case UPB_TYPE_MESSAGE: { -#if PHP_MAJOR_VERSION >= 7 - if (Z_ISREF_P(value)) { - ZVAL_DEREF(value); - } -#endif - if (Z_TYPE_P(value) != IS_OBJECT) { zend_error(E_USER_ERROR, "Given value is not message."); return false; @@ -219,6 +218,11 @@ bool native_slot_set_by_map(upb_fieldtype_t type, const zend_class_entry* klass, void* memory, zval* value TSRMLS_DC) { +#if PHP_MAJOR_VERSION >= 7 + if (Z_ISREF_P(value)) { + ZVAL_DEREF(value); + } +#endif switch (type) { case UPB_TYPE_STRING: case UPB_TYPE_BYTES: {
diff --git a/php/ext/google/protobuf/type_check.c b/php/ext/google/protobuf/type_check.c index 31dc444..d8a9245 100644 --- a/php/ext/google/protobuf/type_check.c +++ b/php/ext/google/protobuf/type_check.c
@@ -373,6 +373,11 @@ } bool protobuf_convert_to_string(zval* from) { +#if PHP_MAJOR_VERSION >= 7 + if (Z_ISREF_P(from)) { + ZVAL_DEREF(from); + } +#endif TSRMLS_FETCH(); switch (Z_TYPE_P(from)) { case IS_STRING: {
diff --git a/php/tests/array_test.php b/php/tests/array_test.php index d47a107..b251404 100644 --- a/php/tests/array_test.php +++ b/php/tests/array_test.php
@@ -533,33 +533,41 @@ # Test reference in array ######################################################### - public function testArrayElementIsReference() + public function testArrayElementIsReferenceInSetters() { + // Bool elements + $values = [true]; + array_walk($values, function (&$value) {}); + $m = new TestMessage(); + $m->setRepeatedBool($values); + + // Int32 elements + $values = [1]; + array_walk($values, function (&$value) {}); + $m = new TestMessage(); + $m->setRepeatedInt32($values); + + // Double elements + $values = [1.0]; + array_walk($values, function (&$value) {}); + $m = new TestMessage(); + $m->setRepeatedDouble($values); + + // String elements + $values = ['a']; + array_walk($values, function (&$value) {}); + $m = new TestMessage(); + $m->setRepeatedString($values); + + // Message elements $m = new TestMessage(); $subs = [1, 2]; - foreach ($subs as &$sub) { $sub = new Sub(['a' => $sub]); } - $m->setRepeatedMessage($subs); } - public function testArrayIsReference() - { - $keys = [['repeated_message' => [['a' => 1]]]]; - - foreach ($keys as &$key) { - foreach ($key['repeated_message'] as &$element) { - $element = new Sub($element); - } - $key = new TestMessage($key); - } - - $m = new TestMessage(); - $m->setRepeatedDeep($keys); - } - ######################################################### # Test memory leak #########################################################
diff --git a/php/tests/generated_class_test.php b/php/tests/generated_class_test.php index 83deaba..93b7b29 100644 --- a/php/tests/generated_class_test.php +++ b/php/tests/generated_class_test.php
@@ -1375,6 +1375,78 @@ $this->assertTrue(true); } + public function testReferenceInArrayConstructor() + { + $keys = [[ + 'optional_bool' => true, + 'repeated_bool' => [true], + 'map_bool_bool' => [true => true], + 'optional_double' => 1.0, + 'repeated_double' => [1.0], + 'map_int32_double' => [1 => 1.0], + 'optional_int32' => 1, + 'repeated_int32' => [1], + 'map_int32_int32' => [1 => 1], + 'optional_string' => 'a', + 'repeated_string' => ['a'], + 'map_string_string' => ['a' => 'a'], + 'optional_message' => ['a' => 1], + 'repeated_message' => [['a' => 1]], + 'map_int32_message' => [1 => ['a' => 1]], + ]]; + + foreach ($keys as &$key) { + foreach ($key as $id => &$value) { + if ($id === 'repeated_bool') { + foreach ($value as &$element) { + } + } + if ($id === 'map_bool_bool') { + foreach ($value as $mapKey => &$element) { + } + } + if ($id === 'repeated_double') { + foreach ($value as &$element) { + } + } + if ($id === 'map_int32_double') { + foreach ($value as $mapKey => &$element) { + } + } + if ($id === 'repeated_int32') { + foreach ($value as &$element) { + } + } + if ($id === 'map_int32_int32') { + foreach ($value as $mapKey => &$element) { + } + } + if ($id === 'repeated_string') { + foreach ($value as &$element) { + } + } + if ($id === 'map_string_string') { + foreach ($value as $mapKey => &$element) { + } + } + if ($id === 'optional_message') { + $value = new Sub($value); + } + if ($id === 'repeated_message') { + foreach ($value as &$element) { + $element = new Sub($element); + } + } + if ($id === 'map_int32_message') { + foreach ($value as $mapKey => &$element) { + $element = new Sub($element); + } + } + } + $key = new TestMessage($key); + } + } + ######################################################### # Test message equals. ######################################################### @@ -1387,4 +1459,35 @@ TestUtil::setTestMessage($n); $this->assertEquals($m, $n); } + + ######################################################### + # Test reference of value + ######################################################### + + public function testValueIsReference() + { + // Bool element + $values = [true]; + array_walk($values, function (&$value) {}); + $m = new TestMessage(); + $m->setOptionalBool($values[0]); + + // Int32 element + $values = [1]; + array_walk($values, function (&$value) {}); + $m = new TestMessage(); + $m->setOptionalInt32($values[0]); + + // Double element + $values = [1.0]; + array_walk($values, function (&$value) {}); + $m = new TestMessage(); + $m->setOptionalDouble($values[0]); + + // String element + $values = ['a']; + array_walk($values, function (&$value) {}); + $m = new TestMessage(); + $m->setOptionalString($values[0]); + } }
diff --git a/php/tests/map_field_test.php b/php/tests/map_field_test.php index 0260879..577be68 100644 --- a/php/tests/map_field_test.php +++ b/php/tests/map_field_test.php
@@ -443,6 +443,43 @@ } ######################################################### + # Test reference in map + ######################################################### + + public function testMapElementIsReference() + { + // Bool elements + $values = [true => true]; + array_walk($values, function (&$value) {}); + $m = new TestMessage(); + $m->setMapBoolBool($values); + + // Int32 elements + $values = [1 => 1]; + array_walk($values, function (&$value) {}); + $m = new TestMessage(); + $m->setMapInt32Int32($values); + + // Double elements + $values = [1 => 1.0]; + array_walk($values, function (&$value) {}); + $m = new TestMessage(); + $m->setMapInt32Double($values); + + // String elements + $values = ['a' => 'a']; + array_walk($values, function (&$value) {}); + $m = new TestMessage(); + $m->setMapStringString($values); + + // Message elements + $values = [1 => new Sub()]; + array_walk($values, function (&$value) {}); + $m = new TestMessage(); + $m->setMapInt32Message($values); + } + + ######################################################### # Test memory leak #########################################################
diff --git a/php/tests/proto/test.proto b/php/tests/proto/test.proto index ca39ea4..e610c58 100644 --- a/php/tests/proto/test.proto +++ b/php/tests/proto/test.proto
@@ -31,7 +31,6 @@ Sub optional_message = 17; bar.TestInclude optional_included_message = 18; TestMessage recursive = 19; - repeated TestMessage repeated_deep = 20; // Repeated repeated int32 repeated_int32 = 31;