Fixed clone for Message, RepeatedField, and MapField. (#8245)

Also updated the code to use a TypeInfo struct for convenient
passing of type and desc together. This simplified a lot of code
and made this change easier to write.

php/ext/google/protobuf/array.c

@@ -55,8 +55,7 @@ typedef struct {
   zend_object std;
   zval arena;
   upb_array *array;
-  upb_fieldtype_t type;
-  const Descriptor* desc;  // When values are messages.
+  TypeInfo type;
 } RepeatedField;
 
 zend_class_entry *RepeatedField_class_entry;
@@ -76,7 +75,6 @@ static zend_object* RepeatedField_create(zend_class_entry *class_type) {
   intern->std.handlers = &RepeatedField_object_handlers;
   Arena_Init(&intern->arena);
   intern->array = NULL;
-  intern->desc = NULL;
   // Skip object_properties_init(), we don't allow derived classes.
   return &intern->std;
 }
@@ -106,13 +104,35 @@ static void RepeatedField_destructor(zend_object* obj) {
 static int RepeatedField_compare_objects(zval *rf1, zval *rf2) {
   RepeatedField* intern1 = (RepeatedField*)Z_OBJ_P(rf1);
   RepeatedField* intern2 = (RepeatedField*)Z_OBJ_P(rf2);
-  upb_fieldtype_t type = intern1->type;
-  const upb_msgdef *m = intern1->desc ? intern1->desc->msgdef : NULL;
 
-  if (type != intern2->type) return 1;
-  if (intern1->desc != intern2->desc) return 1;
+  return TypeInfo_Eq(intern1->type, intern2->type) &&
+                 ArrayEq(intern1->array, intern2->array, intern1->type)
+             ? 0
+             : 1;
+}
+
+/**
+ * RepeatedField_clone_obj()
+ *
+ * Object handler for cloning an object in PHP. Called when PHP code does:
+ *
+ *   $rf2 = clone $rf1;
+ */
+static zend_object *RepeatedField_clone_obj(zval *object) {
+  RepeatedField* intern = (RepeatedField*)Z_OBJ_P(object);
+  upb_arena *arena = Arena_Get(&intern->arena);
+  upb_array *clone = upb_array_new(arena, intern->type.type);
+  size_t n = upb_array_size(intern->array);
+  size_t i;
+
+  for (i = 0; i < n; i++) {
+    upb_msgval msgval = upb_array_get(intern->array, i);
+    upb_array_append(clone, msgval, arena);
+  }
 
-  return ArrayEq(intern1->array, intern2->array, type, m) ? 0 : 1;
+  zval ret;
+  RepeatedField_GetPhpWrapper(&ret, clone, intern->type, &intern->arena);
+  return Z_OBJ_P(&ret);
 }
 
 static HashTable *RepeatedField_GetProperties(PROTO_VAL *object) {
@@ -129,8 +149,8 @@ static zval *RepeatedField_GetPropertyPtrPtr(PROTO_VAL *object,
 
 // These are documented in the header file.
 
-void RepeatedField_GetPhpWrapper(zval *val, upb_array *arr,
-                                 const upb_fielddef *f, zval *arena) {
+void RepeatedField_GetPhpWrapper(zval *val, upb_array *arr, TypeInfo type,
+                                 zval *arena) {
   if (!arr) {
     ZVAL_NULL(val);
     return;
@@ -142,27 +162,24 @@ void RepeatedField_GetPhpWrapper(zval *val, upb_array *arr,
     intern->std.handlers = &RepeatedField_object_handlers;
     ZVAL_COPY(&intern->arena, arena);
     intern->array = arr;
-    intern->type = upb_fielddef_type(f);
-    intern->desc = Descriptor_GetFromFieldDef(f);
+    intern->type = type;
     // Skip object_properties_init(), we don't allow derived classes.
     ObjCache_Add(intern->array, &intern->std);
     ZVAL_OBJ(val, &intern->std);
   }
 }
 
-upb_array *RepeatedField_GetUpbArray(zval *val, const upb_fielddef *f,
+upb_array *RepeatedField_GetUpbArray(zval *val, TypeInfo type,
                                      upb_arena *arena) {
   if (Z_ISREF_P(val)) {
     ZVAL_DEREF(val);
   }
 
   if (Z_TYPE_P(val) == IS_ARRAY) {
     // Auto-construct, eg. [1, 2, 3] -> upb_array([1, 2, 3]).
-    upb_array *arr = upb_array_new(arena, upb_fielddef_type(f));
+    upb_array *arr = upb_array_new(arena, type.type);
     HashTable *table = HASH_OF(val);
     HashPosition pos;
-    upb_fieldtype_t type = upb_fielddef_type(f);
-    const Descriptor *desc = Descriptor_GetFromFieldDef(f);
 
     zend_hash_internal_pointer_reset_ex(table, &pos);
 
@@ -172,7 +189,7 @@ upb_array *RepeatedField_GetUpbArray(zval *val, const upb_fielddef *f,
 
       if (!zv) return arr;
 
-      if (!Convert_PhpToUpbAutoWrap(zv, &val, type, desc, arena)) {
+      if (!Convert_PhpToUpbAutoWrap(zv, &val, type, arena)) {
         return NULL;
       }
 
@@ -183,9 +200,8 @@ upb_array *RepeatedField_GetUpbArray(zval *val, const upb_fielddef *f,
              Z_OBJCE_P(val) == RepeatedField_class_entry) {
     // Unwrap existing RepeatedField object to get the upb_array* inside.
     RepeatedField *intern = (RepeatedField*)Z_OBJ_P(val);
-    const Descriptor *desc = Descriptor_GetFromFieldDef(f);
 
-    if (intern->type != upb_fielddef_type(f) || intern->desc != desc) {
+    if (!TypeInfo_Eq(intern->type, type)) {
       php_error_docref(NULL, E_USER_ERROR,
                        "Wrong type for this repeated field.");
     }
@@ -198,8 +214,7 @@ upb_array *RepeatedField_GetUpbArray(zval *val, const upb_fielddef *f,
   }
 }
 
-bool ArrayEq(const upb_array *a1, const upb_array *a2, upb_fieldtype_t type,
-             const upb_msgdef *m) {
+bool ArrayEq(const upb_array *a1, const upb_array *a2, TypeInfo type) {
   size_t i;
   size_t n;
 
@@ -212,7 +227,7 @@ bool ArrayEq(const upb_array *a1, const upb_array *a2, upb_fieldtype_t type,
   for (i = 0; i < n; i++) {
     upb_msgval val1 = upb_array_get(a1, i);
     upb_msgval val2 = upb_array_get(a2, i);
-    if (!ValueEq(val1, val2, type, m)) return false;
+    if (!ValueEq(val1, val2, type)) return false;
   }
 
   return true;
@@ -238,16 +253,16 @@ PHP_METHOD(RepeatedField, __construct) {
     return;
   }
 
-  intern->type = pbphp_dtype_to_type(type);
-  intern->desc = Descriptor_GetFromClassEntry(klass);
+  intern->type.type = pbphp_dtype_to_type(type);
+  intern->type.desc = Descriptor_GetFromClassEntry(klass);
 
-  if (intern->type == UPB_TYPE_MESSAGE && klass == NULL) {
+  if (intern->type.type == UPB_TYPE_MESSAGE && klass == NULL) {
     php_error_docref(NULL, E_USER_ERROR,
                      "Message/enum type must have concrete class.");
     return;
   }
 
-  intern->array = upb_array_new(arena, intern->type);
+  intern->array = upb_array_new(arena, intern->type.type);
   ObjCache_Add(intern->array, &intern->std);
 }
 
@@ -264,7 +279,7 @@ PHP_METHOD(RepeatedField, append) {
   upb_msgval msgval;
 
   if (zend_parse_parameters(ZEND_NUM_ARGS(), "z", &php_val) != SUCCESS ||
-      !Convert_PhpToUpb(php_val, &msgval, intern->type, intern->desc, arena)) {
+      !Convert_PhpToUpb(php_val, &msgval, intern->type, arena)) {
     return;
   }
 
@@ -321,7 +336,7 @@ PHP_METHOD(RepeatedField, offsetGet) {
   }
 
   msgval = upb_array_get(intern->array, index);
-  Convert_UpbToPhp(msgval, &ret, intern->type, intern->desc, &intern->arena);
+  Convert_UpbToPhp(msgval, &ret, intern->type, &intern->arena);
   RETURN_ZVAL(&ret, 0, 1);
 }
 
@@ -357,7 +372,7 @@ PHP_METHOD(RepeatedField, offsetSet) {
     return;
   }
 
-  if (!Convert_PhpToUpb(val, &msgval, intern->type, intern->desc, arena)) {
+  if (!Convert_PhpToUpb(val, &msgval, intern->type, arena)) {
     return;
   }
 
@@ -563,7 +578,7 @@ PHP_METHOD(RepeatedFieldIter, current) {
 
   msgval = upb_array_get(array, index);
 
-  Convert_UpbToPhp(msgval, &ret, field->type, field->desc, &field->arena);
+  Convert_UpbToPhp(msgval, &ret, field->type, &field->arena);
   RETURN_ZVAL(&ret, 0, 1);
 }
 
@@ -638,6 +653,7 @@ void Array_ModuleInit() {
 #else
   h->compare = RepeatedField_compare_objects;
 #endif
+  h->clone_obj = RepeatedField_clone_obj;
   h->get_properties = RepeatedField_GetProperties;
   h->get_property_ptr_ptr = RepeatedField_GetPropertyPtrPtr;
 

php/ext/google/protobuf/array.h

@@ -33,34 +33,35 @@
 
 #include <php.h>
 
+#include "def.h"
 #include "php-upb.h"
 
 // Registers PHP classes for RepeatedField.
 void Array_ModuleInit();
 
 // Gets a upb_array* for the PHP object |val|:
 //  * If |val| is a RepeatedField object, we first check its type and verify
-//    that that the elements have the correct type for |f|. If so, we return the
-//    wrapped upb_array*. We also make sure that this array's arena is fused to
-//    |arena|, so the returned upb_array is guaranteed to live as long as
+//    that that the elements have the correct type for |type|. If so, we return
+//    the wrapped upb_array*. We also make sure that this array's arena is fused
+//    to |arena|, so the returned upb_array is guaranteed to live as long as
 //    |arena|.
 //  * If |val| is a PHP Array, we attempt to create a new upb_array using
 //    |arena| and add all of the PHP elements to it.
 //
 // If an error occurs, we raise a PHP error and return NULL.
-upb_array *RepeatedField_GetUpbArray(zval *val, const upb_fielddef *f, upb_arena *arena);
+upb_array *RepeatedField_GetUpbArray(zval *val, TypeInfo type,
+                                     upb_arena *arena);
 
-// Creates a PHP RepeatedField object for the given upb_array* and |f| and
+// Creates a PHP RepeatedField object for the given upb_array* and |type| and
 // returns it in |val|. The PHP object will keep a reference to this |arena| to
 // ensure the underlying array data stays alive.
 //
 // If |arr| is NULL, this will return a PHP null object.
-void RepeatedField_GetPhpWrapper(zval *val, upb_array *arr,
-                                 const upb_fielddef *f, zval *arena);
+void RepeatedField_GetPhpWrapper(zval *val, upb_array *arr, TypeInfo type,
+                                 zval *arena);
 
 // Returns true if the given arrays are equal. Both arrays must be of this
 // |type| and, if the type is |UPB_TYPE_MESSAGE|, must have the same |m|.
-bool ArrayEq(const upb_array *a1, const upb_array *a2, upb_fieldtype_t type,
-             const upb_msgdef *m);
+bool ArrayEq(const upb_array *a1, const upb_array *a2, TypeInfo type);
 
 #endif  // PHP_PROTOBUF_ARRAY_H_

php/ext/google/protobuf/convert.c

@@ -348,15 +348,15 @@ static bool to_string(zval* from) {
   }
 }
 
-bool Convert_PhpToUpb(zval *php_val, upb_msgval *upb_val, upb_fieldtype_t type,
-                      const Descriptor *desc, upb_arena *arena) {
+bool Convert_PhpToUpb(zval *php_val, upb_msgval *upb_val, TypeInfo type,
+                      upb_arena *arena) {
   int64_t i64;
 
   if (Z_ISREF_P(php_val)) {
     ZVAL_DEREF(php_val);
   }
 
-  switch (type) {
+  switch (type.type) {
     case UPB_TYPE_INT64:
       return Convert_PhpToInt64(php_val, &upb_val->int64_val);
     case UPB_TYPE_INT32:
@@ -408,17 +408,17 @@ bool Convert_PhpToUpb(zval *php_val, upb_msgval *upb_val, upb_fieldtype_t type,
       return true;
     }
     case UPB_TYPE_MESSAGE:
-      PBPHP_ASSERT(desc);
-      return Message_GetUpbMessage(php_val, desc, arena,
+      PBPHP_ASSERT(type.desc);
+      return Message_GetUpbMessage(php_val, type.desc, arena,
                                    (upb_msg **)&upb_val->msg_val);
   }
 
   return false;
 }
 
-void Convert_UpbToPhp(upb_msgval upb_val, zval *php_val, upb_fieldtype_t type,
-                      const Descriptor *desc, zval *arena) {
-  switch (type) {
+void Convert_UpbToPhp(upb_msgval upb_val, zval *php_val, TypeInfo type,
+                      zval *arena) {
+  switch (type.type) {
     case UPB_TYPE_INT64:
 #if SIZEOF_ZEND_LONG == 8
       ZVAL_LONG(php_val, upb_val.int64_val);
@@ -467,25 +467,24 @@ void Convert_UpbToPhp(upb_msgval upb_val, zval *php_val, upb_fieldtype_t type,
       break;
     }
     case UPB_TYPE_MESSAGE:
-      PBPHP_ASSERT(desc);
-      Message_GetPhpWrapper(php_val, desc, (upb_msg*)upb_val.msg_val, arena);
+      PBPHP_ASSERT(type.desc);
+      Message_GetPhpWrapper(php_val, type.desc, (upb_msg *)upb_val.msg_val,
+                            arena);
       break;
   }
 }
 
-bool Convert_PhpToUpbAutoWrap(zval *val, upb_msgval *upb_val,
-                              upb_fieldtype_t type, const Descriptor *desc,
+bool Convert_PhpToUpbAutoWrap(zval *val, upb_msgval *upb_val, TypeInfo type,
                               upb_arena *arena) {
-  const upb_msgdef *subm = desc ? desc->msgdef : NULL;
+  const upb_msgdef *subm = type.desc ? type.desc->msgdef : NULL;
   if (subm && upb_msgdef_iswrapper(subm) && Z_TYPE_P(val) != IS_OBJECT) {
     // Assigning a scalar to a wrapper-typed value. We will automatically wrap
     // the value, so the user doesn't need to create a FooWrapper(['value': X])
     // message manually.
     upb_msg *wrapper = upb_msg_new(subm, arena);
     const upb_fielddef *val_f = upb_msgdef_itof(subm, 1);
-    upb_fieldtype_t type_f = upb_fielddef_type(val_f);
     upb_msgval msgval;
-    if (!Convert_PhpToUpb(val, &msgval, type_f, NULL, arena)) return false;
+    if (!Convert_PhpToUpb(val, &msgval, TypeInfo_Get(val_f), arena)) return false;
     upb_msg_set(wrapper, val_f, msgval, arena);
     upb_val->msg_val = wrapper;
     return true;
@@ -495,7 +494,7 @@ bool Convert_PhpToUpbAutoWrap(zval *val, upb_msgval *upb_val,
     //   ['foo_submsg': new Foo(['a' => 1])]
     // not:
     //   ['foo_submsg': ['a' => 1]]
-    return Convert_PhpToUpb(val, upb_val, type, desc, arena);
+    return Convert_PhpToUpb(val, upb_val, type, arena);
   }
 }
 

php/ext/google/protobuf/convert.h

@@ -46,8 +46,8 @@ bool Convert_PhpToInt64(const zval *php_val, int64_t *i64);
 // UPB_TYPE_MESSAGE, then |desc| must be the Descriptor for this message type.
 // If type is string, message, or bytes, then |arena| will be used to copy
 // string data or fuse this arena to the given message's arena.
-bool Convert_PhpToUpb(zval *php_val, upb_msgval *upb_val, upb_fieldtype_t type,
-                      const Descriptor *desc, upb_arena *arena);
+bool Convert_PhpToUpb(zval *php_val, upb_msgval *upb_val, TypeInfo type,
+                      upb_arena *arena);
 
 // Similar to Convert_PhpToUpb, but supports automatically wrapping the wrapper
 // types if a primitive is specified:
@@ -56,16 +56,15 @@ bool Convert_PhpToUpb(zval *php_val, upb_msgval *upb_val, upb_fieldtype_t type,
 //
 // We currently allow this implicit conversion in initializers, but not for
 // assignment.
-bool Convert_PhpToUpbAutoWrap(zval *val, upb_msgval *upb_val,
-                              upb_fieldtype_t type, const Descriptor *desc,
+bool Convert_PhpToUpbAutoWrap(zval *val, upb_msgval *upb_val, TypeInfo type,
                               upb_arena *arena);
 
 // Converts |upb_val| to a PHP zval according to |type|. This may involve
 // creating a PHP wrapper object. If type == UPB_TYPE_MESSAGE, then |desc| must
 // be the Descriptor for this message type. Any newly created wrapper object
 // will reference |arena|.
-void Convert_UpbToPhp(upb_msgval upb_val, zval *php_val, upb_fieldtype_t type,
-                      const Descriptor *desc, zval *arena);
+void Convert_UpbToPhp(upb_msgval upb_val, zval *php_val, TypeInfo type,
+                      zval *arena);
 
 // Registers the GPBUtil class.
 void Convert_ModuleInit(void);

php/ext/google/protobuf/def.h

@@ -72,4 +72,27 @@ Descriptor* Descriptor_GetFromClassEntry(zend_class_entry *ce);
 Descriptor* Descriptor_GetFromMessageDef(const upb_msgdef *m);
 Descriptor* Descriptor_GetFromFieldDef(const upb_fielddef *f);
 
+// Packages up a upb_fieldtype_t with a Descriptor, since many functions need
+// both.
+typedef struct {
+  upb_fieldtype_t type;
+  const Descriptor *desc;  // When type == UPB_TYPE_MESSAGE.
+} TypeInfo;
+
+static inline TypeInfo TypeInfo_Get(const upb_fielddef *f) {
+  TypeInfo ret = {upb_fielddef_type(f), Descriptor_GetFromFieldDef(f)};
+  return ret;
+}
+
+static inline TypeInfo TypeInfo_FromType(upb_fieldtype_t type) {
+  TypeInfo ret = {type};
+  return ret;
+}
+
+static inline bool TypeInfo_Eq(TypeInfo a, TypeInfo b) {
+  if (a.type != b.type) return false;
+  if (a.type == UPB_TYPE_MESSAGE && a.desc != b.desc) return false;
+  return true;
+}
+
 #endif  // PHP_PROTOBUF_DEF_H_