From 5797540ed03a048f8db0ced443de0fef2843c2ed Mon Sep 17 00:00:00 2001 From: tira-misu Date: Thu, 17 Oct 2019 23:22:21 +0200 Subject: [PATCH] #5544 Fix of Array of table is not sorted if CreateDirect() is used (#5546) * Fix C/C++ CreateDirect with sorted vectors If a struct has a key the vector has to be sorted. To sort the vector you can't use "const". * Changes due to code review * Improve code readability --- include/flatbuffers/reflection_generated.h | 44 +++++++++++----------- src/idl_gen_cpp.cpp | 33 +++++++++++++--- tests/monster_test_generated.h | 16 ++++---- 3 files changed, 57 insertions(+), 36 deletions(-) diff --git a/include/flatbuffers/reflection_generated.h b/include/flatbuffers/reflection_generated.h index 0e73a0fee..77a2c7f90 100644 --- a/include/flatbuffers/reflection_generated.h +++ b/include/flatbuffers/reflection_generated.h @@ -463,14 +463,14 @@ inline flatbuffers::Offset CreateEnum( inline flatbuffers::Offset CreateEnumDirect( flatbuffers::FlatBufferBuilder &_fbb, const char *name = nullptr, - const std::vector> *values = nullptr, + std::vector> *values = nullptr, bool is_union = false, flatbuffers::Offset underlying_type = 0, - const std::vector> *attributes = nullptr, + std::vector> *attributes = nullptr, const std::vector> *documentation = nullptr) { auto name__ = name ? _fbb.CreateString(name) : 0; - auto values__ = values ? _fbb.CreateVector>(*values) : 0; - auto attributes__ = attributes ? _fbb.CreateVector>(*attributes) : 0; + auto values__ = values ? _fbb.CreateVectorOfSortedTables(values) : 0; + auto attributes__ = attributes ? _fbb.CreateVectorOfSortedTables(attributes) : 0; auto documentation__ = documentation ? _fbb.CreateVector>(*documentation) : 0; return reflection::CreateEnum( _fbb, @@ -647,10 +647,10 @@ inline flatbuffers::Offset CreateFieldDirect( bool deprecated = false, bool required = false, bool key = false, - const std::vector> *attributes = nullptr, + std::vector> *attributes = nullptr, const std::vector> *documentation = nullptr) { auto name__ = name ? _fbb.CreateString(name) : 0; - auto attributes__ = attributes ? _fbb.CreateVector>(*attributes) : 0; + auto attributes__ = attributes ? _fbb.CreateVectorOfSortedTables(attributes) : 0; auto documentation__ = documentation ? _fbb.CreateVector>(*documentation) : 0; return reflection::CreateField( _fbb, @@ -785,15 +785,15 @@ inline flatbuffers::Offset CreateObject( inline flatbuffers::Offset CreateObjectDirect( flatbuffers::FlatBufferBuilder &_fbb, const char *name = nullptr, - const std::vector> *fields = nullptr, + std::vector> *fields = nullptr, bool is_struct = false, int32_t minalign = 0, int32_t bytesize = 0, - const std::vector> *attributes = nullptr, + std::vector> *attributes = nullptr, const std::vector> *documentation = nullptr) { auto name__ = name ? _fbb.CreateString(name) : 0; - auto fields__ = fields ? _fbb.CreateVector>(*fields) : 0; - auto attributes__ = attributes ? _fbb.CreateVector>(*attributes) : 0; + auto fields__ = fields ? _fbb.CreateVectorOfSortedTables(fields) : 0; + auto attributes__ = attributes ? _fbb.CreateVectorOfSortedTables(attributes) : 0; auto documentation__ = documentation ? _fbb.CreateVector>(*documentation) : 0; return reflection::CreateObject( _fbb, @@ -907,10 +907,10 @@ inline flatbuffers::Offset CreateRPCCallDirect( const char *name = nullptr, flatbuffers::Offset request = 0, flatbuffers::Offset response = 0, - const std::vector> *attributes = nullptr, + std::vector> *attributes = nullptr, const std::vector> *documentation = nullptr) { auto name__ = name ? _fbb.CreateString(name) : 0; - auto attributes__ = attributes ? _fbb.CreateVector>(*attributes) : 0; + auto attributes__ = attributes ? _fbb.CreateVectorOfSortedTables(attributes) : 0; auto documentation__ = documentation ? _fbb.CreateVector>(*documentation) : 0; return reflection::CreateRPCCall( _fbb, @@ -1008,12 +1008,12 @@ inline flatbuffers::Offset CreateService( inline flatbuffers::Offset CreateServiceDirect( flatbuffers::FlatBufferBuilder &_fbb, const char *name = nullptr, - const std::vector> *calls = nullptr, - const std::vector> *attributes = nullptr, + std::vector> *calls = nullptr, + std::vector> *attributes = nullptr, const std::vector> *documentation = nullptr) { auto name__ = name ? _fbb.CreateString(name) : 0; - auto calls__ = calls ? _fbb.CreateVector>(*calls) : 0; - auto attributes__ = attributes ? _fbb.CreateVector>(*attributes) : 0; + auto calls__ = calls ? _fbb.CreateVectorOfSortedTables(calls) : 0; + auto attributes__ = attributes ? _fbb.CreateVectorOfSortedTables(attributes) : 0; auto documentation__ = documentation ? _fbb.CreateVector>(*documentation) : 0; return reflection::CreateService( _fbb, @@ -1126,17 +1126,17 @@ inline flatbuffers::Offset CreateSchema( inline flatbuffers::Offset CreateSchemaDirect( flatbuffers::FlatBufferBuilder &_fbb, - const std::vector> *objects = nullptr, - const std::vector> *enums = nullptr, + std::vector> *objects = nullptr, + std::vector> *enums = nullptr, const char *file_ident = nullptr, const char *file_ext = nullptr, flatbuffers::Offset root_table = 0, - const std::vector> *services = nullptr) { - auto objects__ = objects ? _fbb.CreateVector>(*objects) : 0; - auto enums__ = enums ? _fbb.CreateVector>(*enums) : 0; + std::vector> *services = nullptr) { + auto objects__ = objects ? _fbb.CreateVectorOfSortedTables(objects) : 0; + auto enums__ = enums ? _fbb.CreateVectorOfSortedTables(enums) : 0; auto file_ident__ = file_ident ? _fbb.CreateString(file_ident) : 0; auto file_ext__ = file_ext ? _fbb.CreateString(file_ext) : 0; - auto services__ = services ? _fbb.CreateVector>(*services) : 0; + auto services__ = services ? _fbb.CreateVectorOfSortedTables(services) : 0; return reflection::CreateSchema( _fbb, objects__, diff --git a/src/idl_gen_cpp.cpp b/src/idl_gen_cpp.cpp index 1efb057c3..505fdaa59 100644 --- a/src/idl_gen_cpp.cpp +++ b/src/idl_gen_cpp.cpp @@ -205,7 +205,7 @@ class CppGenerator : public BaseGenerator { } void GenExtraIncludes() { - for(std::size_t i = 0; i < parser_.opts.cpp_includes.size(); ++i) { + for (std::size_t i = 0; i < parser_.opts.cpp_includes.size(); ++i) { code_ += "#include \"" + parser_.opts.cpp_includes[i] + "\""; } if (!parser_.opts.cpp_includes.empty()) { @@ -278,7 +278,7 @@ class CppGenerator : public BaseGenerator { code_ += "bool operator==(const " + nativeName + " &lhs, const " + nativeName + " &rhs);"; code_ += "bool operator!=(const " + nativeName + " &lhs, const " + - nativeName + " &rhs);"; + nativeName + " &rhs);"; } } code_ += ""; @@ -527,6 +527,16 @@ class CppGenerator : public BaseGenerator { return cpp_qualified_name; } + bool TypeHasKey(const Type &type) { + if (type.base_type != BASE_TYPE_STRUCT) { return false; } + for (auto it = type.struct_def->fields.vec.begin(); + it != type.struct_def->fields.vec.end(); ++it) { + const auto &field = **it; + if (field.key) { return true; } + } + return false; + } + void GenComment(const std::vector &dc, const char *prefix = "") { std::string text; ::flatbuffers::GenComment(dc, &text, nullptr, prefix); @@ -1267,7 +1277,7 @@ class CppGenerator : public BaseGenerator { if (ev.union_type.base_type == BASE_TYPE_STRUCT) { if (ev.union_type.struct_def->fixed) { code_ += " return verifier.Verify<{{TYPE}}>(static_cast(obj), 0);"; + "uint8_t *>(obj), 0);"; } else { code_ += getptr; code_ += " return verifier.VerifyTable(ptr);"; @@ -1528,7 +1538,11 @@ class CppGenerator : public BaseGenerator { } else { type = GenTypeWire(vtype, "", false); } - code_.SetValue("PARAM_TYPE", "const std::vector<" + type + "> *"); + if (TypeHasKey(vtype)) { + code_.SetValue("PARAM_TYPE", "std::vector<" + type + "> *"); + } else { + code_.SetValue("PARAM_TYPE", "const std::vector<" + type + "> *"); + } code_.SetValue("PARAM_VALUE", "nullptr"); } else { code_.SetValue("PARAM_TYPE", GenTypeWire(field.value.type, " ", true)); @@ -2198,14 +2212,21 @@ class CppGenerator : public BaseGenerator { } else if (field.value.type.base_type == BASE_TYPE_VECTOR) { code_ += " auto {{FIELD_NAME}}__ = {{FIELD_NAME}} ? \\"; const auto vtype = field.value.type.VectorType(); + const auto has_key = TypeHasKey(vtype); if (IsStruct(vtype)) { const auto type = WrapInNameSpace(*vtype.struct_def); - code_ += "_fbb.CreateVectorOfStructs<" + type + ">\\"; + code_ += (has_key ? "_fbb.CreateVectorOfSortedStructs<" + : "_fbb.CreateVectorOfStructs<") + + type + ">\\"; + } else if (has_key) { + const auto type = WrapInNameSpace(*vtype.struct_def); + code_ += "_fbb.CreateVectorOfSortedTables<" + type + ">\\"; } else { const auto type = GenTypeWire(vtype, "", false); code_ += "_fbb.CreateVector<" + type + ">\\"; } - code_ += "(*{{FIELD_NAME}}) : 0;"; + code_ += + has_key ? "({{FIELD_NAME}}) : 0;" : "(*{{FIELD_NAME}}) : 0;"; } } } diff --git a/tests/monster_test_generated.h b/tests/monster_test_generated.h index 1f05b3acd..d1b5c38de 100644 --- a/tests/monster_test_generated.h +++ b/tests/monster_test_generated.h @@ -2024,7 +2024,7 @@ inline flatbuffers::Offset CreateMonsterDirect( flatbuffers::Offset test = 0, const std::vector *test4 = nullptr, const std::vector> *testarrayofstring = nullptr, - const std::vector> *testarrayoftables = nullptr, + std::vector> *testarrayoftables = nullptr, flatbuffers::Offset enemy = 0, const std::vector *testnestedflatbuffer = nullptr, flatbuffers::Offset testempty = 0, @@ -2042,16 +2042,16 @@ inline flatbuffers::Offset CreateMonsterDirect( float testf2 = 3.0f, float testf3 = 0.0f, const std::vector> *testarrayofstring2 = nullptr, - const std::vector *testarrayofsortedstruct = nullptr, + std::vector *testarrayofsortedstruct = nullptr, const std::vector *flex = nullptr, const std::vector *test5 = nullptr, const std::vector *vector_of_longs = nullptr, const std::vector *vector_of_doubles = nullptr, flatbuffers::Offset parent_namespace_test = 0, - const std::vector> *vector_of_referrables = nullptr, + std::vector> *vector_of_referrables = nullptr, uint64_t single_weak_reference = 0, const std::vector *vector_of_weak_references = nullptr, - const std::vector> *vector_of_strong_referrables = nullptr, + std::vector> *vector_of_strong_referrables = nullptr, uint64_t co_owning_reference = 0, const std::vector *vector_of_co_owning_references = nullptr, uint64_t non_owning_reference = 0, @@ -2066,18 +2066,18 @@ inline flatbuffers::Offset CreateMonsterDirect( auto inventory__ = inventory ? _fbb.CreateVector(*inventory) : 0; auto test4__ = test4 ? _fbb.CreateVectorOfStructs(*test4) : 0; auto testarrayofstring__ = testarrayofstring ? _fbb.CreateVector>(*testarrayofstring) : 0; - auto testarrayoftables__ = testarrayoftables ? _fbb.CreateVector>(*testarrayoftables) : 0; + auto testarrayoftables__ = testarrayoftables ? _fbb.CreateVectorOfSortedTables(testarrayoftables) : 0; auto testnestedflatbuffer__ = testnestedflatbuffer ? _fbb.CreateVector(*testnestedflatbuffer) : 0; auto testarrayofbools__ = testarrayofbools ? _fbb.CreateVector(*testarrayofbools) : 0; auto testarrayofstring2__ = testarrayofstring2 ? _fbb.CreateVector>(*testarrayofstring2) : 0; - auto testarrayofsortedstruct__ = testarrayofsortedstruct ? _fbb.CreateVectorOfStructs(*testarrayofsortedstruct) : 0; + auto testarrayofsortedstruct__ = testarrayofsortedstruct ? _fbb.CreateVectorOfSortedStructs(testarrayofsortedstruct) : 0; auto flex__ = flex ? _fbb.CreateVector(*flex) : 0; auto test5__ = test5 ? _fbb.CreateVectorOfStructs(*test5) : 0; auto vector_of_longs__ = vector_of_longs ? _fbb.CreateVector(*vector_of_longs) : 0; auto vector_of_doubles__ = vector_of_doubles ? _fbb.CreateVector(*vector_of_doubles) : 0; - auto vector_of_referrables__ = vector_of_referrables ? _fbb.CreateVector>(*vector_of_referrables) : 0; + auto vector_of_referrables__ = vector_of_referrables ? _fbb.CreateVectorOfSortedTables(vector_of_referrables) : 0; auto vector_of_weak_references__ = vector_of_weak_references ? _fbb.CreateVector(*vector_of_weak_references) : 0; - auto vector_of_strong_referrables__ = vector_of_strong_referrables ? _fbb.CreateVector>(*vector_of_strong_referrables) : 0; + auto vector_of_strong_referrables__ = vector_of_strong_referrables ? _fbb.CreateVectorOfSortedTables(vector_of_strong_referrables) : 0; auto vector_of_co_owning_references__ = vector_of_co_owning_references ? _fbb.CreateVector(*vector_of_co_owning_references) : 0; auto vector_of_non_owning_references__ = vector_of_non_owning_references ? _fbb.CreateVector(*vector_of_non_owning_references) : 0; auto vector_of_enums__ = vector_of_enums ? _fbb.CreateVector(*vector_of_enums) : 0;