Implement unions as members of structs.

mojo/public/cpp/bindings/lib/array_internal.h

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef MOJO_PUBLIC_CPP_BINDINGS_LIB_ARRAY_INTERNAL_H_
 #define MOJO_PUBLIC_CPP_BINDINGS_LIB_ARRAY_INTERNAL_H_
 
 #include <new>
 #include <vector>
 
@@ skipping 258 matching lines @@
             VALIDATION_ERROR_UNEXPECTED_NULL_POINTER,
             MakeMessageWithArrayIndex("null in array expecting valid pointers",
                                       header->num_elements,
                                       i).c_str());
         return false;
       }
       if (!ValidateEncodedPointer(&elements[i].offset)) {
         ReportValidationError(VALIDATION_ERROR_ILLEGAL_POINTER);
         return false;
       }
-      if (!ValidateCaller<P, ElementValidateParams>::Run(
-              DecodePointerRaw(&elements[i].offset), bounds_checker)) {
+      if (!ValidateCaller<P, ElementValidateParams, IsUnionDataType<P>::value>::
+              Run(DecodePointerRaw(&elements[i].offset), bounds_checker)) {
         return false;
       }
     }
     return true;
   }
 
  private:
+  template <typename T, typename Params, bool isUnion>

[viettrungluu, 2015/03/04 20:58:13]

nit: (FYI) camelCase is never used in (our) C++

Anyhow, I don't think this extra parameter is needed. I seems to me that if you
can determine from T if it's a union data type (e.g., in the above callsite),
you should be able to write template specializations appropriately too.

[azani, 2015/03/05 23:59:24]

I've updated the case issue. I could be wrong, but I cannot figure out a way to
write the specialization without this extra parameter.

I tried

ValidateCaller<typename EnableIf<IsUnionDataType<T>::value>::type, Params>

However that does not work because T cannot be deduced. Also, I think there may
be some value in the clarity of having an extra parameter rather than something
more complicated. But maybe you have something I could not think of in mind.

[viettrungluu, 2015/03/17 20:08:02]

Maybe something like the example from
http://en.cppreference.com/w/cpp/types/enable_if would work?

// the partial specialization of A is enabled via a template parameter
template<class T, class Enable = void>
class A {}; // primary template
 
template<class T>
class A<T, typename std::enable_if<std::is_floating_point<T>::value >::type> {
}; // specialization for floating point types

There are other ways you could probably make it work. E.g., I believe you could
add something like:

private:
 typedef typename EnableIf<...>::type maybe_enable_;

to the specialized |struct ValidateCaller|.

The point is that you only need to make the substitution fail, not that the
substitution need actually be meaningful.

(Note that the substitution must be in the declaration, and not in a definition,
since the latter are only expanded on use. I.e., putting such a typedef inside
Run() wouldn't work, since things would only blow up when someone tried to call
Run().)

[azani, 2015/03/18 01:04:39]

I have not been able to get these to work.

Putting a typedef in the struct definition doesn't work at all. It appears that
isn't taken into account when doing template specialization partial ordering.

Using EnableIf could possibly be made to work. However, the problem is that if I
use EnableIf only on the union version of ValidateCaller, it creates an
ambiguity between the union version and the struct version. 

But if I also use EnableIf on the struct version of ValidateCaller also, then
another ambiguity arises between the struct version and the array version. I
imagine it can probably be made to work somehow, but IMO it looks like it will
require complicated template magic and so the version I already have is likely
to be a bit easier to understand for readers.

We can chat about it in person tomorrow if you want.

+  struct ValidateCaller {};
+
   template <typename T, typename Params>
-  struct ValidateCaller {
+  struct ValidateCaller<T, Params, false> {
     static bool Run(const void* data, BoundsChecker* bounds_checker) {
       static_assert((IsSame<Params, NoValidateParams>::value),
                     "Struct type should not have array validate params");
 
       return T::Validate(data, bounds_checker);
     }
   };
 
+  template <typename T, typename Params>
+  struct ValidateCaller<T, Params, true> {
+    static bool Run(const void* data, BoundsChecker* bounds_checker) {
+      static_assert((IsSame<Params, NoValidateParams>::value),
+                    "Union type should not have array validate params");
+
+      return T::Validate(data, bounds_checker, true);
+    }
+  };
+
   template <typename Key, typename Value, typename Params>
-  struct ValidateCaller<Map_Data<Key, Value>, Params> {
+  struct ValidateCaller<Map_Data<Key, Value>, Params, false> {
     static bool Run(const void* data, BoundsChecker* bounds_checker) {
       return Map_Data<Key, Value>::template Validate<Params>(data,
                                                              bounds_checker);
     }
   };
 
   template <typename T, typename Params>
-  struct ValidateCaller<Array_Data<T>, Params> {
+  struct ValidateCaller<Array_Data<T>, Params, false> {
     static bool Run(const void* data, BoundsChecker* bounds_checker) {
       return Array_Data<T>::template Validate<Params>(data, bounds_checker);
     }
   };
 };
 
 template <typename T>
 class Array_Data {
  public:
   typedef ArrayDataTraits<T> Traits;
@@ skipping 185 matching lines @@
 
 template <>
 struct WrapperTraits<String, false> {
   typedef String_Data* DataType;
 };
 
 }  // namespace internal
 }  // namespace mojo
 
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_LIB_ARRAY_INTERNAL_H_