Implement unions as members of structs.

mojo/public/tools/bindings/generators/cpp_templates/union_declaration.tmpl

Index: mojo/public/tools/bindings/generators/cpp_templates/union_declaration.tmpl
diff --git a/mojo/public/tools/bindings/generators/cpp_templates/union_declaration.tmpl b/mojo/public/tools/bindings/generators/cpp_templates/union_declaration.tmpl
index 6e6fe95a74c1dc207af7f5d252494a1936fd7e14..224a268711fde7591b7a0f5298f7eed2943e008f 100644
--- a/mojo/public/tools/bindings/generators/cpp_templates/union_declaration.tmpl
+++ b/mojo/public/tools/bindings/generators/cpp_templates/union_declaration.tmpl
@@ -5,9 +5,22 @@
 class {{class_name}} {
  public:
   static {{class_name}}* New(mojo::internal::Buffer* buf);
+  {{class_name}}();
+  // Do nothing in the destructor since it won't be called.
+  ~{{class_name}}() {}
+
+  static bool Validate(const void* data,
+                       mojo::internal::BoundsChecker* bounds_checker,
+                       bool claim_memory);

[yzshen1, 2015/02/17 19:34:35]

I think it seems nicer to name it |inlined| because "claim memory
 may not be the only thing affected during validation.

[azani, 2015/02/18 00:27:57]

Done.

 
   static bool Validate(const void* data,
-                       mojo::internal::BoundsChecker* bounds_checker);
+                       mojo::internal::BoundsChecker* bounds_checker) {

[yzshen1, 2015/02/17 19:34:35]

Maybe we should remove this, so that if we don't handle union validation
explicitly (for example, in array/map validation), code compilation will fail?

[azani, 2015/02/18 00:27:57]

Done.

+    return Validate(data, bounds_checker, true);
+  }
+
+  bool is_null() const {

[yzshen1, 2015/02/17 19:34:35]

This is not correct: when the union is non-inlined, the first four bytes are the
size of the union, which is 16. (I wish we used 16 for both inlined/non-inlined
case. It is more consistent and less confusing. But qsr has a different opinion,
maybe we should show him the real code?)

Besides, please avoid adding methods if possible, because with this people are
not able to define a union member |is_null| anymore.

[azani, 2015/02/18 00:27:57]

I think I had a different understanding of the outcome of that thread. I thought
we were not going to store the size (since the size is always 16 bytes and just
reserve the space, including for a "not null" flag. (The not null flag could
probably be implemented differently and I would appreciate some advice. Should I
just use a bit mask? Where should I locate the mask?)
|is_null| doesn't conflict with union members since this is the _Data class
which doesn't have methods for each member.

+    return (flags & 0x1) == 0;
+  }
 
   enum class {{enum_name}} : uint32_t {
 {%  for field in union.fields %}
@@ -33,16 +46,12 @@ class {{class_name}} {
     uint64_t unknown;
   };
 
-  uint32_t reserved;
+  uint32_t flags;

[yzshen1, 2015/02/17 19:34:35]

The name |flags| seems confusing. It is the size of the union in non-lined case.

[azani, 2015/02/18 00:27:57]

See above. I thought we were just reserving the space since the size is set at
16 bytes.

   {{enum_name}} tag;
   Union_ data;
 
   void EncodePointersAndHandles(std::vector<mojo::Handle>* handles);
   void DecodePointersAndHandles(std::vector<mojo::Handle>* handles);
- 
- private:
-  {{class_name}}();
-  ~{{class_name}}() = delete;
 };
 static_assert(sizeof({{class_name}}) == 16,
               "Bad sizeof({{class_name}})");