Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Feb 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
76 changes: 46 additions & 30 deletions php/ext/google/protobuf/array.c
Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
Expand All @@ -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);

Expand All @@ -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;
}

Expand All @@ -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.");
}
Expand All @@ -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;

Expand All @@ -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;
Expand All @@ -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);
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;

Expand Down
19 changes: 10 additions & 9 deletions php/ext/google/protobuf/array.h
Expand Up @@ -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_
31 changes: 15 additions & 16 deletions php/ext/google/protobuf/convert.c
Expand Up @@ -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:
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
}

Expand Down
11 changes: 5 additions & 6 deletions php/ext/google/protobuf/convert.h
Expand Up @@ -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:
Expand All @@ -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);
Expand Down
23 changes: 23 additions & 0 deletions php/ext/google/protobuf/def.h
Expand Up @@ -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_