Add ::Set function to Unions to make memory ownership clear.

Unions own the NativeTable* value member because they need to destroy them
when the Union goes out of scope.  Currently, the data is destroyed by calling
delete, which means that the member needs to be allocated with new.  However,
making the allocation the responsibility of the client and the destruction
the responsibility of the Union can lead to potential errors.  Adding a
Set function will ensure that the memory is allocated correctly so that it
can be deleted later.

From cl/142161569.

Change-Id: I4605f26d2749164819bfae0140e5fae08442b50a

samples/monster_generated.h

@@ -38,21 +38,6 @@ enum Equipment {
   Equipment_MAX = Equipment_Weapon
 };
 
-struct EquipmentUnion {
-  Equipment type;
-
-  flatbuffers::NativeTable *table;
-  EquipmentUnion() : type(Equipment_NONE), table(nullptr) {}
-  EquipmentUnion(const EquipmentUnion &);
-  EquipmentUnion &operator=(const EquipmentUnion &);
-  ~EquipmentUnion();
-
-  static flatbuffers::NativeTable *UnPack(const void *union_obj, Equipment type, const flatbuffers::resolver_function_t *resolver);
-  flatbuffers::Offset<void> Pack(flatbuffers::FlatBufferBuilder &_fbb, const flatbuffers::rehasher_function_t *rehasher = nullptr) const;
-
-  WeaponT *AsWeapon() { return type == Equipment_Weapon ? reinterpret_cast<WeaponT *>(table) : nullptr; }
-};
-
 inline const char **EnumNamesEquipment() {
   static const char *names[] = { "NONE", "Weapon", nullptr };
   return names;
@@ -68,6 +53,31 @@ template<> struct EquipmentTraits<Weapon> {
   static const Equipment enum_value = Equipment_Weapon;
 };
 
+struct EquipmentUnion {
+  Equipment type = Equipment_NONE;
+
+  flatbuffers::NativeTable *table = nullptr;
+  EquipmentUnion() : type(Equipment_NONE), table(nullptr) {}
+  EquipmentUnion(const EquipmentUnion &);
+  EquipmentUnion &operator=(const EquipmentUnion &);
+  ~EquipmentUnion() { Reset(); }
+  void Reset();
+
+  template <typename T>
+  void Set(T&& value) {
+    Reset();
+    type = EquipmentTraits<typename T::TableType>::enum_value;
+    if (type != Equipment_NONE) {
+      table = new T(std::move(value));
+    }
+  }
+
+  static flatbuffers::NativeTable *UnPack(const void *union_obj, Equipment type, const flatbuffers::resolver_function_t *resolver);
+  flatbuffers::Offset<void> Pack(flatbuffers::FlatBufferBuilder &_fbb, const flatbuffers::rehasher_function_t *rehasher = nullptr) const;
+
+  WeaponT *AsWeapon() { return type == Equipment_Weapon ? reinterpret_cast<WeaponT *>(table) : nullptr; }
+};
+
 inline bool VerifyEquipment(flatbuffers::Verifier &verifier, const void *union_obj, Equipment type);
 
 MANUALLY_ALIGNED_STRUCT(4) Vec3 FLATBUFFERS_FINAL_CLASS {
@@ -347,11 +357,13 @@ inline flatbuffers::Offset<void> EquipmentUnion::Pack(flatbuffers::FlatBufferBui
   }
 }
 
-inline EquipmentUnion::~EquipmentUnion() {
+inline void EquipmentUnion::Reset() {
   switch (type) {
     case Equipment_Weapon: delete reinterpret_cast<WeaponT *>(table); break;
-    default:;
+    default: break;
   }
+  table = nullptr;
+  type = Equipment_NONE;
 }
 
 inline const MyGame::Sample::Monster *GetMonster(const void *buf) {

src/idl_gen_cpp.cpp

@@ -472,36 +472,6 @@ class CppGenerator : public BaseGenerator {
               GenTypeBasic(enum_def.underlying_type, false) + ")\n";
     code += "\n";
 
-    if (parser_.opts.generate_object_based_api && enum_def.is_union) {
-      // Generate a union type
-      code += "struct " + enum_def.name + "Union {\n";
-      code += "  " + enum_def.name + " type;\n\n";
-      code += "  flatbuffers::NativeTable *table;\n";
-      code += "  " + enum_def.name + "Union() : type(";
-      code += GetEnumValUse(enum_def, *enum_def.vals.Lookup("NONE"), parser_.opts);
-      code += "), table(nullptr) {}\n";
-      code += "  " + enum_def.name + "Union(const ";
-      code += enum_def.name + "Union &);\n";
-      code += "  " + enum_def.name + "Union &operator=(const ";
-      code += enum_def.name + "Union &);\n";
-      code += "  ~" + enum_def.name + "Union();\n\n";
-      code += "  " + UnionUnPackSignature(enum_def, true) + ";\n";
-      code += "  " + UnionPackSignature(enum_def, true) + ";\n\n";
-      for (auto it = enum_def.vals.vec.begin(); it != enum_def.vals.vec.end();
-           ++it) {
-        auto &ev = **it;
-        if (ev.value) {
-          auto native_name = NativeName(WrapInNameSpace(*ev.struct_def));
-          code += "  " + native_name + " *As";
-          code += ev.name + "() { return type == ";
-          code += GetEnumValUse(enum_def, ev, parser_.opts);
-          code += " ? reinterpret_cast<" + native_name;
-          code += " *>(table) : nullptr; }\n";
-        }
-      }
-      code += "};\n\n";
-    }
-
     // Generate a generate string table for enum values.
     // Problem is, if values are very sparse that could generate really big
     // tables. Ideally in that case we generate a map lookup instead, but for
@@ -552,6 +522,50 @@ class CppGenerator : public BaseGenerator {
       }
     }
 
+    if (parser_.opts.generate_object_based_api && enum_def.is_union) {
+      // Generate a union type
+      code += "struct " + enum_def.name + "Union {\n";
+      code += "  " + enum_def.name + " type = ";
+      code += GetEnumValUse(enum_def, *enum_def.vals.Lookup("NONE"), parser_.opts);
+      code += ";\n\n";
+      code += "  flatbuffers::NativeTable *table = nullptr;\n";
+      code += "  " + enum_def.name + "Union() : type(";
+      code += GetEnumValUse(enum_def, *enum_def.vals.Lookup("NONE"), parser_.opts);
+      code += "), table(nullptr) {}\n";
+      code += "  " + enum_def.name + "Union(const ";
+      code += enum_def.name + "Union &);\n";
+      code += "  " + enum_def.name + "Union &operator=(const ";
+      code += enum_def.name + "Union &);\n";
+      code += "  ~" + enum_def.name + "Union() { Reset(); }\n";
+      code += "  void Reset();\n\n";
+      code += "  template <typename T>\n";
+      code += "  void Set(T&& value) {\n";
+      code += "    Reset();\n";
+      code += "    type = " + enum_def.name;
+      code += "Traits<typename T::TableType>::enum_value;\n";
+      code += "    if (type != ";
+      code += GetEnumValUse(enum_def, *enum_def.vals.Lookup("NONE"), parser_.opts);
+      code += ") {\n";
+      code += "      table = new T(std::forward<T>(value));\n";
+      code += "    }\n";
+      code += "  }\n\n";
+      code += "  " + UnionUnPackSignature(enum_def, true) + ";\n";
+      code += "  " + UnionPackSignature(enum_def, true) + ";\n\n";
+      for (auto it = enum_def.vals.vec.begin(); it != enum_def.vals.vec.end();
+           ++it) {
+        auto &ev = **it;
+        if (ev.value) {
+          auto native_name = NativeName(WrapInNameSpace(*ev.struct_def));
+          code += "  " + native_name + " *As";
+          code += ev.name + "() { return type == ";
+          code += GetEnumValUse(enum_def, ev, parser_.opts);
+          code += " ? reinterpret_cast<" + native_name;
+          code += " *>(table) : nullptr; }\n";
+        }
+      }
+      code += "};\n\n";
+    }
+
     if (enum_def.is_union) {
       code += UnionVerifySignature(enum_def) + ";\n\n";
     }
@@ -614,8 +628,7 @@ class CppGenerator : public BaseGenerator {
       code += "    default: return 0;\n  }\n}\n\n";
 
       // Generate a union destructor.
-      code += "inline " + enum_def.name + "Union::~";
-      code += enum_def.name + "Union() {\n";
+      code += "inline void " + enum_def.name + "Union::Reset() {\n";
       code += "  switch (type) {\n";
       for (auto it = enum_def.vals.vec.begin(); it != enum_def.vals.vec.end();
            ++it) {
@@ -627,7 +640,13 @@ class CppGenerator : public BaseGenerator {
           code += " *>(table); break;\n";
         }
       }
-      code += "    default:;\n  }\n}\n\n";
+      code += "    default: break;\n";
+      code += "  }\n";
+      code += "  table = nullptr;\n";
+      code += "  type = ";
+      code += GetEnumValUse(enum_def, *enum_def.vals.Lookup("NONE"), parser_.opts);
+      code += ";\n";
+      code += "}\n\n";
     }
   }
 

tests/monster_test_generated.h

@@ -52,23 +52,6 @@ enum Any {
   Any_MAX = Any_MyGame_Example2_Monster
 };
 
-struct AnyUnion {
-  Any type;
-
-  flatbuffers::NativeTable *table;
-  AnyUnion() : type(Any_NONE), table(nullptr) {}
-  AnyUnion(const AnyUnion &);
-  AnyUnion &operator=(const AnyUnion &);
-  ~AnyUnion();
-
-  static flatbuffers::NativeTable *UnPack(const void *union_obj, Any type, const flatbuffers::resolver_function_t *resolver);
-  flatbuffers::Offset<void> Pack(flatbuffers::FlatBufferBuilder &_fbb, const flatbuffers::rehasher_function_t *rehasher = nullptr) const;
-
-  MonsterT *AsMonster() { return type == Any_Monster ? reinterpret_cast<MonsterT *>(table) : nullptr; }
-  TestSimpleTableWithEnumT *AsTestSimpleTableWithEnum() { return type == Any_TestSimpleTableWithEnum ? reinterpret_cast<TestSimpleTableWithEnumT *>(table) : nullptr; }
-  MyGame::Example2::MonsterT *AsMyGame_Example2_Monster() { return type == Any_MyGame_Example2_Monster ? reinterpret_cast<MyGame::Example2::MonsterT *>(table) : nullptr; }
-};
-
 inline const char **EnumNamesAny() {
   static const char *names[] = { "NONE", "Monster", "TestSimpleTableWithEnum", "MyGame_Example2_Monster", nullptr };
   return names;
@@ -92,6 +75,33 @@ template<> struct AnyTraits<MyGame::Example2::Monster> {
   static const Any enum_value = Any_MyGame_Example2_Monster;
 };
 
+struct AnyUnion {
+  Any type = Any_NONE;
+
+  flatbuffers::NativeTable *table = nullptr;
+  AnyUnion() : type(Any_NONE), table(nullptr) {}
+  AnyUnion(const AnyUnion &);
+  AnyUnion &operator=(const AnyUnion &);
+  ~AnyUnion() { Reset(); }
+  void Reset();
+
+  template <typename T>
+  void Set(T&& value) {
+    Reset();
+    type = AnyTraits<typename T::TableType>::enum_value;
+    if (type != Any_NONE) {
+      table = new T(std::move(value));
+    }
+  }
+
+  static flatbuffers::NativeTable *UnPack(const void *union_obj, Any type, const flatbuffers::resolver_function_t *resolver);
+  flatbuffers::Offset<void> Pack(flatbuffers::FlatBufferBuilder &_fbb, const flatbuffers::rehasher_function_t *rehasher = nullptr) const;
+
+  MonsterT *AsMonster() { return type == Any_Monster ? reinterpret_cast<MonsterT *>(table) : nullptr; }
+  TestSimpleTableWithEnumT *AsTestSimpleTableWithEnum() { return type == Any_TestSimpleTableWithEnum ? reinterpret_cast<TestSimpleTableWithEnumT *>(table) : nullptr; }
+  MyGame::Example2::MonsterT *AsMyGame_Example2_Monster() { return type == Any_MyGame_Example2_Monster ? reinterpret_cast<MyGame::Example2::MonsterT *>(table) : nullptr; }
+};
+
 inline bool VerifyAny(flatbuffers::Verifier &verifier, const void *union_obj, Any type);
 
 MANUALLY_ALIGNED_STRUCT(2) Test FLATBUFFERS_FINAL_CLASS {
@@ -767,13 +777,15 @@ inline flatbuffers::Offset<void> AnyUnion::Pack(flatbuffers::FlatBufferBuilder &
   }
 }
 
-inline AnyUnion::~AnyUnion() {
+inline void AnyUnion::Reset() {
   switch (type) {
     case Any_Monster: delete reinterpret_cast<MonsterT *>(table); break;
     case Any_TestSimpleTableWithEnum: delete reinterpret_cast<TestSimpleTableWithEnumT *>(table); break;
     case Any_MyGame_Example2_Monster: delete reinterpret_cast<MyGame::Example2::MonsterT *>(table); break;
-    default:;
+    default: break;
   }
+  table = nullptr;
+  type = Any_NONE;
 }
 
 inline const MyGame::Example::Monster *GetMonster(const void *buf) {