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;
