Fix Mutate() methods of Array<scalar/struct> (override 5508) (#5526)

* Draft with Array specialization (#5508)

* Array specialization + SFINAE to fold copy-paste (#5508)

* Add implicit specialization of Array<scalar> and Array<struct> (#5508)

- Tag dispatching is used for implicit specialization
- Array<scalar> and Array<struct> have different iterators and accessors
- Array<scalar> and Array<struct> have different Mutate() methods

* Add implicit specialization of Array<scalar> and Array<struct> (#5508)

- Tag dispatching is used for implicit specialization
- Array<scalar> and Array<struct> have different iterators and accessors
- Array<scalar> and Array<struct> have different Mutate() methods

include/flatbuffers/base.h

@@ -330,22 +330,20 @@ template<typename T> T EndianSwap(T t) {
   if (sizeof(T) == 1) {   // Compile-time if-then's.
     return t;
   } else if (sizeof(T) == 2) {
-    union { T t; uint16_t i; } u;
-    u.t = t;
+    union { T t; uint16_t i; } u = { t };
     u.i = FLATBUFFERS_BYTESWAP16(u.i);
     return u.t;
   } else if (sizeof(T) == 4) {
-    union { T t; uint32_t i; } u;
-    u.t = t;
+    union { T t; uint32_t i; } u = { t };
     u.i = FLATBUFFERS_BYTESWAP32(u.i);
     return u.t;
   } else if (sizeof(T) == 8) {
-    union { T t; uint64_t i; } u;
-    u.t = t;
+    union { T t; uint64_t i; } u = { t };
     u.i = FLATBUFFERS_BYTESWAP64(u.i);
     return u.t;
   } else {
     FLATBUFFERS_ASSERT(0);
+    return t;
   }
 }
 

include/flatbuffers/flatbuffers.h

@@ -402,18 +402,23 @@ template<typename T> static inline size_t VectorLength(const Vector<T> *v) {
 
 // This is used as a helper type for accessing arrays.
 template<typename T, uint16_t length> class Array {
+  typedef
+      typename flatbuffers::integral_constant<bool, flatbuffers::is_scalar<T>::value>
+          scalar_tag;
+  typedef
+      typename flatbuffers::conditional<scalar_tag::value, T, const T *>::type
+          IndirectHelperType;
+
  public:
-  typedef VectorIterator<T, typename IndirectHelper<T>::return_type>
-      const_iterator;
+  typedef typename IndirectHelper<IndirectHelperType>::return_type return_type;
+  typedef VectorIterator<T, return_type> const_iterator;
   typedef VectorReverseIterator<const_iterator> const_reverse_iterator;
 
-  typedef typename IndirectHelper<T>::return_type return_type;
-
   FLATBUFFERS_CONSTEXPR uint16_t size() const { return length; }
 
   return_type Get(uoffset_t i) const {
     FLATBUFFERS_ASSERT(i < size());
-    return IndirectHelper<T>::Read(Data(), i);
+    return IndirectHelper<IndirectHelperType>::Read(Data(), i);
   }
 
   return_type operator[](uoffset_t i) const { return Get(i); }
@@ -432,22 +437,22 @@ template<typename T, uint16_t length> class Array {
   const_reverse_iterator crbegin() const { return rbegin(); }
   const_reverse_iterator crend() const { return rend(); }
 
-  // Change elements if you have a non-const pointer to this object.
-  void Mutate(uoffset_t i, const T &val) {
-    FLATBUFFERS_ASSERT(i < size());
-    WriteScalar(data() + i, val);
-  }
-
   // Get a mutable pointer to elements inside this array.
-  // @note This method should be only used to mutate arrays of structs followed
-  //  by a @p Mutate operation. For primitive types use @p Mutate directly.
+  // This method used to mutate arrays of structs followed by a @p Mutate
+  // operation. For primitive types use @p Mutate directly.
   // @warning Assignments and reads to/from the dereferenced pointer are not
   //  automatically converted to the correct endianness.
-  T *GetMutablePointer(uoffset_t i) const {
+  typename flatbuffers::conditional<scalar_tag::value, void, T *>::type
+  GetMutablePointer(uoffset_t i) const {
     FLATBUFFERS_ASSERT(i < size());
     return const_cast<T *>(&data()[i]);
   }
 
+  // Change elements if you have a non-const pointer to this object.
+  void Mutate(uoffset_t i, const T &val) {
+    MutateImpl(scalar_tag(), i, val);
+  }
+
   // The raw data in little endian format. Use with care.
   const uint8_t *Data() const { return data_; }
 
@@ -458,6 +463,17 @@ template<typename T, uint16_t length> class Array {
   T *data() { return reinterpret_cast<T *>(Data()); }
 
  protected:
+  void MutateImpl(flatbuffers::integral_constant<bool, true>, uoffset_t i,
+                  const T &val) {
+    FLATBUFFERS_ASSERT(i < size());
+    WriteScalar(data() + i, val);
+  }
+
+  void MutateImpl(flatbuffers::integral_constant<bool, false>, uoffset_t i,
+                  const T &val) {
+    *(GetMutablePointer(i)) = val;
+  }
+
   // This class is only used to access pre-existing data. Don't ever
   // try to construct these manually.
   // 'constexpr' allows us to use 'size()' at compile time.
@@ -475,6 +491,30 @@ template<typename T, uint16_t length> class Array {
   // This class is a pointer. Copying will therefore create an invalid object.
   // Private and unimplemented copy constructor.
   Array(const Array &);
+  Array &operator=(const Array &);
+};
+
+// Specialization for Array[struct] with access using Offset<void> pointer.
+// This specialization used by idl_gen_text.cpp.
+template<typename T, uint16_t length> class Array<Offset<T>, length> {
+  static_assert(flatbuffers::is_same<T, void>::value, "unexpected type T");
+
+ public:
+  const uint8_t *Data() const { return data_; }
+
+  // Make idl_gen_text.cpp::PrintContainer happy.
+  const void *operator[](uoffset_t) const {
+    FLATBUFFERS_ASSERT(false);
+    return nullptr;
+  }
+
+ private:
+  // This class is only used to access pre-existing data.
+  Array();
+  Array(const Array &);
+  Array &operator=(const Array &);
+
+  uint8_t data_[1];
 };
 
 // Lexicographically compare two strings (possibly containing nulls), and

include/flatbuffers/stl_emulation.h

@@ -139,7 +139,11 @@ inline void vector_emplace_back(std::vector<T> *vector, V &&data) {
     template <typename T> using is_floating_point = std::is_floating_point<T>;
     template <typename T> using is_unsigned = std::is_unsigned<T>;
     template <typename T> using make_unsigned = std::make_unsigned<T>;
-  #else
+    template<bool B, class T, class F>
+    using conditional = std::conditional<B, T, F>;
+    template<class T, T v>
+    using integral_constant = std::integral_constant<T, v>;
+#else
     // Map C++ TR1 templates defined by stlport.
     template <typename T> using is_scalar = std::tr1::is_scalar<T>;
     template <typename T, typename U> using is_same = std::tr1::is_same<T,U>;
@@ -157,7 +161,11 @@ inline void vector_emplace_back(std::vector<T> *vector, V &&data) {
     template<> struct make_unsigned<long> { using type = unsigned long; };
     template<>
     struct make_unsigned<long long> { using type = unsigned long long; };
-  #endif  // !FLATBUFFERS_CPP98_STL
+    template<bool B, class T, class F>
+    using conditional = std::tr1::conditional<B, T, F>;
+    template<class T, T v>
+    using integral_constant = std::tr1::integral_constant<T, v>;
+#endif  // !FLATBUFFERS_CPP98_STL
 #else
   // MSVC 2010 doesn't support C++11 aliases.
   template <typename T> struct is_scalar : public std::is_scalar<T> {};
@@ -166,6 +174,10 @@ inline void vector_emplace_back(std::vector<T> *vector, V &&data) {
         public std::is_floating_point<T> {};
   template <typename T> struct is_unsigned : public std::is_unsigned<T> {};
   template <typename T> struct make_unsigned : public std::make_unsigned<T> {};
+  template<bool B, class T, class F>
+  struct conditional : public std::conditional<B, T, F> {};
+  template<class T, T v>
+  struct integral_constant : public std::integral_constant<T, v> {};
 #endif  // defined(FLATBUFFERS_TEMPLATES_ALIASES)
 
 #ifndef FLATBUFFERS_CPP98_STL

tests/test.cpp

@@ -2895,35 +2895,37 @@ void FixedLengthArrayTest() {
   TEST_EQ(mArStruct->b()->size(), 15);
   TEST_EQ(mArStruct->b()->Get(aStruct.b()->size() - 1), -14);
   TEST_EQ(mArStruct->c(), 12);
-  TEST_NOTNULL(mArStruct->d()->Get(0).a());
-  TEST_EQ(mArStruct->d()->Get(0).a()->Get(0), 1);
-  TEST_EQ(mArStruct->d()->Get(0).a()->Get(1), 2);
-  TEST_NOTNULL(mArStruct->d()->Get(1).a());
-  TEST_EQ(mArStruct->d()->Get(1).a()->Get(0), 3);
-  TEST_EQ(mArStruct->d()->Get(1).a()->Get(1), 4);
+  TEST_NOTNULL(mArStruct->d()->Get(0));
+  TEST_NOTNULL(mArStruct->d()->Get(0)->a());
+  TEST_EQ(mArStruct->d()->Get(0)->a()->Get(0), 1);
+  TEST_EQ(mArStruct->d()->Get(0)->a()->Get(1), 2);
+  TEST_NOTNULL(mArStruct->d()->Get(1));
+  TEST_NOTNULL(mArStruct->d()->Get(1)->a());
+  TEST_EQ(mArStruct->d()->Get(1)->a()->Get(0), 3);
+  TEST_EQ(mArStruct->d()->Get(1)->a()->Get(1), 4);
   TEST_NOTNULL(mArStruct->mutable_d()->GetMutablePointer(1));
   TEST_NOTNULL(mArStruct->mutable_d()->GetMutablePointer(1)->mutable_a());
   mArStruct->mutable_d()->GetMutablePointer(1)->mutable_a()->Mutate(1, 5);
-  TEST_EQ(mArStruct->d()->Get(1).a()->Get(1), 5);
-  TEST_EQ(mArStruct->d()->Get(0).b() == MyGame::Example::TestEnum::B, true);
-  TEST_NOTNULL(mArStruct->d()->Get(0).c());
-  TEST_EQ(mArStruct->d()->Get(0).c()->Get(0) == MyGame::Example::TestEnum::C,
+  TEST_EQ(mArStruct->d()->Get(1)->a()->Get(1), 5);
+  TEST_EQ(mArStruct->d()->Get(0)->b() == MyGame::Example::TestEnum::B, true);
+  TEST_NOTNULL(mArStruct->d()->Get(0)->c());
+  TEST_EQ(mArStruct->d()->Get(0)->c()->Get(0) == MyGame::Example::TestEnum::C,
           true);
-  TEST_EQ(mArStruct->d()->Get(0).c()->Get(1) == MyGame::Example::TestEnum::A,
+  TEST_EQ(mArStruct->d()->Get(0)->c()->Get(1) == MyGame::Example::TestEnum::A,
           true);
-  TEST_EQ(mArStruct->d()->Get(0).d()->Get(0),
+  TEST_EQ(mArStruct->d()->Get(0)->d()->Get(0),
           flatbuffers::numeric_limits<int64_t>::max());
-  TEST_EQ(mArStruct->d()->Get(0).d()->Get(1),
+  TEST_EQ(mArStruct->d()->Get(0)->d()->Get(1),
           flatbuffers::numeric_limits<int64_t>::min());
-  TEST_EQ(mArStruct->d()->Get(1).b() == MyGame::Example::TestEnum::C, true);
-  TEST_NOTNULL(mArStruct->d()->Get(1).c());
-  TEST_EQ(mArStruct->d()->Get(1).c()->Get(0) == MyGame::Example::TestEnum::C,
+  TEST_EQ(mArStruct->d()->Get(1)->b() == MyGame::Example::TestEnum::C, true);
+  TEST_NOTNULL(mArStruct->d()->Get(1)->c());
+  TEST_EQ(mArStruct->d()->Get(1)->c()->Get(0) == MyGame::Example::TestEnum::C,
           true);
-  TEST_EQ(mArStruct->d()->Get(1).c()->Get(1) == MyGame::Example::TestEnum::A,
+  TEST_EQ(mArStruct->d()->Get(1)->c()->Get(1) == MyGame::Example::TestEnum::A,
           true);
-  TEST_EQ(mArStruct->d()->Get(1).d()->Get(0),
+  TEST_EQ(mArStruct->d()->Get(1)->d()->Get(0),
           flatbuffers::numeric_limits<int64_t>::min());
-  TEST_EQ(mArStruct->d()->Get(1).d()->Get(1),
+  TEST_EQ(mArStruct->d()->Get(1)->d()->Get(1),
           flatbuffers::numeric_limits<int64_t>::max());
   for (int i = 0; i < mArStruct->b()->size() - 1; i++)
     TEST_EQ(mArStruct->b()->Get(i), i + 1);