Fix more issues for reference values (#5613)

* Fix more issues for reference values

* Revert change in gdb test

* Add more tests

php/ext/google/protobuf/storage.c

@@ -161,6 +161,11 @@ bool native_slot_set(upb_fieldtype_t type, const zend_class_entry* klass,
 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 @@ bool native_slot_set_by_array(upb_fieldtype_t type,
       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_array(upb_fieldtype_t type,
 
 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: {

php/ext/google/protobuf/type_check.c

@@ -373,6 +373,11 @@ bool protobuf_convert_to_bool(zval* from, int8_t* to) {
 }
 
 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: {

php/tests/array_test.php

@@ -533,31 +533,39 @@ class RepeatedFieldTest extends \PHPUnit\Framework\TestCase
     # Test reference in array
     #########################################################
 
-    public function testArrayElementIsReference()
+    public function testArrayElementIsReferenceInSetters()
     {
+        // Bool elements
+        $values = [true];
+        array_walk($values, function (&$value) {});
         $m = new TestMessage();
-        $subs = [1, 2];
-
-        foreach ($subs as &$sub) {
-            $sub = new Sub(['a' => $sub]);
-        }
+        $m->setRepeatedBool($values);
 
-        $m->setRepeatedMessage($subs);
-    }
+        // Int32 elements
+        $values = [1];
+        array_walk($values, function (&$value) {});
+        $m = new TestMessage();
+        $m->setRepeatedInt32($values);
 
-    public function testArrayIsReference()
-    {
-        $keys = [['repeated_message' => [['a' => 1]]]];
+        // Double elements
+        $values = [1.0];
+        array_walk($values, function (&$value) {});
+        $m = new TestMessage();
+        $m->setRepeatedDouble($values);
 
-        foreach ($keys as &$key) {
-            foreach ($key['repeated_message'] as &$element) {
-              $element = new Sub($element);
-            }
-            $key = new TestMessage($key);
-        }
+        // String elements
+        $values = ['a'];
+        array_walk($values, function (&$value) {});
+        $m = new TestMessage();
+        $m->setRepeatedString($values);
 
+        // Message elements
         $m = new TestMessage();
-        $m->setRepeatedDeep($keys);
+        $subs = [1, 2];
+        foreach ($subs as &$sub) {
+            $sub = new Sub(['a' => $sub]);
+        }
+        $m->setRepeatedMessage($subs);
     }
 
     #########################################################

php/tests/generated_class_test.php

@@ -1375,6 +1375,78 @@ class GeneratedClassTest extends TestBase
         $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 @@ class GeneratedClassTest extends TestBase
         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]);
+    }
 }

php/tests/map_field_test.php

@@ -442,6 +442,43 @@ class MapFieldTest extends \PHPUnit\Framework\TestCase {
         $this->assertSame(3, $i);
     }
 
+    #########################################################
+    # 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
     #########################################################

php/tests/proto/test.proto

@@ -31,7 +31,6 @@ message TestMessage {
   Sub optional_message = 17;
   bar.TestInclude optional_included_message = 18;
   TestMessage recursive = 19;
-  repeated TestMessage repeated_deep = 20;
 
   // Repeated
   repeated    int32 repeated_int32    = 31;