[C++] Got rid of memset's in constructors (#5938)
authorbakinovsky-m <bakinovsky-m@users.noreply.github.com>
Tue, 2 Jun 2020 00:58:52 +0000 (03:58 +0300)
committerGitHub <noreply@github.com>
Tue, 2 Jun 2020 00:58:52 +0000 (17:58 -0700)
* [C++] removed array's memsets from struct parametrized constructor

now POD-typed arrays are zero-initialized
and class-typed arrays are default-initialized

* [C++] memset -> zero/default initialization in default constructor

* [C++] Struct-type and array default initialization

* [C++] Newly generated tests

* [C++] forgotten test

* [C++] curly brace by code style

* [C++] test if memory is 0's after placement new

* [C++] memory leak fix

* [C++] simplifying and non-dynamic memory in test

* [C++] code cleanup

* [C++] disable old-compiler warning

* [C++] windows build fix (try)

* [C++] debug-new win build fix

include/flatbuffers/base.h
samples/monster_generated.h
src/idl_gen_cpp.cpp
tests/arrays_test_generated.h
tests/cpp17/generated_cpp17/monster_test_generated.h
tests/monster_test_generated.h
tests/namespace_test/namespace_test1_generated.h
tests/native_type_test_generated.h
tests/test.cpp
tests/union_vector/union_vector_generated.h

index 9557380..3d7aeb4 100644 (file)
@@ -309,6 +309,7 @@ typedef uintmax_t largest_scalar_t;
 #define FLATBUFFERS_MAX_ALIGNMENT 16
 
 #if defined(_MSC_VER)
+  #pragma warning(disable: 4351) // C4351: new behavior: elements of array ... will be default initialized
   #pragma warning(push)
   #pragma warning(disable: 4127) // C4127: conditional expression is constant
 #endif
index 00aa70b..42482f6 100644 (file)
@@ -179,8 +179,10 @@ FLATBUFFERS_MANUALLY_ALIGNED_STRUCT(4) Vec3 FLATBUFFERS_FINAL_CLASS {
   static const flatbuffers::TypeTable *MiniReflectTypeTable() {
     return Vec3TypeTable();
   }
-  Vec3() {
-    memset(static_cast<void *>(this), 0, sizeof(Vec3));
+  Vec3()
+      : x_(0),
+        y_(0),
+        z_(0) {
   }
   Vec3(float _x, float _y, float _z)
       : x_(flatbuffers::EndianScalar(_x)),
index 3b798ae..57010f5 100644 (file)
@@ -2851,38 +2851,100 @@ class CppGenerator : public BaseGenerator {
 
   static void PaddingNoop(int bits, std::string *code_ptr, int *id) {
     (void)bits;
-    *code_ptr += "    (void)padding" + NumToString((*id)++) + "__;";
+    *code_ptr += "    (void)padding" + NumToString((*id)++) + "__;\n";
+  }
+
+  void GenStructDefaultConstructor(const StructDef &struct_def) {
+    std::string init_list;
+    std::string body;
+    bool first_in_init_list = true;
+    int padding_initializer_id = 0;
+    int padding_body_id = 0;
+    for (auto it = struct_def.fields.vec.begin();
+         it != struct_def.fields.vec.end();
+         ++it) {
+      const auto field = *it;
+      const auto field_name = field->name + "_";
+
+      if (first_in_init_list) {
+        first_in_init_list = false;
+      } else {
+        init_list += ",";
+        init_list += "\n        ";
+      }
+
+      init_list += field_name;
+      if (IsStruct(field->value.type) || IsArray(field->value.type)) {
+        // this is either default initialization of struct
+        // or
+        // implicit initialization of array
+        // for each object in array it:
+        // * sets it as zeros for POD types (integral, floating point, etc)
+        // * calls default constructor for classes/structs
+        init_list += "()";
+      } else {
+        init_list += "(0)";
+      }
+      if (field->padding) {
+        GenPadding(*field, &init_list, &padding_initializer_id,
+                   PaddingInitializer);
+        GenPadding(*field, &body, &padding_body_id, PaddingNoop);
+      }
+    }
+
+    if (init_list.empty()) {
+      code_ += "  {{STRUCT_NAME}}()";
+      code_ += "  {}";
+    } else {
+      code_.SetValue("INIT_LIST", init_list);
+      code_.SetValue("DEFAULT_CONSTRUCTOR_BODY", body);
+      code_ += "  {{STRUCT_NAME}}()";
+      code_ += "      : {{INIT_LIST}} {";
+      code_ += "{{DEFAULT_CONSTRUCTOR_BODY}}  }";
+    }
   }
 
   void GenStructConstructor(const StructDef &struct_def) {
     std::string arg_list;
     std::string init_list;
     int padding_id = 0;
-    auto first = struct_def.fields.vec.begin();
+    bool first_arg = true;
+    bool first_init = true;
     for (auto it = struct_def.fields.vec.begin();
          it != struct_def.fields.vec.end(); ++it) {
       const auto &field = **it;
       const auto &field_type = field.value.type;
-      if (IsArray(field_type)) {
-        first++;
-        continue;
-      }
       const auto member_name = Name(field) + "_";
       const auto arg_name = "_" + Name(field);
       const auto arg_type = GenTypeGet(field_type, " ", "const ", " &", true);
 
-      if (it != first) { arg_list += ", "; }
-      arg_list += arg_type;
-      arg_list += arg_name;
       if (!IsArray(field_type)) {
-        if (it != first && init_list != "") { init_list += ",\n        "; }
-        init_list += member_name;
-        if (IsScalar(field_type.base_type)) {
-          auto type = GenUnderlyingCast(field, false, arg_name);
-          init_list += "(flatbuffers::EndianScalar(" + type + "))";
+        if (first_arg) {
+          first_arg = false;
         } else {
-          init_list += "(" + arg_name + ")";
+          arg_list += ", ";
         }
+        arg_list += arg_type;
+        arg_list += arg_name;
+      }
+      if (first_init) {
+        first_init = false;
+      } else {
+        init_list += ",";
+        init_list += "\n        ";
+      }
+      init_list += member_name;
+      if (IsScalar(field_type.base_type)) {
+        auto type = GenUnderlyingCast(field, false, arg_name);
+        init_list += "(flatbuffers::EndianScalar(" + type + "))";
+      } else if (IsArray(field_type)) {
+        // implicit initialization of array
+        // for each object in array it:
+        // * sets it as zeros for POD types (integral, floating point, etc)
+        // * calls default constructor for classes/structs
+        init_list += "()";
+      } else {
+        init_list += "(" + arg_name + ")";
       }
       if (field.padding) {
         GenPadding(field, &init_list, &padding_id, PaddingInitializer);
@@ -2898,27 +2960,6 @@ class CppGenerator : public BaseGenerator {
       } else {
         code_ += "  {{STRUCT_NAME}}({{ARG_LIST}}) {";
       }
-      padding_id = 0;
-      for (auto it = struct_def.fields.vec.begin();
-           it != struct_def.fields.vec.end(); ++it) {
-        const auto &field = **it;
-        const auto &field_type = field.value.type;
-        if (IsArray(field_type)) {
-          const auto &elem_type = field_type.VectorType();
-          (void)elem_type;
-          FLATBUFFERS_ASSERT(
-              (IsScalar(elem_type.base_type) || IsStruct(elem_type)) &&
-              "invalid declaration");
-          const auto &member = Name(field) + "_";
-          code_ +=
-              "    std::memset(" + member + ", 0, sizeof(" + member + "));";
-        }
-        if (field.padding) {
-          std::string padding;
-          GenPadding(field, &padding, &padding_id, PaddingNoop);
-          code_ += padding;
-        }
-      }
       code_ += "  }";
     }
   }
@@ -2974,10 +3015,7 @@ class CppGenerator : public BaseGenerator {
     GenFullyQualifiedNameGetter(struct_def, Name(struct_def));
 
     // Generate a default constructor.
-    code_ += "  {{STRUCT_NAME}}() {";
-    code_ +=
-        "    memset(static_cast<void *>(this), 0, sizeof({{STRUCT_NAME}}));";
-    code_ += "  }";
+    GenStructDefaultConstructor(struct_def);
 
     // Generate a constructor that takes all fields as arguments,
     // excluding arrays
index 66599d1..b7aae3b 100644 (file)
@@ -75,15 +75,23 @@ FLATBUFFERS_MANUALLY_ALIGNED_STRUCT(8) NestedStruct FLATBUFFERS_FINAL_CLASS {
   static const flatbuffers::TypeTable *MiniReflectTypeTable() {
     return NestedStructTypeTable();
   }
-  NestedStruct() {
-    memset(static_cast<void *>(this), 0, sizeof(NestedStruct));
+  NestedStruct()
+      : a_(),
+        b_(0),
+        c_(),
+        padding0__(0),
+        padding1__(0),
+        d_() {
+    (void)padding0__;
+    (void)padding1__;
   }
   NestedStruct(MyGame::Example::TestEnum _b)
-      : b_(flatbuffers::EndianScalar(static_cast<int8_t>(_b))) {
-    std::memset(a_, 0, sizeof(a_));
-    std::memset(c_, 0, sizeof(c_));
-    (void)padding0__;    (void)padding1__;
-    std::memset(d_, 0, sizeof(d_));
+      : a_(),
+        b_(flatbuffers::EndianScalar(static_cast<int8_t>(_b))),
+        c_(),
+        padding0__(0),
+        padding1__(0),
+        d_() {
   }
   const flatbuffers::Array<int32_t, 2> *a() const {
     return reinterpret_cast<const flatbuffers::Array<int32_t, 2> *>(a_);
@@ -140,22 +148,33 @@ FLATBUFFERS_MANUALLY_ALIGNED_STRUCT(8) ArrayStruct FLATBUFFERS_FINAL_CLASS {
   static const flatbuffers::TypeTable *MiniReflectTypeTable() {
     return ArrayStructTypeTable();
   }
-  ArrayStruct() {
-    memset(static_cast<void *>(this), 0, sizeof(ArrayStruct));
+  ArrayStruct()
+      : a_(0),
+        b_(),
+        c_(0),
+        padding0__(0),
+        padding1__(0),
+        padding2__(0),
+        d_(),
+        e_(0),
+        padding3__(0),
+        f_() {
+    (void)padding0__;
+    (void)padding1__;
+    (void)padding2__;
+    (void)padding3__;
   }
   ArrayStruct(float _a, int8_t _c, int32_t _e)
       : a_(flatbuffers::EndianScalar(_a)),
+        b_(),
         c_(flatbuffers::EndianScalar(_c)),
         padding0__(0),
         padding1__(0),
         padding2__(0),
+        d_(),
         e_(flatbuffers::EndianScalar(_e)),
-        padding3__(0) {
-    std::memset(b_, 0, sizeof(b_));
-    (void)padding0__;    (void)padding1__;    (void)padding2__;
-    std::memset(d_, 0, sizeof(d_));
-    (void)padding3__;
-    std::memset(f_, 0, sizeof(f_));
+        padding3__(0),
+        f_() {
   }
   float a() const {
     return flatbuffers::EndianScalar(a_);
index 043c21e..c8fbb25 100644 (file)
@@ -477,14 +477,16 @@ FLATBUFFERS_MANUALLY_ALIGNED_STRUCT(2) Test FLATBUFFERS_FINAL_CLASS {
   static const flatbuffers::TypeTable *MiniReflectTypeTable() {
     return TestTypeTable();
   }
-  Test() {
-    memset(static_cast<void *>(this), 0, sizeof(Test));
+  Test()
+      : a_(0),
+        b_(0),
+        padding0__(0) {
+    (void)padding0__;
   }
   Test(int16_t _a, int8_t _b)
       : a_(flatbuffers::EndianScalar(_a)),
         b_(flatbuffers::EndianScalar(_b)),
         padding0__(0) {
-    (void)padding0__;
   }
   int16_t a() const {
     return flatbuffers::EndianScalar(a_);
@@ -517,8 +519,19 @@ FLATBUFFERS_MANUALLY_ALIGNED_STRUCT(8) Vec3 FLATBUFFERS_FINAL_CLASS {
   static const flatbuffers::TypeTable *MiniReflectTypeTable() {
     return Vec3TypeTable();
   }
-  Vec3() {
-    memset(static_cast<void *>(this), 0, sizeof(Vec3));
+  Vec3()
+      : x_(0),
+        y_(0),
+        z_(0),
+        padding0__(0),
+        test1_(0),
+        test2_(0),
+        padding1__(0),
+        test3_(),
+        padding2__(0) {
+    (void)padding0__;
+    (void)padding1__;
+    (void)padding2__;
   }
   Vec3(float _x, float _y, float _z, double _test1, MyGame::Example::Color _test2, const MyGame::Example::Test &_test3)
       : x_(flatbuffers::EndianScalar(_x)),
@@ -530,9 +543,6 @@ FLATBUFFERS_MANUALLY_ALIGNED_STRUCT(8) Vec3 FLATBUFFERS_FINAL_CLASS {
         padding1__(0),
         test3_(_test3),
         padding2__(0) {
-    (void)padding0__;
-    (void)padding1__;
-    (void)padding2__;
   }
   float x() const {
     return flatbuffers::EndianScalar(x_);
@@ -582,8 +592,9 @@ FLATBUFFERS_MANUALLY_ALIGNED_STRUCT(4) Ability FLATBUFFERS_FINAL_CLASS {
   static const flatbuffers::TypeTable *MiniReflectTypeTable() {
     return AbilityTypeTable();
   }
-  Ability() {
-    memset(static_cast<void *>(this), 0, sizeof(Ability));
+  Ability()
+      : id_(0),
+        distance_(0) {
   }
   Ability(uint32_t _id, uint32_t _distance)
       : id_(flatbuffers::EndianScalar(_id)),
index 219e835..38f33eb 100644 (file)
@@ -592,14 +592,16 @@ FLATBUFFERS_MANUALLY_ALIGNED_STRUCT(2) Test FLATBUFFERS_FINAL_CLASS {
   static const flatbuffers::TypeTable *MiniReflectTypeTable() {
     return TestTypeTable();
   }
-  Test() {
-    memset(static_cast<void *>(this), 0, sizeof(Test));
+  Test()
+      : a_(0),
+        b_(0),
+        padding0__(0) {
+    (void)padding0__;
   }
   Test(int16_t _a, int8_t _b)
       : a_(flatbuffers::EndianScalar(_a)),
         b_(flatbuffers::EndianScalar(_b)),
         padding0__(0) {
-    (void)padding0__;
   }
   int16_t a() const {
     return flatbuffers::EndianScalar(a_);
@@ -643,8 +645,19 @@ FLATBUFFERS_MANUALLY_ALIGNED_STRUCT(8) Vec3 FLATBUFFERS_FINAL_CLASS {
   static const flatbuffers::TypeTable *MiniReflectTypeTable() {
     return Vec3TypeTable();
   }
-  Vec3() {
-    memset(static_cast<void *>(this), 0, sizeof(Vec3));
+  Vec3()
+      : x_(0),
+        y_(0),
+        z_(0),
+        padding0__(0),
+        test1_(0),
+        test2_(0),
+        padding1__(0),
+        test3_(),
+        padding2__(0) {
+    (void)padding0__;
+    (void)padding1__;
+    (void)padding2__;
   }
   Vec3(float _x, float _y, float _z, double _test1, MyGame::Example::Color _test2, const MyGame::Example::Test &_test3)
       : x_(flatbuffers::EndianScalar(_x)),
@@ -656,9 +669,6 @@ FLATBUFFERS_MANUALLY_ALIGNED_STRUCT(8) Vec3 FLATBUFFERS_FINAL_CLASS {
         padding1__(0),
         test3_(_test3),
         padding2__(0) {
-    (void)padding0__;
-    (void)padding1__;
-    (void)padding2__;
   }
   float x() const {
     return flatbuffers::EndianScalar(x_);
@@ -723,8 +733,9 @@ FLATBUFFERS_MANUALLY_ALIGNED_STRUCT(4) Ability FLATBUFFERS_FINAL_CLASS {
   static const flatbuffers::TypeTable *MiniReflectTypeTable() {
     return AbilityTypeTable();
   }
-  Ability() {
-    memset(static_cast<void *>(this), 0, sizeof(Ability));
+  Ability()
+      : id_(0),
+        distance_(0) {
   }
   Ability(uint32_t _id, uint32_t _distance)
       : id_(flatbuffers::EndianScalar(_id)),
index 7212276..f711b14 100644 (file)
@@ -66,8 +66,9 @@ FLATBUFFERS_MANUALLY_ALIGNED_STRUCT(4) StructInNestedNS FLATBUFFERS_FINAL_CLASS
   static const flatbuffers::TypeTable *MiniReflectTypeTable() {
     return StructInNestedNSTypeTable();
   }
-  StructInNestedNS() {
-    memset(static_cast<void *>(this), 0, sizeof(StructInNestedNS));
+  StructInNestedNS()
+      : a_(0),
+        b_(0) {
   }
   StructInNestedNS(int32_t _a, int32_t _b)
       : a_(flatbuffers::EndianScalar(_a)),
index d591a4e..1b38724 100644 (file)
@@ -30,8 +30,10 @@ FLATBUFFERS_MANUALLY_ALIGNED_STRUCT(4) Vector3D FLATBUFFERS_FINAL_CLASS {
   static const flatbuffers::TypeTable *MiniReflectTypeTable() {
     return Vector3DTypeTable();
   }
-  Vector3D() {
-    memset(static_cast<void *>(this), 0, sizeof(Vector3D));
+  Vector3D()
+      : x_(0),
+        y_(0),
+        z_(0) {
   }
   Vector3D(float _x, float _y, float _z)
       : x_(flatbuffers::EndianScalar(_x)),
index 1b00b4d..56883cf 100644 (file)
@@ -3222,6 +3222,24 @@ void FixedLengthArrayTest() {
   // Check alignment
   TEST_EQ(0, reinterpret_cast<uintptr_t>(mArStruct->d()) % 8);
   TEST_EQ(0, reinterpret_cast<uintptr_t>(mArStruct->f()) % 8);
+
+  // Check if default constructor set all memory zero
+  const size_t arr_size = sizeof(MyGame::Example::ArrayStruct);
+  char non_zero_memory[arr_size];
+  // set memory chunk of size ArrayStruct to 1's
+  std::memset(static_cast<void *>(non_zero_memory), 1, arr_size);
+  // after placement-new it should be all 0's
+#if defined (_MSC_VER) && defined (_DEBUG)
+  #undef new
+#endif
+  MyGame::Example::ArrayStruct *ap = new (non_zero_memory) MyGame::Example::ArrayStruct;
+#if defined (_MSC_VER) && defined (_DEBUG)
+  #define new DEBUG_NEW
+#endif
+  (void)ap;
+  for (size_t i = 0; i < arr_size; ++i) {
+    TEST_EQ(non_zero_memory[i], 0);
+  }
 #endif
 }
 
index f93dbf0..8ff683f 100644 (file)
@@ -202,8 +202,8 @@ FLATBUFFERS_MANUALLY_ALIGNED_STRUCT(4) Rapunzel FLATBUFFERS_FINAL_CLASS {
   static const flatbuffers::TypeTable *MiniReflectTypeTable() {
     return RapunzelTypeTable();
   }
-  Rapunzel() {
-    memset(static_cast<void *>(this), 0, sizeof(Rapunzel));
+  Rapunzel()
+      : hair_length_(0) {
   }
   Rapunzel(int32_t _hair_length)
       : hair_length_(flatbuffers::EndianScalar(_hair_length)) {
@@ -235,8 +235,8 @@ FLATBUFFERS_MANUALLY_ALIGNED_STRUCT(4) BookReader FLATBUFFERS_FINAL_CLASS {
   static const flatbuffers::TypeTable *MiniReflectTypeTable() {
     return BookReaderTypeTable();
   }
-  BookReader() {
-    memset(static_cast<void *>(this), 0, sizeof(BookReader));
+  BookReader()
+      : books_read_(0) {
   }
   BookReader(int32_t _books_read)
       : books_read_(flatbuffers::EndianScalar(_books_read)) {