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.

Sign up for GitHub

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

Jump to bottom

UnPackTo disable merge by default #7527

Merged
merged 4 commits into from Sep 14, 2022
Merged

UnPackTo disable merge by default #7527

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
45bad5f
UnPackTo disable merge by default
dbaileychess Sep 13, 2022
e620dab
avoid double free in test
dbaileychess Sep 13, 2022
408d884
remove merge parameter
dbaileychess Sep 14, 2022
dcea716
remove shrink to fit
dbaileychess Sep 14, 2022
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
16 changes: 9 additions & 7 deletions samples/monster_generated.h
View file
Open in desktop
Expand Up @@ -352,7 +352,7 @@ struct Monster FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table {
verifier.EndTable();
}
MonsterT *UnPack(const flatbuffers::resolver_function_t *_resolver = nullptr) const;
void UnPackTo(MonsterT *_o, const flatbuffers::resolver_function_t *_resolver = nullptr) const;
void UnPackTo(MonsterT *_o, const flatbuffers::resolver_function_t *_resolver = nullptr, bool _merge = false) const;
static flatbuffers::Offset<Monster> Pack(flatbuffers::FlatBufferBuilder &_fbb, const MonsterT* _o, const flatbuffers::rehasher_function_t *_rehasher = nullptr);
};

Expand Down Expand Up @@ -499,7 +499,7 @@ struct Weapon FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table {
verifier.EndTable();
}
WeaponT *UnPack(const flatbuffers::resolver_function_t *_resolver = nullptr) const;
void UnPackTo(WeaponT *_o, const flatbuffers::resolver_function_t *_resolver = nullptr) const;
void UnPackTo(WeaponT *_o, const flatbuffers::resolver_function_t *_resolver = nullptr, bool _merge = false) const;
static flatbuffers::Offset<Weapon> Pack(flatbuffers::FlatBufferBuilder &_fbb, const WeaponT* _o, const flatbuffers::rehasher_function_t *_rehasher = nullptr);
};

Expand Down Expand Up @@ -598,19 +598,20 @@ inline MonsterT *Monster::UnPack(const flatbuffers::resolver_function_t *_resolv
return _o.release();
}

inline void Monster::UnPackTo(MonsterT *_o, const flatbuffers::resolver_function_t *_resolver) const {
inline void Monster::UnPackTo(MonsterT *_o, const flatbuffers::resolver_function_t *_resolver, bool _merge) const {
(void)_o;
(void)_resolver;
(void)_merge;
{ auto _e = pos(); if (_e) _o->pos = flatbuffers::unique_ptr<MyGame::Sample::Vec3>(new MyGame::Sample::Vec3(*_e)); }
{ auto _e = mana(); _o->mana = _e; }
{ auto _e = hp(); _o->hp = _e; }
{ auto _e = name(); if (_e) _o->name = _e->str(); }
{ auto _e = inventory(); if (_e) { _o->inventory.resize(_e->size()); std::copy(_e->begin(), _e->end(), _o->inventory.begin()); } }
{ auto _e = inventory(); if (_e) {_o->inventory.resize(_e->size()); std::copy(_e->begin(), _e->end(), _o->inventory.begin()); } }
{ auto _e = color(); _o->color = _e; }
{ auto _e = weapons(); if (_e) { _o->weapons.resize(_e->size()); for (flatbuffers::uoffset_t _i = 0; _i < _e->size(); _i++) { _o->weapons[_i] = flatbuffers::unique_ptr<MyGame::Sample::WeaponT>(_e->Get(_i)->UnPack(_resolver)); } } }
{ auto _e = weapons(); if (_e) {_o->weapons.resize(_e->size()); for (flatbuffers::uoffset_t _i = 0; _i < _e->size(); _i++) { if(_o->weapons[_i]) { _e->Get(_i)->UnPackTo(_o->weapons[_i].get(), _resolver, _merge); } else { _o->weapons[_i] = flatbuffers::unique_ptr<MyGame::Sample::WeaponT>(_e->Get(_i)->UnPack(_resolver)); }; } } else if(!_merge) { _o->weapons.resize(0); _o->weapons.shrink_to_fit(); } }
{ auto _e = equipped_type(); _o->equipped.type = _e; }
{ auto _e = equipped(); if (_e) _o->equipped.value = MyGame::Sample::EquipmentUnion::UnPack(_e, equipped_type(), _resolver); }
{ auto _e = path(); if (_e) { _o->path.resize(_e->size()); for (flatbuffers::uoffset_t _i = 0; _i < _e->size(); _i++) { _o->path[_i] = *_e->Get(_i); } } }
{ auto _e = path(); if (_e) {_o->path.resize(_e->size()); for (flatbuffers::uoffset_t _i = 0; _i < _e->size(); _i++) { _o->path[_i] = *_e->Get(_i); } } else if(!_merge) { _o->path.resize(0); _o->path.shrink_to_fit(); } }
}

inline flatbuffers::Offset<Monster> Monster::Pack(flatbuffers::FlatBufferBuilder &_fbb, const MonsterT* _o, const flatbuffers::rehasher_function_t *_rehasher) {
Expand Down Expand Up @@ -663,9 +664,10 @@ inline WeaponT *Weapon::UnPack(const flatbuffers::resolver_function_t *_resolver
return _o.release();
}

inline void Weapon::UnPackTo(WeaponT *_o, const flatbuffers::resolver_function_t *_resolver) const {
inline void Weapon::UnPackTo(WeaponT *_o, const flatbuffers::resolver_function_t *_resolver, bool _merge) const {
(void)_o;
(void)_resolver;
(void)_merge;
{ auto _e = name(); if (_e) _o->name = _e->str(); }
{ auto _e = damage(); _o->damage = _e; }
}
Expand Down
68 changes: 34 additions & 34 deletions src/idl_gen_cpp.cpp
View file
Open in desktop
Expand Up @@ -26,10 +26,6 @@
#include "flatbuffers/idl.h"
#include "flatbuffers/util.h"

#ifndef FLATBUFFERS_CPP_OBJECT_UNPACKTO
#define FLATBUFFERS_CPP_OBJECT_UNPACKTO 0
#endif

namespace flatbuffers {

// Make numerical literal with type-suffix.
Expand Down Expand Up @@ -74,13 +70,17 @@ static std::string GenIncludeGuard(const std::string &file_name,
return guard;
}

static bool IsVectorOfPointers(const FieldDef& field) {
const auto& type = field.value.type;
const auto& vector_type = type.VectorType();
static bool IsVectorOfPointers(const FieldDef &field) {
const auto &type = field.value.type;
const auto &vector_type = type.VectorType();
return type.base_type == BASE_TYPE_VECTOR &&
vector_type.base_type == BASE_TYPE_STRUCT &&
!vector_type.struct_def->fixed &&
!field.native_inline;
!vector_type.struct_def->fixed && !field.native_inline;
}

static bool IsPointer(const FieldDef &field) {
return field.value.type.base_type == BASE_TYPE_STRUCT &&
!IsStruct(field.value.type);
}

namespace cpp {
Expand Down Expand Up @@ -1065,7 +1065,8 @@ class CppGenerator : public BaseGenerator {
return "void " + (inclass ? "" : Name(struct_def) + "::") + "UnPackTo(" +
NativeName(Name(struct_def), &struct_def, opts) + " *" +
"_o, const flatbuffers::resolver_function_t *_resolver" +
(inclass ? " = nullptr" : "") + ") const";
(inclass ? " = nullptr" : "") + ", bool _merge" +
(inclass ? " = false" : "") + ") const";
}

void GenMiniReflectPre(const StructDef *struct_def) {
Expand Down Expand Up @@ -1882,7 +1883,8 @@ class CppGenerator : public BaseGenerator {
if (vec_type.base_type == BASE_TYPE_UTYPE) continue;
const auto cpp_type = field.attributes.Lookup("cpp_type");
const auto cpp_ptr_type = field.attributes.Lookup("cpp_ptr_type");
const bool is_ptr = IsVectorOfPointers(field) || (cpp_type && cpp_ptr_type->constant != "naked");
const bool is_ptr = IsVectorOfPointers(field) ||
(cpp_type && cpp_ptr_type->constant != "naked");
if (is_ptr) { return true; }
}
}
Expand Down Expand Up @@ -2006,7 +2008,8 @@ class CppGenerator : public BaseGenerator {
? cpp_type->constant
: GenTypeNative(vec_type, /*invector*/ true,
field, /*forcopy*/ true);
const bool is_ptr = IsVectorOfPointers(field) || (cpp_type && cpp_ptr_type->constant != "naked");
const bool is_ptr = IsVectorOfPointers(field) ||
(cpp_type && cpp_ptr_type->constant != "naked");
CodeWriter cw(" ");
cw.SetValue("FIELD", Name(field));
cw.SetValue("TYPE", type_name);
Expand Down Expand Up @@ -3013,7 +3016,8 @@ class CppGenerator : public BaseGenerator {
if (field.value.type.element == BASE_TYPE_UTYPE) {
name = StripUnionType(Name(field));
}
code += "{ _o->" + name + ".resize(_e->size()); ";
const std::string vector_field = "_o->" + name;
code += "{" + vector_field + ".resize(_e->size()); ";
if (!field.value.type.enum_def && !IsBool(field.value.type.element) &&
IsOneByte(field.value.type.element)) {
// For vectors of bytes, std::copy is used to improve performance.
Expand Down Expand Up @@ -3053,7 +3057,7 @@ class CppGenerator : public BaseGenerator {
// (*resolver)(&_o->field, (hash_value_t)(_e));
// else
// _o->field = nullptr;
code += "//vector resolver, " + PtrType(&field) + "\n";
code += "/*vector resolver, " + PtrType(&field) + "*/ ";
code += "if (_resolver) ";
code += "(*_resolver)";
code += "(reinterpret_cast<void **>(&_o->" + name + "[_i]" +
Expand All @@ -3070,25 +3074,20 @@ class CppGenerator : public BaseGenerator {
code += "/* else do nothing */";
}
} else {
// clang-format off
#if FLATBUFFERS_CPP_OBJECT_UNPACKTO
const bool is_pointer = IsVectorOfPointers(field);
if (is_pointer) {
code += "if(_o->" + name + "[_i]" + ") { ";
code += indexing + "->UnPackTo(_o->" + name +
"[_i].get(), _resolver);";
"[_i].get(), _resolver, _merge);";
code += " } else { ";
}
#endif
code += "_o->" + name + "[_i]" + access + " = ";
code += GenUnpackVal(field.value.type.VectorType(), indexing, true,
field);
#if FLATBUFFERS_CPP_OBJECT_UNPACKTO
if (is_pointer) { code += "; }"; }
#endif
// clang-format on
}
code += "; } }";
code += "; } } else if(!_merge) { " + vector_field + ".resize(0); " +
vector_field + ".shrink_to_fit(); }";
}
break;
}
Expand Down Expand Up @@ -3116,7 +3115,7 @@ class CppGenerator : public BaseGenerator {
// (*resolver)(&_o->field, (hash_value_t)(_e));
// else
// _o->field = nullptr;
code += "//scalar resolver, " + PtrType(&field) + " \n";
code += "/*scalar resolver, " + PtrType(&field) + "*/ ";
code += "if (_resolver) ";
code += "(*_resolver)";
code += "(reinterpret_cast<void **>(&_o->" + Name(field) + "), ";
Expand All @@ -3133,21 +3132,21 @@ class CppGenerator : public BaseGenerator {
} else {
// Generate code for assigning the value, of the form:
// _o->field = value;
// clang-format off
#if FLATBUFFERS_CPP_OBJECT_UNPACKTO
const bool is_pointer = IsVectorOfPointers(field);
const bool is_pointer = IsPointer(field);

const std::string out_field = "_o->" + Name(field);

if (is_pointer) {
code += "{ if(_o->" + Name(field) + ") { ";
code += "_e->UnPackTo(_o->" + Name(field) + ".get(), _resolver);";
code += "{ if(" + out_field + ") { ";
code += "_e->UnPackTo(" + out_field + ".get(), _resolver, _merge);";
code += " } else { ";
}
#endif
code += "_o->" + Name(field) + " = ";
code += out_field + " = ";
code += GenUnpackVal(field.value.type, "_e", false, field) + ";";
#if FLATBUFFERS_CPP_OBJECT_UNPACKTO
if (is_pointer) { code += " } }"; }
#endif
// clang-format on
if (is_pointer) {
code += " } } else if (!_merge && " + out_field + ") {" +
out_field + ".reset(); }";
}
}
break;
}
Expand Down Expand Up @@ -3399,6 +3398,7 @@ class CppGenerator : public BaseGenerator {
"inline " + TableUnPackToSignature(struct_def, false, opts_) + " {";
code_ += " (void)_o;";
code_ += " (void)_resolver;";
code_ += " (void)_merge;";

for (auto it = struct_def.fields.vec.begin();
it != struct_def.fields.vec.end(); ++it) {
Expand Down
5 changes: 3 additions & 2 deletions tests/arrays_test_generated.h
View file
Open in desktop
Expand Up @@ -299,7 +299,7 @@ struct ArrayTable FLATBUFFERS_FINAL_CLASS : private flatbuffers::Table {
verifier.EndTable();
}
ArrayTableT *UnPack(const flatbuffers::resolver_function_t *_resolver = nullptr) const;
void UnPackTo(ArrayTableT *_o, const flatbuffers::resolver_function_t *_resolver = nullptr) const;
void UnPackTo(ArrayTableT *_o, const flatbuffers::resolver_function_t *_resolver = nullptr, bool _merge = false) const;
static flatbuffers::Offset<ArrayTable> Pack(flatbuffers::FlatBufferBuilder &_fbb, const ArrayTableT* _o, const flatbuffers::rehasher_function_t *_rehasher = nullptr);
};

Expand Down Expand Up @@ -357,9 +357,10 @@ inline ArrayTableT *ArrayTable::UnPack(const flatbuffers::resolver_function_t *_
return _o.release();
}

inline void ArrayTable::UnPackTo(ArrayTableT *_o, const flatbuffers::resolver_function_t *_resolver) const {
inline void ArrayTable::UnPackTo(ArrayTableT *_o, const flatbuffers::resolver_function_t *_resolver, bool _merge) const {
(void)_o;
(void)_resolver;
(void)_merge;
{ auto _e = a(); if (_e) _o->a = flatbuffers::unique_ptr<MyGame::Example::ArrayStruct>(new MyGame::Example::ArrayStruct(*_e)); }
}

Expand Down