#5544 Fix of Array of table is not sorted if Create<type>Direct() is used (#5546)

* Fix C/C++ Create<Type>Direct 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

diff --git a/include/flatbuffers/reflection_generated.h b/include/flatbuffers/reflection_generated.h
index 0e73a0f..77a2c7f 100644 (file)
--- a/include/flatbuffers/reflection_generated.h
+++ b/include/flatbuffers/reflection_generated.h
@@ -463,14 +463,14 @@ inline flatbuffers::Offset<Enum> CreateEnum(
 inline flatbuffers::Offset<Enum> CreateEnumDirect(
     flatbuffers::FlatBufferBuilder &_fbb,
     const char *name = nullptr,
-    const std::vector<flatbuffers::Offset<reflection::EnumVal>> *values = nullptr,
+    std::vector<flatbuffers::Offset<reflection::EnumVal>> *values = nullptr,
     bool is_union = false,
     flatbuffers::Offset<reflection::Type> underlying_type = 0,
-    const std::vector<flatbuffers::Offset<reflection::KeyValue>> *attributes = nullptr,
+    std::vector<flatbuffers::Offset<reflection::KeyValue>> *attributes = nullptr,
     const std::vector<flatbuffers::Offset<flatbuffers::String>> *documentation = nullptr) {
   auto name__ = name ? _fbb.CreateString(name) : 0;
-  auto values__ = values ? _fbb.CreateVector<flatbuffers::Offset<reflection::EnumVal>>(*values) : 0;
-  auto attributes__ = attributes ? _fbb.CreateVector<flatbuffers::Offset<reflection::KeyValue>>(*attributes) : 0;
+  auto values__ = values ? _fbb.CreateVectorOfSortedTables<reflection::EnumVal>(values) : 0;
+  auto attributes__ = attributes ? _fbb.CreateVectorOfSortedTables<reflection::KeyValue>(attributes) : 0;
   auto documentation__ = documentation ? _fbb.CreateVector<flatbuffers::Offset<flatbuffers::String>>(*documentation) : 0;
   return reflection::CreateEnum(
       _fbb,
@@ -647,10 +647,10 @@ inline flatbuffers::Offset<Field> CreateFieldDirect(
     bool deprecated = false,
     bool required = false,
     bool key = false,
-    const std::vector<flatbuffers::Offset<reflection::KeyValue>> *attributes = nullptr,
+    std::vector<flatbuffers::Offset<reflection::KeyValue>> *attributes = nullptr,
     const std::vector<flatbuffers::Offset<flatbuffers::String>> *documentation = nullptr) {
   auto name__ = name ? _fbb.CreateString(name) : 0;
-  auto attributes__ = attributes ? _fbb.CreateVector<flatbuffers::Offset<reflection::KeyValue>>(*attributes) : 0;
+  auto attributes__ = attributes ? _fbb.CreateVectorOfSortedTables<reflection::KeyValue>(attributes) : 0;
   auto documentation__ = documentation ? _fbb.CreateVector<flatbuffers::Offset<flatbuffers::String>>(*documentation) : 0;
   return reflection::CreateField(
       _fbb,
@@ -785,15 +785,15 @@ inline flatbuffers::Offset<Object> CreateObject(
 inline flatbuffers::Offset<Object> CreateObjectDirect(
     flatbuffers::FlatBufferBuilder &_fbb,
     const char *name = nullptr,
-    const std::vector<flatbuffers::Offset<reflection::Field>> *fields = nullptr,
+    std::vector<flatbuffers::Offset<reflection::Field>> *fields = nullptr,
     bool is_struct = false,
     int32_t minalign = 0,
     int32_t bytesize = 0,
-    const std::vector<flatbuffers::Offset<reflection::KeyValue>> *attributes = nullptr,
+    std::vector<flatbuffers::Offset<reflection::KeyValue>> *attributes = nullptr,
     const std::vector<flatbuffers::Offset<flatbuffers::String>> *documentation = nullptr) {
   auto name__ = name ? _fbb.CreateString(name) : 0;
-  auto fields__ = fields ? _fbb.CreateVector<flatbuffers::Offset<reflection::Field>>(*fields) : 0;
-  auto attributes__ = attributes ? _fbb.CreateVector<flatbuffers::Offset<reflection::KeyValue>>(*attributes) : 0;
+  auto fields__ = fields ? _fbb.CreateVectorOfSortedTables<reflection::Field>(fields) : 0;
+  auto attributes__ = attributes ? _fbb.CreateVectorOfSortedTables<reflection::KeyValue>(attributes) : 0;
   auto documentation__ = documentation ? _fbb.CreateVector<flatbuffers::Offset<flatbuffers::String>>(*documentation) : 0;
   return reflection::CreateObject(
       _fbb,
@@ -907,10 +907,10 @@ inline flatbuffers::Offset<RPCCall> CreateRPCCallDirect(
     const char *name = nullptr,
     flatbuffers::Offset<reflection::Object> request = 0,
     flatbuffers::Offset<reflection::Object> response = 0,
-    const std::vector<flatbuffers::Offset<reflection::KeyValue>> *attributes = nullptr,
+    std::vector<flatbuffers::Offset<reflection::KeyValue>> *attributes = nullptr,
     const std::vector<flatbuffers::Offset<flatbuffers::String>> *documentation = nullptr) {
   auto name__ = name ? _fbb.CreateString(name) : 0;
-  auto attributes__ = attributes ? _fbb.CreateVector<flatbuffers::Offset<reflection::KeyValue>>(*attributes) : 0;
+  auto attributes__ = attributes ? _fbb.CreateVectorOfSortedTables<reflection::KeyValue>(attributes) : 0;
   auto documentation__ = documentation ? _fbb.CreateVector<flatbuffers::Offset<flatbuffers::String>>(*documentation) : 0;
   return reflection::CreateRPCCall(
       _fbb,
@@ -1008,12 +1008,12 @@ inline flatbuffers::Offset<Service> CreateService(
 inline flatbuffers::Offset<Service> CreateServiceDirect(
     flatbuffers::FlatBufferBuilder &_fbb,
     const char *name = nullptr,
-    const std::vector<flatbuffers::Offset<reflection::RPCCall>> *calls = nullptr,
-    const std::vector<flatbuffers::Offset<reflection::KeyValue>> *attributes = nullptr,
+    std::vector<flatbuffers::Offset<reflection::RPCCall>> *calls = nullptr,
+    std::vector<flatbuffers::Offset<reflection::KeyValue>> *attributes = nullptr,
     const std::vector<flatbuffers::Offset<flatbuffers::String>> *documentation = nullptr) {
   auto name__ = name ? _fbb.CreateString(name) : 0;
-  auto calls__ = calls ? _fbb.CreateVector<flatbuffers::Offset<reflection::RPCCall>>(*calls) : 0;
-  auto attributes__ = attributes ? _fbb.CreateVector<flatbuffers::Offset<reflection::KeyValue>>(*attributes) : 0;
+  auto calls__ = calls ? _fbb.CreateVectorOfSortedTables<reflection::RPCCall>(calls) : 0;
+  auto attributes__ = attributes ? _fbb.CreateVectorOfSortedTables<reflection::KeyValue>(attributes) : 0;
   auto documentation__ = documentation ? _fbb.CreateVector<flatbuffers::Offset<flatbuffers::String>>(*documentation) : 0;
   return reflection::CreateService(
       _fbb,
@@ -1126,17 +1126,17 @@ inline flatbuffers::Offset<Schema> CreateSchema(
 
 inline flatbuffers::Offset<Schema> CreateSchemaDirect(
     flatbuffers::FlatBufferBuilder &_fbb,
-    const std::vector<flatbuffers::Offset<reflection::Object>> *objects = nullptr,
-    const std::vector<flatbuffers::Offset<reflection::Enum>> *enums = nullptr,
+    std::vector<flatbuffers::Offset<reflection::Object>> *objects = nullptr,
+    std::vector<flatbuffers::Offset<reflection::Enum>> *enums = nullptr,
     const char *file_ident = nullptr,
     const char *file_ext = nullptr,
     flatbuffers::Offset<reflection::Object> root_table = 0,
-    const std::vector<flatbuffers::Offset<reflection::Service>> *services = nullptr) {
-  auto objects__ = objects ? _fbb.CreateVector<flatbuffers::Offset<reflection::Object>>(*objects) : 0;
-  auto enums__ = enums ? _fbb.CreateVector<flatbuffers::Offset<reflection::Enum>>(*enums) : 0;
+    std::vector<flatbuffers::Offset<reflection::Service>> *services = nullptr) {
+  auto objects__ = objects ? _fbb.CreateVectorOfSortedTables<reflection::Object>(objects) : 0;
+  auto enums__ = enums ? _fbb.CreateVectorOfSortedTables<reflection::Enum>(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<flatbuffers::Offset<reflection::Service>>(*services) : 0;
+  auto services__ = services ? _fbb.CreateVectorOfSortedTables<reflection::Service>(services) : 0;
   return reflection::CreateSchema(
       _fbb,
       objects__,

diff --git a/src/idl_gen_cpp.cpp b/src/idl_gen_cpp.cpp
index 1efb057..505fdaa 100644 (file)
--- 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<std::string> &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<const "
-                     "uint8_t *>(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 1f05b3a..d1b5c38 100644 (file)
--- a/tests/monster_test_generated.h
+++ b/tests/monster_test_generated.h
@@ -2024,7 +2024,7 @@ inline flatbuffers::Offset<Monster> CreateMonsterDirect(
     flatbuffers::Offset<void> test = 0,
     const std::vector<MyGame::Example::Test> *test4 = nullptr,
     const std::vector<flatbuffers::Offset<flatbuffers::String>> *testarrayofstring = nullptr,
-    const std::vector<flatbuffers::Offset<MyGame::Example::Monster>> *testarrayoftables = nullptr,
+    std::vector<flatbuffers::Offset<MyGame::Example::Monster>> *testarrayoftables = nullptr,
     flatbuffers::Offset<MyGame::Example::Monster> enemy = 0,
     const std::vector<uint8_t> *testnestedflatbuffer = nullptr,
     flatbuffers::Offset<MyGame::Example::Stat> testempty = 0,
@@ -2042,16 +2042,16 @@ inline flatbuffers::Offset<Monster> CreateMonsterDirect(
     float testf2 = 3.0f,
     float testf3 = 0.0f,
     const std::vector<flatbuffers::Offset<flatbuffers::String>> *testarrayofstring2 = nullptr,
-    const std::vector<MyGame::Example::Ability> *testarrayofsortedstruct = nullptr,
+    std::vector<MyGame::Example::Ability> *testarrayofsortedstruct = nullptr,
     const std::vector<uint8_t> *flex = nullptr,
     const std::vector<MyGame::Example::Test> *test5 = nullptr,
     const std::vector<int64_t> *vector_of_longs = nullptr,
     const std::vector<double> *vector_of_doubles = nullptr,
     flatbuffers::Offset<MyGame::InParentNamespace> parent_namespace_test = 0,
-    const std::vector<flatbuffers::Offset<MyGame::Example::Referrable>> *vector_of_referrables = nullptr,
+    std::vector<flatbuffers::Offset<MyGame::Example::Referrable>> *vector_of_referrables = nullptr,
     uint64_t single_weak_reference = 0,
     const std::vector<uint64_t> *vector_of_weak_references = nullptr,
-    const std::vector<flatbuffers::Offset<MyGame::Example::Referrable>> *vector_of_strong_referrables = nullptr,
+    std::vector<flatbuffers::Offset<MyGame::Example::Referrable>> *vector_of_strong_referrables = nullptr,
     uint64_t co_owning_reference = 0,
     const std::vector<uint64_t> *vector_of_co_owning_references = nullptr,
     uint64_t non_owning_reference = 0,
@@ -2066,18 +2066,18 @@ inline flatbuffers::Offset<Monster> CreateMonsterDirect(
   auto inventory__ = inventory ? _fbb.CreateVector<uint8_t>(*inventory) : 0;
   auto test4__ = test4 ? _fbb.CreateVectorOfStructs<MyGame::Example::Test>(*test4) : 0;
   auto testarrayofstring__ = testarrayofstring ? _fbb.CreateVector<flatbuffers::Offset<flatbuffers::String>>(*testarrayofstring) : 0;
-  auto testarrayoftables__ = testarrayoftables ? _fbb.CreateVector<flatbuffers::Offset<MyGame::Example::Monster>>(*testarrayoftables) : 0;
+  auto testarrayoftables__ = testarrayoftables ? _fbb.CreateVectorOfSortedTables<MyGame::Example::Monster>(testarrayoftables) : 0;
   auto testnestedflatbuffer__ = testnestedflatbuffer ? _fbb.CreateVector<uint8_t>(*testnestedflatbuffer) : 0;
   auto testarrayofbools__ = testarrayofbools ? _fbb.CreateVector<uint8_t>(*testarrayofbools) : 0;
   auto testarrayofstring2__ = testarrayofstring2 ? _fbb.CreateVector<flatbuffers::Offset<flatbuffers::String>>(*testarrayofstring2) : 0;
-  auto testarrayofsortedstruct__ = testarrayofsortedstruct ? _fbb.CreateVectorOfStructs<MyGame::Example::Ability>(*testarrayofsortedstruct) : 0;
+  auto testarrayofsortedstruct__ = testarrayofsortedstruct ? _fbb.CreateVectorOfSortedStructs<MyGame::Example::Ability>(testarrayofsortedstruct) : 0;
   auto flex__ = flex ? _fbb.CreateVector<uint8_t>(*flex) : 0;
   auto test5__ = test5 ? _fbb.CreateVectorOfStructs<MyGame::Example::Test>(*test5) : 0;
   auto vector_of_longs__ = vector_of_longs ? _fbb.CreateVector<int64_t>(*vector_of_longs) : 0;
   auto vector_of_doubles__ = vector_of_doubles ? _fbb.CreateVector<double>(*vector_of_doubles) : 0;
-  auto vector_of_referrables__ = vector_of_referrables ? _fbb.CreateVector<flatbuffers::Offset<MyGame::Example::Referrable>>(*vector_of_referrables) : 0;
+  auto vector_of_referrables__ = vector_of_referrables ? _fbb.CreateVectorOfSortedTables<MyGame::Example::Referrable>(vector_of_referrables) : 0;
   auto vector_of_weak_references__ = vector_of_weak_references ? _fbb.CreateVector<uint64_t>(*vector_of_weak_references) : 0;
-  auto vector_of_strong_referrables__ = vector_of_strong_referrables ? _fbb.CreateVector<flatbuffers::Offset<MyGame::Example::Referrable>>(*vector_of_strong_referrables) : 0;
+  auto vector_of_strong_referrables__ = vector_of_strong_referrables ? _fbb.CreateVectorOfSortedTables<MyGame::Example::Referrable>(vector_of_strong_referrables) : 0;
   auto vector_of_co_owning_references__ = vector_of_co_owning_references ? _fbb.CreateVector<uint64_t>(*vector_of_co_owning_references) : 0;
   auto vector_of_non_owning_references__ = vector_of_non_owning_references ? _fbb.CreateVector<uint64_t>(*vector_of_non_owning_references) : 0;
   auto vector_of_enums__ = vector_of_enums ? _fbb.CreateVector<uint8_t>(*vector_of_enums) : 0;