TrailingObjects::FixedSizeStorage constexpr fixes + tests

Summary:
This change fixes issues with `LLVM_CONSTEXPR` functions and
`TrailingObjects::FixedSizeStorage`. In particular, some of the
functions marked `LLVM_CONSTEXPR` used by `FixedSizeStorage` were not
implemented such that they evaluate successfully as part of a constant
expression despite constant arguments.

This change also implements a more traditional template-meta path to
accommodate MSVC, and adds unit tests for `FixedSizeStorage`.

Drive-by fix: the access control for members of `TrailingObjectsImpl` is
tightened.

Reviewers: faisalv, rsmith, aaron.ballman

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D22668

llvm-svn: 277270

4 files changed, 127 insertions, 13 deletions

diff --git a/llvm/include/llvm/Support/AlignOf.h b/llvm/include/llvm/Support/AlignOf.h
index af7f20028b6..333bf160c10 100644
--- a/llvm/include/llvm/Support/AlignOf.h
+++ b/llvm/include/llvm/Support/AlignOf.h
@@ -103,7 +103,7 @@ template <typename T> constexpr unsigned AlignOf<T>::Alignment;
 ///  class besides some cosmetic cleanliness.  Example usage:
 ///  alignOf<int>() returns the alignment of an int.
 template <typename T>
-inline unsigned alignOf() { return AlignOf<T>::Alignment; }
+LLVM_CONSTEXPR inline unsigned alignOf() { return AlignOf<T>::Alignment; }
 
 /// \struct AlignedCharArray
 /// \brief Helper for building an aligned character array type.
diff --git a/llvm/include/llvm/Support/MathExtras.h b/llvm/include/llvm/Support/MathExtras.h
index 704561c716e..fdf7f27381b 100644
--- a/llvm/include/llvm/Support/MathExtras.h
+++ b/llvm/include/llvm/Support/MathExtras.h
@@ -674,6 +674,27 @@ inline uint64_t alignTo(uint64_t Value, uint64_t Align, uint64_t Skew = 0) {
   return (Value + Align - 1 - Skew) / Align * Align + Skew;
 }
 
+/// Returns the next integer (mod 2**64) that is greater than or equal to
+/// \p Value and is a multiple of \c Align. \c Align must be non-zero.
+template <uint64_t Align>
+LLVM_CONSTEXPR inline uint64_t alignTo(uint64_t Value) {
+  static_assert(Align != 0u, "Align must be non-zero");
+  return (Value + Align - 1) / Align * Align;
+}
+
+/// \c alignTo for contexts where a constant expression is required.
+/// \sa alignTo
+///
+/// \todo FIXME: remove when \c LLVM_CONSTEXPR becomes really \c constexpr
+template <uint64_t Align>
+struct AlignTo {
+  static_assert(Align != 0u, "Align must be non-zero");
+  template <uint64_t Value>
+  struct from_value {
+    static const uint64_t value = (Value + Align - 1) / Align * Align;
+  };
+};
+
 /// Returns the largest uint64_t less than or equal to \p Value and is
 /// \p Skew mod \p Align. \p Align must be non-zero
 inline uint64_t alignDown(uint64_t Value, uint64_t Align, uint64_t Skew = 0) {
diff --git a/llvm/include/llvm/Support/TrailingObjects.h b/llvm/include/llvm/Support/TrailingObjects.h
index 5a21cddf973..427ae1cb3f6 100644
--- a/llvm/include/llvm/Support/TrailingObjects.h
+++ b/llvm/include/llvm/Support/TrailingObjects.h
@@ -127,29 +127,34 @@ template <typename Ty1, typename Ty2> struct ExtractSecondType {
 
 template <int Align, typename BaseTy, typename TopTrailingObj, typename PrevTy,
           typename... MoreTys>
-struct TrailingObjectsImpl {
+class TrailingObjectsImpl {
   // The main template definition is never used -- the two
   // specializations cover all possibilities.
 };
 
 template <int Align, typename BaseTy, typename TopTrailingObj, typename PrevTy,
           typename NextTy, typename... MoreTys>
-struct TrailingObjectsImpl<Align, BaseTy, TopTrailingObj, PrevTy, NextTy,
-                           MoreTys...>
+class TrailingObjectsImpl<Align, BaseTy, TopTrailingObj, PrevTy, NextTy,
+                          MoreTys...>
     : public TrailingObjectsImpl<Align, BaseTy, TopTrailingObj, NextTy,
                                  MoreTys...> {
 
   typedef TrailingObjectsImpl<Align, BaseTy, TopTrailingObj, NextTy, MoreTys...>
       ParentType;
 
-  // Ensure the methods we inherit are not hidden.
-  using ParentType::getTrailingObjectsImpl;
-  using ParentType::additionalSizeToAllocImpl;
+  struct RequiresRealignment {
+    static const bool value =
+        llvm::AlignOf<PrevTy>::Alignment < llvm::AlignOf<NextTy>::Alignment;
+  };
 
   static LLVM_CONSTEXPR bool requiresRealignment() {
-    return llvm::AlignOf<PrevTy>::Alignment < llvm::AlignOf<NextTy>::Alignment;
+    return RequiresRealignment::value;
   }
 
+protected:
+  // Ensure the inherited getTrailingObjectsImpl is not hidden.
+  using ParentType::getTrailingObjectsImpl;
+
   // These two functions are helper functions for
   // TrailingObjects::getTrailingObjects. They recurse to the left --
   // the result for each type in the list of trailing types depends on
@@ -195,20 +200,37 @@ struct TrailingObjectsImpl<Align, BaseTy, TopTrailingObj, PrevTy, NextTy,
   static LLVM_CONSTEXPR size_t additionalSizeToAllocImpl(
       size_t SizeSoFar, size_t Count1,
       typename ExtractSecondType<MoreTys, size_t>::type... MoreCounts) {
-    return additionalSizeToAllocImpl(
+    return ParentType::additionalSizeToAllocImpl(
         (requiresRealignment()
-             ? llvm::alignTo(SizeSoFar, llvm::alignOf<NextTy>())
+             ? llvm::alignTo<llvm::AlignOf<NextTy>::Alignment>(SizeSoFar)
              : SizeSoFar) +
             sizeof(NextTy) * Count1,
         MoreCounts...);
   }
+
+  // additionalSizeToAllocImpl for contexts where a constant expression is
+  // required.
+  // FIXME: remove when LLVM_CONSTEXPR becomes really constexpr
+  template <size_t SizeSoFar, size_t Count1, size_t... MoreCounts>
+  struct AdditionalSizeToAllocImpl {
+    static_assert(sizeof...(MoreTys) == sizeof...(MoreCounts),
+                  "Number of counts do not match number of types");
+    static const size_t value = ParentType::template AdditionalSizeToAllocImpl<
+        (RequiresRealignment::value
+             ? llvm::AlignTo<llvm::AlignOf<NextTy>::Alignment>::
+                   template from_value<SizeSoFar>::value
+             : SizeSoFar) +
+            sizeof(NextTy) * Count1,
+        MoreCounts...>::value;
+  };
 };
 
 // The base case of the TrailingObjectsImpl inheritance recursion,
 // when there's no more trailing types.
 template <int Align, typename BaseTy, typename TopTrailingObj, typename PrevTy>
-struct TrailingObjectsImpl<Align, BaseTy, TopTrailingObj, PrevTy>
+class TrailingObjectsImpl<Align, BaseTy, TopTrailingObj, PrevTy>
     : public TrailingObjectsAligner<Align> {
+protected:
   // This is a dummy method, only here so the "using" doesn't fail --
   // it will never be called, because this function recurses backwards
   // up the inheritance chain to subclasses.
@@ -218,6 +240,13 @@ struct TrailingObjectsImpl<Align, BaseTy, TopTrailingObj, PrevTy>
     return SizeSoFar;
   }
 
+  // additionalSizeToAllocImpl for contexts where a constant expression is
+  // required.
+  // FIXME: remove when LLVM_CONSTEXPR becomes really constexpr
+  template <size_t SizeSoFar> struct AdditionalSizeToAllocImpl {
+    static const size_t value = SizeSoFar;
+  };
+
   template <bool CheckAlignment> static void verifyTrailingObjectsAlignment() {}
 };
 
@@ -235,7 +264,7 @@ class TrailingObjects : private trailing_objects_internal::TrailingObjectsImpl<
                             BaseTy, TrailingTys...> {
 
   template <int A, typename B, typename T, typename P, typename... M>
-  friend struct trailing_objects_internal::TrailingObjectsImpl;
+  friend class trailing_objects_internal::TrailingObjectsImpl;
 
   template <typename... Tys> class Foo {};
 
@@ -343,6 +372,21 @@ public:
     return sizeof(BaseTy) + ParentType::additionalSizeToAllocImpl(0, Counts...);
   }
 
+  // totalSizeToAlloc for contexts where a constant expression is required.
+  // FIXME: remove when LLVM_CONSTEXPR becomes really constexpr
+  template <typename... Tys> struct TotalSizeToAlloc {
+    static_assert(
+        std::is_same<Foo<TrailingTys...>, Foo<Tys...>>::value,
+        "Arguments to TotalSizeToAlloc do not match with TrailingObjects");
+    template <size_t... Counts> struct with_counts {
+      static_assert(sizeof...(TrailingTys) == sizeof...(Counts),
+                    "Number of counts do not match number of types");
+      static const size_t value =
+          sizeof(BaseTy) +
+          ParentType::template AdditionalSizeToAllocImpl<0, Counts...>::value;
+    };
+  };
+
   /// A type where its ::with_counts template member has a ::type member
   /// suitable for use as uninitialized storage for an object with the given
   /// trailing object counts. The template arguments are similar to those
@@ -360,7 +404,9 @@ public:
   /// \endcode
   template <typename... Tys> struct FixedSizeStorage {
     template <size_t... Counts> struct with_counts {
-      enum { Size = totalSizeToAlloc<Tys...>(Counts...) };
+      enum {
+        Size = TotalSizeToAlloc<Tys...>::template with_counts<Counts...>::value
+      };
       typedef llvm::AlignedCharArray<
           llvm::AlignOf<BaseTy>::Alignment, Size
           > type;
diff --git a/llvm/unittests/Support/TrailingObjectsTest.cpp b/llvm/unittests/Support/TrailingObjectsTest.cpp
index a1d3e7b3c86..65fdcb93623 100644
--- a/llvm/unittests/Support/TrailingObjectsTest.cpp
+++ b/llvm/unittests/Support/TrailingObjectsTest.cpp
@@ -41,6 +41,9 @@ public:
   unsigned numShorts() const { return NumShorts; }
 
   // Pull some protected members in as public, for testability.
+  template <typename... Ty>
+  using FixedSizeStorage = TrailingObjects::FixedSizeStorage<Ty...>;
+
   using TrailingObjects::totalSizeToAlloc;
   using TrailingObjects::additionalSizeToAlloc;
   using TrailingObjects::getTrailingObjects;
@@ -94,6 +97,9 @@ public:
   }
 
   // Pull some protected members in as public, for testability.
+  template <typename... Ty>
+  using FixedSizeStorage = TrailingObjects::FixedSizeStorage<Ty...>;
+
   using TrailingObjects::totalSizeToAlloc;
   using TrailingObjects::additionalSizeToAlloc;
   using TrailingObjects::getTrailingObjects;
@@ -106,7 +112,20 @@ TEST(TrailingObjects, OneArg) {
   EXPECT_EQ(Class1::additionalSizeToAlloc<short>(1), sizeof(short));
   EXPECT_EQ(Class1::additionalSizeToAlloc<short>(3), sizeof(short) * 3);
 
+  EXPECT_EQ(
+      llvm::alignOf<Class1>(),
+      llvm::alignOf<Class1::FixedSizeStorage<short>::with_counts<1>::type>());
+  EXPECT_EQ(sizeof(Class1::FixedSizeStorage<short>::with_counts<1>::type),
+            llvm::alignTo(Class1::totalSizeToAlloc<short>(1),
+                          llvm::alignOf<Class1>()));
   EXPECT_EQ(Class1::totalSizeToAlloc<short>(1), sizeof(Class1) + sizeof(short));
+
+  EXPECT_EQ(
+      llvm::alignOf<Class1>(),
+      llvm::alignOf<Class1::FixedSizeStorage<short>::with_counts<3>::type>());
+  EXPECT_EQ(sizeof(Class1::FixedSizeStorage<short>::with_counts<3>::type),
+            llvm::alignTo(Class1::totalSizeToAlloc<short>(3),
+                          llvm::alignOf<Class1>()));
   EXPECT_EQ(Class1::totalSizeToAlloc<short>(3),
             sizeof(Class1) + sizeof(short) * 3);
 
@@ -131,6 +150,14 @@ TEST(TrailingObjects, TwoArg) {
   EXPECT_EQ((Class2::additionalSizeToAlloc<double, short>(3, 1)),
             sizeof(double) * 3 + sizeof(short));
 
+  EXPECT_EQ(
+      llvm::alignOf<Class2>(),
+      (llvm::alignOf<
+          Class2::FixedSizeStorage<double, short>::with_counts<1, 1>::type>()));
+  EXPECT_EQ(
+      sizeof(Class2::FixedSizeStorage<double, short>::with_counts<1, 1>::type),
+      llvm::alignTo(Class2::totalSizeToAlloc<double, short>(1, 1),
+                    llvm::alignOf<Class2>()));
   EXPECT_EQ((Class2::totalSizeToAlloc<double, short>(1, 1)),
             sizeof(Class2) + sizeof(double) + sizeof(short));
 
@@ -165,6 +192,16 @@ TEST(TrailingObjects, ThreeArg) {
   EXPECT_EQ((Class3::additionalSizeToAlloc<double, short, bool>(1, 1, 3)),
             sizeof(double) + sizeof(short) + 3 * sizeof(bool));
   EXPECT_EQ(sizeof(Class3), llvm::alignTo(1, llvm::alignOf<double>()));
+
+  EXPECT_EQ(llvm::alignOf<Class3>(),
+            (llvm::alignOf<Class3::FixedSizeStorage<
+                 double, short, bool>::with_counts<1, 1, 3>::type>()));
+  EXPECT_EQ(
+      sizeof(Class3::FixedSizeStorage<double, short,
+                                      bool>::with_counts<1, 1, 3>::type),
+      llvm::alignTo(Class3::totalSizeToAlloc<double, short, bool>(1, 1, 3),
+                    llvm::alignOf<Class3>()));
+
   std::unique_ptr<char[]> P(new char[1000]);
   Class3 *C = reinterpret_cast<Class3 *>(P.get());
   EXPECT_EQ(C->getTrailingObjects<double>(), reinterpret_cast<double *>(C + 1));
@@ -186,6 +223,16 @@ TEST(TrailingObjects, Realignment) {
   EXPECT_EQ((Class4::additionalSizeToAlloc<char, long>(1, 1)),
             llvm::alignTo(sizeof(long) + 1, llvm::alignOf<long>()));
   EXPECT_EQ(sizeof(Class4), llvm::alignTo(1, llvm::alignOf<long>()));
+
+  EXPECT_EQ(
+      llvm::alignOf<Class4>(),
+      (llvm::alignOf<
+          Class4::FixedSizeStorage<char, long>::with_counts<1, 1>::type>()));
+  EXPECT_EQ(
+      sizeof(Class4::FixedSizeStorage<char, long>::with_counts<1, 1>::type),
+      llvm::alignTo(Class4::totalSizeToAlloc<char, long>(1, 1),
+                    llvm::alignOf<Class4>()));
+
   std::unique_ptr<char[]> P(new char[1000]);
   Class4 *C = reinterpret_cast<Class4 *>(P.get());
   EXPECT_EQ(C->getTrailingObjects<char>(), reinterpret_cast<char *>(C + 1));