From c8e421be29b57334d45fecb333342b9859f75f1d Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Mon, 1 Feb 2021 10:43:31 -0800 Subject: [PATCH] Fixed clone for Message, RepeatedField, and MapField. 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 | 76 +++++++++++-------- php/ext/google/protobuf/array.h | 19 ++--- php/ext/google/protobuf/convert.c | 31 ++++---- php/ext/google/protobuf/convert.h | 11 ++- php/ext/google/protobuf/def.h | 23 ++++++ php/ext/google/protobuf/map.c | 120 ++++++++++++++++++------------ php/ext/google/protobuf/map.h | 15 +++- php/ext/google/protobuf/message.c | 111 +++++++++++++++------------ php/ext/google/protobuf/message.h | 3 +- php/tests/ArrayTest.php | 12 +++ php/tests/GeneratedClassTest.php | 22 ++++++ php/tests/MapFieldTest.php | 18 +++++ 12 files changed, 298 insertions(+), 163 deletions(-) diff --git a/php/ext/google/protobuf/array.c b/php/ext/google/protobuf/array.c index e6412c083258..36ebb8d44f1e 100644 --- a/php/ext/google/protobuf/array.c +++ b/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,15 +162,14 @@ 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); @@ -158,11 +177,9 @@ upb_array *RepeatedField_GetUpbArray(zval *val, const upb_fielddef *f, 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; diff --git a/php/ext/google/protobuf/array.h b/php/ext/google/protobuf/array.h index 921e0bf564f4..031effa71123 100644 --- a/php/ext/google/protobuf/array.h +++ b/php/ext/google/protobuf/array.h @@ -33,6 +33,7 @@ #include +#include "def.h" #include "php-upb.h" // Registers PHP classes for RepeatedField. @@ -40,27 +41,27 @@ 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_ diff --git a/php/ext/google/protobuf/convert.c b/php/ext/google/protobuf/convert.c index 1c2f6288b5a9..c518ccaa4da6 100644 --- a/php/ext/google/protobuf/convert.c +++ b/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); } } diff --git a/php/ext/google/protobuf/convert.h b/php/ext/google/protobuf/convert.h index 06c05c72750c..1bae233425ff 100644 --- a/php/ext/google/protobuf/convert.h +++ b/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); diff --git a/php/ext/google/protobuf/def.h b/php/ext/google/protobuf/def.h index 2a89cd0fd4e6..372c889b31cc 100644 --- a/php/ext/google/protobuf/def.h +++ b/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_ diff --git a/php/ext/google/protobuf/map.c b/php/ext/google/protobuf/map.c index 26776e677e7e..3df3ed345f05 100644 --- a/php/ext/google/protobuf/map.c +++ b/php/ext/google/protobuf/map.c @@ -51,14 +51,31 @@ typedef struct { zend_object std; zval arena; upb_map *map; - upb_fieldtype_t key_type; - upb_fieldtype_t val_type; - const Descriptor* desc; // When values are messages. + MapField_Type type; } MapField; zend_class_entry *MapField_class_entry; static zend_object_handlers MapField_object_handlers; +static bool MapType_Eq(MapField_Type a, MapField_Type b) { + return a.key_type == b.key_type && TypeInfo_Eq(a.val_type, b.val_type); +} + +static TypeInfo KeyType(MapField_Type type) { + TypeInfo ret = {type.key_type}; + return ret; +} + +MapField_Type MapType_Get(const upb_fielddef *f) { + const upb_msgdef *ent = upb_fielddef_msgsubdef(f); + const upb_fielddef *key_f = upb_msgdef_itof(ent, 1); + const upb_fielddef *val_f = upb_msgdef_itof(ent, 2); + MapField_Type type = { + upb_fielddef_type(key_f), + {upb_fielddef_type(val_f), Descriptor_GetFromFieldDef(val_f)}}; + return type; +} + // PHP Object Handlers ///////////////////////////////////////////////////////// /** @@ -102,15 +119,36 @@ static void MapField_destructor(zend_object* obj) { static int MapField_compare_objects(zval *map1, zval *map2) { MapField* intern1 = (MapField*)Z_OBJ_P(map1); MapField* intern2 = (MapField*)Z_OBJ_P(map2); - const upb_msgdef *m = intern1->desc ? intern1->desc->msgdef : NULL; - upb_fieldtype_t key_type = intern1->key_type; - upb_fieldtype_t val_type = intern1->val_type; - if (key_type != intern2->key_type) return 1; - if (val_type != intern2->val_type) return 1; - if (intern1->desc != intern2->desc) return 1; + return MapType_Eq(intern1->type, intern2->type) && + MapEq(intern1->map, intern2->map, intern1->type) + ? 0 + : 1; +} + +/** + * MapField_clone_obj() + * + * Object handler for cloning an object in PHP. Called when PHP code does: + * + * $map2 = clone $map1; + */ +static zend_object *MapField_clone_obj(zval *object) { + MapField* intern = (MapField*)Z_OBJ_P(object); + upb_arena *arena = Arena_Get(&intern->arena); + upb_map *clone = + upb_map_new(arena, intern->type.key_type, intern->type.val_type.type); + size_t iter = UPB_MAP_BEGIN; + + while (upb_mapiter_next(intern->map, &iter)) { + upb_msgval key = upb_mapiter_key(intern->map, iter); + upb_msgval val = upb_mapiter_value(intern->map, iter); + upb_map_set(clone, key, val, arena); + } - return MapEq(intern1->map, intern2->map, key_type, val_type, m) ? 0 : 1; + zval ret; + MapField_GetPhpWrapper(&ret, clone, intern->type, &intern->arena); + return Z_OBJ_P(&ret); } static zval *Map_GetPropertyPtrPtr(PROTO_VAL *object, PROTO_STR *member, @@ -126,7 +164,7 @@ static HashTable *Map_GetProperties(PROTO_VAL *object) { // These are documented in the header file. -void MapField_GetPhpWrapper(zval *val, upb_map *map, const upb_fielddef *f, +void MapField_GetPhpWrapper(zval *val, upb_map *map, MapField_Type type, zval *arena) { if (!map) { ZVAL_NULL(val); @@ -134,37 +172,25 @@ void MapField_GetPhpWrapper(zval *val, upb_map *map, const upb_fielddef *f, } if (!ObjCache_Get(map, val)) { - const upb_msgdef *ent = upb_fielddef_msgsubdef(f); - const upb_fielddef *key_f = upb_msgdef_itof(ent, 1); - const upb_fielddef *val_f = upb_msgdef_itof(ent, 2); MapField *intern = emalloc(sizeof(MapField)); zend_object_std_init(&intern->std, MapField_class_entry); intern->std.handlers = &MapField_object_handlers; ZVAL_COPY(&intern->arena, arena); intern->map = map; - intern->key_type = upb_fielddef_type(key_f); - intern->val_type = upb_fielddef_type(val_f); - intern->desc = Descriptor_GetFromFieldDef(val_f); + intern->type = type; // Skip object_properties_init(), we don't allow derived classes. ObjCache_Add(intern->map, &intern->std); ZVAL_OBJ(val, &intern->std); } } -upb_map *MapField_GetUpbMap(zval *val, const upb_fielddef *f, upb_arena *arena) { - const upb_msgdef *ent = upb_fielddef_msgsubdef(f); - const upb_fielddef *key_f = upb_msgdef_itof(ent, 1); - const upb_fielddef *val_f = upb_msgdef_itof(ent, 2); - upb_fieldtype_t key_type = upb_fielddef_type(key_f); - upb_fieldtype_t val_type = upb_fielddef_type(val_f); - const Descriptor *desc = Descriptor_GetFromFieldDef(val_f); - +upb_map *MapField_GetUpbMap(zval *val, MapField_Type type, upb_arena *arena) { if (Z_ISREF_P(val)) { ZVAL_DEREF(val); } if (Z_TYPE_P(val) == IS_ARRAY) { - upb_map *map = upb_map_new(arena, key_type, val_type); + upb_map *map = upb_map_new(arena, type.key_type, type.val_type.type); HashTable *table = HASH_OF(val); HashPosition pos; @@ -181,8 +207,8 @@ upb_map *MapField_GetUpbMap(zval *val, const upb_fielddef *f, upb_arena *arena) if (!php_val) return map; - if (!Convert_PhpToUpb(&php_key, &upb_key, key_type, NULL, arena) || - !Convert_PhpToUpbAutoWrap(php_val, &upb_val, val_type, desc, arena)) { + if (!Convert_PhpToUpb(&php_key, &upb_key, KeyType(type), arena) || + !Convert_PhpToUpbAutoWrap(php_val, &upb_val, type.val_type, arena)) { return NULL; } @@ -194,8 +220,7 @@ upb_map *MapField_GetUpbMap(zval *val, const upb_fielddef *f, upb_arena *arena) Z_OBJCE_P(val) == MapField_class_entry) { MapField *intern = (MapField*)Z_OBJ_P(val); - if (intern->key_type != key_type || intern->val_type != val_type || - intern->desc != desc) { + if (!MapType_Eq(intern->type, type)) { php_error_docref(NULL, E_USER_ERROR, "Wrong type for this map field."); return NULL; } @@ -208,8 +233,7 @@ upb_map *MapField_GetUpbMap(zval *val, const upb_fielddef *f, upb_arena *arena) } } -bool MapEq(const upb_map *m1, const upb_map *m2, upb_fieldtype_t key_type, - upb_fieldtype_t val_type, const upb_msgdef *m) { +bool MapEq(const upb_map *m1, const upb_map *m2, MapField_Type type) { size_t iter = UPB_MAP_BEGIN; if ((m1 == NULL) != (m2 == NULL)) return false; @@ -222,7 +246,7 @@ bool MapEq(const upb_map *m1, const upb_map *m2, upb_fieldtype_t key_type, upb_msgval val2; if (!upb_map_get(m2, key, &val2)) return false; - if (!ValueEq(val1, val2, val_type, m)) return false; + if (!ValueEq(val1, val2, type.val_type)) return false; } return true; @@ -250,12 +274,12 @@ PHP_METHOD(MapField, __construct) { return; } - intern->key_type = pbphp_dtype_to_type(key_type); - intern->val_type = pbphp_dtype_to_type(val_type); - intern->desc = Descriptor_GetFromClassEntry(klass); + intern->type.key_type = pbphp_dtype_to_type(key_type); + intern->type.val_type.type = pbphp_dtype_to_type(val_type); + intern->type.val_type.desc = Descriptor_GetFromClassEntry(klass); // Check that the key type is an allowed type. - switch (intern->key_type) { + switch (intern->type.key_type) { case UPB_TYPE_INT32: case UPB_TYPE_INT64: case UPB_TYPE_UINT32: @@ -269,13 +293,14 @@ PHP_METHOD(MapField, __construct) { zend_error(E_USER_ERROR, "Invalid key type for map."); } - if (intern->val_type == UPB_TYPE_MESSAGE && klass == NULL) { + if (intern->type.val_type.type == UPB_TYPE_MESSAGE && klass == NULL) { php_error_docref(NULL, E_USER_ERROR, "Message/enum type must have concrete class."); return; } - intern->map = upb_map_new(arena, intern->key_type, intern->val_type); + intern->map = + upb_map_new(arena, intern->type.key_type, intern->type.val_type.type); ObjCache_Add(intern->map, &intern->std); } @@ -296,7 +321,7 @@ PHP_METHOD(MapField, offsetExists) { upb_msgval upb_key; if (zend_parse_parameters(ZEND_NUM_ARGS(), "z", &key) != SUCCESS || - !Convert_PhpToUpb(key, &upb_key, intern->key_type, intern->desc, NULL)) { + !Convert_PhpToUpb(key, &upb_key, KeyType(intern->type), NULL)) { return; } @@ -322,7 +347,7 @@ PHP_METHOD(MapField, offsetGet) { upb_msgval upb_key, upb_val; if (zend_parse_parameters(ZEND_NUM_ARGS(), "z", &key) != SUCCESS || - !Convert_PhpToUpb(key, &upb_key, intern->key_type, intern->desc, NULL)) { + !Convert_PhpToUpb(key, &upb_key, KeyType(intern->type), NULL)) { return; } @@ -331,7 +356,7 @@ PHP_METHOD(MapField, offsetGet) { return; } - Convert_UpbToPhp(upb_val, &ret, intern->val_type, intern->desc, &intern->arena); + Convert_UpbToPhp(upb_val, &ret, intern->type.val_type, &intern->arena); RETURN_ZVAL(&ret, 0, 1); } @@ -355,8 +380,8 @@ PHP_METHOD(MapField, offsetSet) { upb_msgval upb_key, upb_val; if (zend_parse_parameters(ZEND_NUM_ARGS(), "zz", &key, &val) != SUCCESS || - !Convert_PhpToUpb(key, &upb_key, intern->key_type, NULL, NULL) || - !Convert_PhpToUpb(val, &upb_val, intern->val_type, intern->desc, arena)) { + !Convert_PhpToUpb(key, &upb_key, KeyType(intern->type), NULL) || + !Convert_PhpToUpb(val, &upb_val, intern->type.val_type, arena)) { return; } @@ -380,7 +405,7 @@ PHP_METHOD(MapField, offsetUnset) { upb_msgval upb_key; if (zend_parse_parameters(ZEND_NUM_ARGS(), "z", &key) != SUCCESS || - !Convert_PhpToUpb(key, &upb_key, intern->key_type, NULL, NULL)) { + !Convert_PhpToUpb(key, &upb_key, KeyType(intern->type), NULL)) { return; } @@ -543,7 +568,7 @@ PHP_METHOD(MapFieldIter, current) { MapField *field = (MapField*)Z_OBJ_P(&intern->map_field); upb_msgval upb_val = upb_mapiter_value(field->map, intern->position); zval ret; - Convert_UpbToPhp(upb_val, &ret, field->val_type, field->desc, &field->arena); + Convert_UpbToPhp(upb_val, &ret, field->type.val_type, &field->arena); RETURN_ZVAL(&ret, 0, 1); } @@ -557,7 +582,7 @@ PHP_METHOD(MapFieldIter, key) { MapField *field = (MapField*)Z_OBJ_P(&intern->map_field); upb_msgval upb_key = upb_mapiter_key(field->map, intern->position); zval ret; - Convert_UpbToPhp(upb_key, &ret, field->key_type, NULL, NULL); + Convert_UpbToPhp(upb_key, &ret, KeyType(field->type), NULL); RETURN_ZVAL(&ret, 0, 1); } @@ -624,6 +649,7 @@ void Map_ModuleInit() { #else h->compare = MapField_compare_objects; #endif + h->clone_obj = MapField_clone_obj; h->get_properties = Map_GetProperties; h->get_property_ptr_ptr = Map_GetPropertyPtrPtr; diff --git a/php/ext/google/protobuf/map.h b/php/ext/google/protobuf/map.h index 6eb0620c2e06..c523cd0da937 100644 --- a/php/ext/google/protobuf/map.h +++ b/php/ext/google/protobuf/map.h @@ -33,10 +33,18 @@ #include +#include "def.h" #include "php-upb.h" void Map_ModuleInit(); +typedef struct { + upb_fieldtype_t key_type; + TypeInfo val_type; +} MapField_Type; + +MapField_Type MapType_Get(const upb_fielddef *f); + // Gets a upb_map* 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 @@ -47,17 +55,16 @@ void Map_ModuleInit(); // |arena| and add all of the PHP elements to it. // // If an error occurs, we raise a PHP error and return NULL. -upb_map *MapField_GetUpbMap(zval *val, const upb_fielddef *f, upb_arena *arena); +upb_map *MapField_GetUpbMap(zval *val, MapField_Type type, upb_arena *arena); // Creates a PHP MapField object for the given upb_map* and |f| and returns it // in |val|. The PHP object will keep a reference to this |arena| to ensure the // underlying array data stays alive. // // If |map| is NULL, this will return a PHP null object. -void MapField_GetPhpWrapper(zval *val, upb_map *arr, const upb_fielddef *f, +void MapField_GetPhpWrapper(zval *val, upb_map *arr, MapField_Type type, zval *arena); -bool MapEq(const upb_map *m1, const upb_map *m2, upb_fieldtype_t key_type, - upb_fieldtype_t val_type, const upb_msgdef *m); +bool MapEq(const upb_map *m1, const upb_map *m2, MapField_Type type); #endif // PHP_PROTOBUF_MAP_H_ diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index 0e61770eefa1..6a1c059746e3 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -60,6 +60,21 @@ typedef struct { zend_class_entry *message_ce; static zend_object_handlers message_object_handlers; +static void Message_SuppressDefaultProperties(zend_class_entry *class_type) { + // We suppress all default properties, because all our properties are handled + // by our read_property handler. + // + // This also allows us to put our zend_object member at the beginning of our + // struct -- instead of putting it at the end with pointer fixups to access + // our own data, as recommended in the docs -- because Zend won't add any of + // its own storage directly after the zend_object if default_properties_count + // == 0. + // + // This is not officially supported, but since it simplifies the code, we'll + // do it for as long as it works in practice. + class_type->default_properties_count = 0; +} + // PHP Object Handlers ///////////////////////////////////////////////////////// /** @@ -69,8 +84,7 @@ static zend_object_handlers message_object_handlers; */ static zend_object* Message_create(zend_class_entry *class_type) { Message *intern = emalloc(sizeof(Message)); - // XXX(haberman): verify whether we actually want to take this route. - class_type->default_properties_count = 0; + Message_SuppressDefaultProperties(class_type); zend_object_std_init(&intern->std, class_type); intern->std.handlers = &message_object_handlers; Arena_Init(&intern->arena); @@ -114,14 +128,14 @@ static void Message_get(Message *intern, const upb_fielddef *f, zval *rv) { if (upb_fielddef_ismap(f)) { upb_mutmsgval msgval = upb_msg_mutable(intern->msg, f, arena); - MapField_GetPhpWrapper(rv, msgval.map, f, &intern->arena); + MapField_GetPhpWrapper(rv, msgval.map, MapType_Get(f), &intern->arena); } else if (upb_fielddef_isseq(f)) { upb_mutmsgval msgval = upb_msg_mutable(intern->msg, f, arena); - RepeatedField_GetPhpWrapper(rv, msgval.array, f, &intern->arena); + RepeatedField_GetPhpWrapper(rv, msgval.array, TypeInfo_Get(f), + &intern->arena); } else { upb_msgval msgval = upb_msg_get(intern->msg, f); - const Descriptor *subdesc = Descriptor_GetFromFieldDef(f); - Convert_UpbToPhp(msgval, rv, upb_fielddef_type(f), subdesc, &intern->arena); + Convert_UpbToPhp(msgval, rv, TypeInfo_Get(f), &intern->arena); } } @@ -130,16 +144,13 @@ static bool Message_set(Message *intern, const upb_fielddef *f, zval *val) { upb_msgval msgval; if (upb_fielddef_ismap(f)) { - msgval.map_val = MapField_GetUpbMap(val, f, arena); + msgval.map_val = MapField_GetUpbMap(val, MapType_Get(f), arena); if (!msgval.map_val) return false; } else if (upb_fielddef_isseq(f)) { - msgval.array_val = RepeatedField_GetUpbArray(val, f, arena); + msgval.array_val = RepeatedField_GetUpbArray(val, TypeInfo_Get(f), arena); if (!msgval.array_val) return false; } else { - upb_fieldtype_t type = upb_fielddef_type(f); - const Descriptor *subdesc = Descriptor_GetFromFieldDef(f); - bool ok = Convert_PhpToUpb(val, &msgval, type, subdesc, arena); - if (!ok) return false; + if (!Convert_PhpToUpb(val, &msgval, TypeInfo_Get(f), arena)) return false; } upb_msg_set(intern->msg, f, msgval, arena); @@ -149,11 +160,10 @@ static bool Message_set(Message *intern, const upb_fielddef *f, zval *val) { static bool MessageEq(const upb_msg *m1, const upb_msg *m2, const upb_msgdef *m); /** - * ValueEq()() + * ValueEq() */ -bool ValueEq(upb_msgval val1, upb_msgval val2, upb_fieldtype_t type, - const upb_msgdef *m) { - switch (type) { +bool ValueEq(upb_msgval val1, upb_msgval val2, TypeInfo type) { + switch (type.type) { case UPB_TYPE_BOOL: return val1.bool_val == val2.bool_val; case UPB_TYPE_INT32: @@ -172,7 +182,7 @@ bool ValueEq(upb_msgval val1, upb_msgval val2, upb_fieldtype_t type, return val1.str_val.size == val2.str_val.size && memcmp(val1.str_val.data, val2.str_val.data, val1.str_val.size) == 0; case UPB_TYPE_MESSAGE: - return MessageEq(val1.msg_val, val2.msg_val, m); + return MessageEq(val1.msg_val, val2.msg_val, type.desc->msgdef); default: return false; } @@ -190,8 +200,6 @@ static bool MessageEq(const upb_msg *m1, const upb_msg *m2, const upb_msgdef *m) const upb_fielddef *f = upb_msg_iter_field(&i); upb_msgval val1 = upb_msg_get(m1, f); upb_msgval val2 = upb_msg_get(m2, f); - upb_fieldtype_t type = upb_fielddef_type(f); - const upb_msgdef *sub_m = upb_fielddef_msgsubdef(f); if (upb_fielddef_haspresence(f)) { if (upb_msg_has(m1, f) != upb_msg_has(m2, f)) { @@ -201,18 +209,11 @@ static bool MessageEq(const upb_msg *m1, const upb_msg *m2, const upb_msgdef *m) } if (upb_fielddef_ismap(f)) { - const upb_fielddef *key_f = upb_msgdef_itof(sub_m, 1); - const upb_fielddef *val_f = upb_msgdef_itof(sub_m, 2); - upb_fieldtype_t key_type = upb_fielddef_type(key_f); - upb_fieldtype_t val_type = upb_fielddef_type(val_f); - const upb_msgdef *val_m = upb_fielddef_msgsubdef(val_f); - if (!MapEq(val1.map_val, val2.map_val, key_type, val_type, val_m)) { - return false; - } + if (!MapEq(val1.map_val, val2.map_val, MapType_Get(f))) return false; } else if (upb_fielddef_isseq(f)) { - if (!ArrayEq(val1.array_val, val2.array_val, type, sub_m)) return false; + if (!ArrayEq(val1.array_val, val2.array_val, TypeInfo_Get(f))) return false; } else { - if (!ValueEq(val1, val2, type, sub_m)) return false; + if (!ValueEq(val1, val2, TypeInfo_Get(f))) return false; } } @@ -392,6 +393,25 @@ static zval *Message_get_property_ptr_ptr(PROTO_VAL *object, PROTO_STR *member, return NULL; // We do not have a properties table. } +/** + * Message_clone_obj() + * + * Object handler for cloning an object in PHP. Called when PHP code does: + * + * $msg2 = clone $msg; + */ +static zend_object *Message_clone_obj(zval *object) { + Message* intern = PROTO_MSG_P(object); + upb_msg *clone = upb_msg_new(intern->desc->msgdef, Arena_Get(&intern->arena)); + + // TODO: copy unknown fields? + // TODO: use official upb msg copy function + memcpy(clone, intern->msg, upb_msgdef_layout(intern->desc->msgdef)->size); + zval ret; + Message_GetPhpWrapper(&ret, intern->desc, clone, &intern->arena); + return Z_OBJ_P(&ret); +} + /** * Message_get_properties() * @@ -415,8 +435,7 @@ void Message_GetPhpWrapper(zval *val, const Descriptor *desc, upb_msg *msg, if (!ObjCache_Get(msg, val)) { Message *intern = emalloc(sizeof(Message)); - // XXX(haberman): verify whether we actually want to take this route. - desc->class_entry->default_properties_count = 0; + Message_SuppressDefaultProperties(desc->class_entry); zend_object_std_init(&intern->std, desc->class_entry); intern->std.handlers = &message_object_handlers; ZVAL_COPY(&intern->arena, arena); @@ -521,15 +540,13 @@ bool Message_InitFromPhp(upb_msg *msg, const upb_msgdef *m, zval *init, } if (upb_fielddef_ismap(f)) { - msgval.map_val = MapField_GetUpbMap(val, f, arena); + msgval.map_val = MapField_GetUpbMap(val, MapType_Get(f), arena); if (!msgval.map_val) return false; } else if (upb_fielddef_isseq(f)) { - msgval.array_val = RepeatedField_GetUpbArray(val, f, arena); + msgval.array_val = RepeatedField_GetUpbArray(val, TypeInfo_Get(f), arena); if (!msgval.array_val) return false; } else { - const Descriptor *desc = Descriptor_GetFromFieldDef(f); - upb_fieldtype_t type = upb_fielddef_type(f); - if (!Convert_PhpToUpbAutoWrap(val, &msgval, type, desc, arena)) { + if (!Convert_PhpToUpbAutoWrap(val, &msgval, TypeInfo_Get(f), arena)) { return false; } } @@ -805,10 +822,9 @@ PHP_METHOD(Message, readWrapperValue) { const upb_msg *wrapper = upb_msg_get(intern->msg, f).msg_val; const upb_msgdef *m = upb_fielddef_msgsubdef(f); const upb_fielddef *val_f = upb_msgdef_itof(m, 1); - const upb_fieldtype_t val_type = upb_fielddef_type(val_f); upb_msgval msgval = upb_msg_get(wrapper, val_f); zval ret; - Convert_UpbToPhp(msgval, &ret, val_type, NULL, &intern->arena); + Convert_UpbToPhp(msgval, &ret, TypeInfo_Get(val_f), &intern->arena); RETURN_ZVAL(&ret, 1, 0); } else { RETURN_NULL(); @@ -861,10 +877,9 @@ PHP_METHOD(Message, writeWrapperValue) { } else { const upb_msgdef *m = upb_fielddef_msgsubdef(f); const upb_fielddef *val_f = upb_msgdef_itof(m, 1); - upb_fieldtype_t val_type = upb_fielddef_type(val_f); upb_msg *wrapper; - if (!Convert_PhpToUpb(val, &msgval, val_type, NULL, arena)) { + if (!Convert_PhpToUpb(val, &msgval, TypeInfo_Get(val_f), arena)) { return; // Error is already set. } @@ -974,9 +989,7 @@ PHP_METHOD(Message, readOneof) { { upb_msgval msgval = upb_msg_get(intern->msg, f); - const Descriptor *subdesc = Descriptor_GetFromFieldDef(f); - Convert_UpbToPhp(msgval, &ret, upb_fielddef_type(f), subdesc, - &intern->arena); + Convert_UpbToPhp(msgval, &ret, TypeInfo_Get(f), &intern->arena); } RETURN_ZVAL(&ret, 1, 0); @@ -1017,8 +1030,7 @@ PHP_METHOD(Message, writeOneof) { f = upb_msgdef_itof(intern->desc->msgdef, field_num); - if (!Convert_PhpToUpb(val, &msgval, upb_fielddef_type(f), - Descriptor_GetFromFieldDef(f), arena)) { + if (!Convert_PhpToUpb(val, &msgval, TypeInfo_Get(f), arena)) { return; } @@ -1225,8 +1237,8 @@ PHP_METHOD(google_protobuf_Timestamp, fromDateTime) { if (call_user_function(EG(function_table), NULL, &function_name, &retval, 1, datetime) == FAILURE || - !Convert_PhpToUpb(&retval, ×tamp_seconds, UPB_TYPE_INT64, NULL, - NULL)) { + !Convert_PhpToUpb(&retval, ×tamp_seconds, + TypeInfo_FromType(UPB_TYPE_INT64), NULL)) { zend_error(E_ERROR, "Cannot get timestamp from DateTime."); return; } @@ -1251,8 +1263,8 @@ PHP_METHOD(google_protobuf_Timestamp, fromDateTime) { if (call_user_function(EG(function_table), NULL, &function_name, &retval, 2, params) == FAILURE || - !Convert_PhpToUpb(&retval, ×tamp_nanos, UPB_TYPE_INT32, NULL, - NULL)) { + !Convert_PhpToUpb(&retval, ×tamp_nanos, + TypeInfo_FromType(UPB_TYPE_INT32), NULL)) { zend_error(E_ERROR, "Cannot format DateTime."); return; } @@ -1338,6 +1350,7 @@ void Message_ModuleInit() { h->unset_property = Message_unset_property; h->get_properties = Message_get_properties; h->get_property_ptr_ptr = Message_get_property_ptr_ptr; + h->clone_obj = Message_clone_obj; WellKnownTypes_ModuleInit(); /* From wkt.inc. */ } diff --git a/php/ext/google/protobuf/message.h b/php/ext/google/protobuf/message.h index 0e6fb5a4fcbe..5b49e0db84db 100644 --- a/php/ext/google/protobuf/message.h +++ b/php/ext/google/protobuf/message.h @@ -56,7 +56,6 @@ bool Message_GetUpbMessage(zval *val, const Descriptor *desc, upb_arena *arena, void Message_GetPhpWrapper(zval *val, const Descriptor *desc, upb_msg *msg, zval *arena); -bool ValueEq(upb_msgval val1, upb_msgval val2, upb_fieldtype_t type, - const upb_msgdef *m); +bool ValueEq(upb_msgval val1, upb_msgval val2, TypeInfo type); #endif // PHP_PROTOBUF_MESSAGE_H_ diff --git a/php/tests/ArrayTest.php b/php/tests/ArrayTest.php index 269d11d3a2f0..0585ca5b1c8f 100644 --- a/php/tests/ArrayTest.php +++ b/php/tests/ArrayTest.php @@ -624,4 +624,16 @@ public function testEquality() new RepeatedField(GPBType::MESSAGE, TestMessage::class) == new RepeatedField(GPBType::MESSAGE, Sub::class)); } + + ######################################################### + # Test clone + ######################################################### + + public function testClone() + { + $arr = new RepeatedField(GPBType::MESSAGE, TestMessage::class); + $arr[] = new TestMessage; + $arr2 = clone $arr; + $this->assertSame($arr[0], $arr2[0]); + } } diff --git a/php/tests/GeneratedClassTest.php b/php/tests/GeneratedClassTest.php index c0de0ba9b26b..bedf4408e9d2 100644 --- a/php/tests/GeneratedClassTest.php +++ b/php/tests/GeneratedClassTest.php @@ -1518,6 +1518,28 @@ public function testOneofStringInArrayConstructor() $this->assertTrue(true); } + ######################################################### + # Test clone. + ######################################################### + + public function testClone() + { + $m = new TestMessage([ + 'optional_int32' => -42, + 'optional_int64' => -43, + 'optional_message' => new Sub([ + 'a' => 33 + ]), + 'map_int32_message' => [1 => new Sub(['a' => 36])], + ]); + $m2 = clone $m; + $this->assertEquals($m->getOptionalInt32(), $m2->getOptionalInt32()); + $this->assertEquals($m->getOptionalInt64(), $m2->getOptionalInt64()); + $this->assertSame($m->getOptionalMessage(), $m2->getOptionalMessage()); + $this->assertSame($m->getMapInt32Message()[1], $m2->getMapInt32Message()[1]); + $this->assertEquals($m->serializeToJsonString(), $m2->serializeToJsonString()); + } + ######################################################### # Test message equals. ######################################################### diff --git a/php/tests/MapFieldTest.php b/php/tests/MapFieldTest.php index a96f24fb580d..2d8ae61c7bf4 100644 --- a/php/tests/MapFieldTest.php +++ b/php/tests/MapFieldTest.php @@ -511,6 +511,24 @@ public function testEquality() new MapField(GPBType::INT32, GPBType::MESSAGE, Sub::class)); } + ######################################################### + # Test clone + ######################################################### + + public function testClone() + { + $map = new MapField(GPBType::INT32, + GPBType::MESSAGE, Sub::class); + + // Test append. + $sub_m = new Sub(); + $sub_m->setA(1); + $map[0] = $sub_m; + + $map2 = clone $map; + $this->assertSame($map[0], $map2[0]); + } + ######################################################### # Test memory leak #########################################################