[PATCH] D12439: Rewrite the TrailingObjects template to provide two new features:

Justin Bogner via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 16 15:02:58 PST 2015


James Y Knight via llvm-commits <llvm-commits at lists.llvm.org> writes:
> jyknight created this revision.
> jyknight added a reviewer: rsmith.
> jyknight added a subscriber: llvm-commits.
>
>  - Automatic alignment of the base type for the alignment requirements
>    of the trailing types.
>
>  - Support for an arbitrary numbers of trailing types, instead of only
>    1 or 2, by using a variadic template implementation.

Well, my brain hurts, but I'm pretty sure this patch is correct. The
understandability cost of this code is pretty high, so I'm not
completely comfortable with it, but as long as you actually need it for
something I suppose it's fine.

> http://reviews.llvm.org/D12439
>
> Files:
>   include/llvm/Support/TrailingObjects.h
>   unittests/Support/TrailingObjectsTest.cpp
>
> Index: unittests/Support/TrailingObjectsTest.cpp
> ===================================================================
> --- unittests/Support/TrailingObjectsTest.cpp
> +++ unittests/Support/TrailingObjectsTest.cpp
> @@ -45,9 +45,10 @@
>    using TrailingObjects::getTrailingObjects;
>  };
>  
> -// Here, there are two singular optional object types appended.
> -// Note that it fails to compile without the alignment spec.
> -class LLVM_ALIGNAS(8) Class2 final : protected TrailingObjects<Class2, double, short> {
> +// Here, there are two singular optional object types appended.  Note
> +// that the alignment of Class2 is automatically increased to account
> +// for the alignment requirements of the trailing objects.

Could you throw an EXPECT_EQ or static assert in the test to prove this
please? I guess we should probably add a ThreeArg test as well now that
NArg is supported (not that it gives much more coverage than the two,
but you know).

> +class Class2 final : protected TrailingObjects<Class2, double, short> {
>    friend TrailingObjects;
>  
>    bool HasShort, HasDouble;
> Index: include/llvm/Support/TrailingObjects.h
> ===================================================================
> --- include/llvm/Support/TrailingObjects.h
> +++ include/llvm/Support/TrailingObjects.h
> @@ -59,6 +59,27 @@
>  
>  namespace llvm {
>  
> +namespace trailing_objects_internal {
> +/// Helper template to calculate the max alignment requirement for a set of
> +/// objects.
> +template <typename First, typename... Rest> class AlignmentCalcHelper {
> +private:
> +  enum {
> +    FirstAlignment = AlignOf<First>::Alignment,
> +    RestAlignment = AlignmentCalcHelper<Rest...>::Alignment,
> +  };
> +
> +public:
> +  enum {
> +    Alignment = FirstAlignment > RestAlignment ? FirstAlignment : RestAlignment
> +  };
> +};
> +
> +template <typename First> class AlignmentCalcHelper<First> {
> +public:
> +  enum { Alignment = AlignOf<First>::Alignment };
> +};
> +
>  /// The base class for TrailingObjects* classes.
>  class TrailingObjectsBase {
>  protected:
> @@ -70,73 +91,211 @@
>    template <typename T> struct OverloadToken {};
>  };
>  
> -// Internally used to indicate that the user didn't supply this value,
> -// so the explicit-specialization for fewer args will be used.
> -class NoTrailingTypeArg {};
>  
> -// TODO: Consider using a single variadic implementation instead of
> -// multiple copies of the TrailingObjects template? [but, variadic
> -// template recursive implementations are annoying...]
> +/// This helper template works-around MSVC 2013's lack of useful
> +/// alignas() support. The argument to LLVM_ALIGNAS(), in MSVC, is
> +/// required to be a literal integer. But, you *can* use template
> +/// specialization to select between a bunch of different LLVM_ALIGNAS
> +/// expressions...
> +template <int Align>
> +class TrailingObjectsAligner : public TrailingObjectsBase {};
> +template <>
> +class LLVM_ALIGNAS(1) TrailingObjectsAligner<1> : public TrailingObjectsBase {};
> +template <>
> +class LLVM_ALIGNAS(2) TrailingObjectsAligner<2> : public TrailingObjectsBase {};
> +template <>
> +class LLVM_ALIGNAS(4) TrailingObjectsAligner<4> : public TrailingObjectsBase {};
> +template <>
> +class LLVM_ALIGNAS(8) TrailingObjectsAligner<8> : public TrailingObjectsBase {};
> +template <>
> +class LLVM_ALIGNAS(16) TrailingObjectsAligner<16> : public TrailingObjectsBase {
> +};
> +template <>
> +class LLVM_ALIGNAS(32) TrailingObjectsAligner<32> : public TrailingObjectsBase {
> +};
> +
> +// Just a little helper for transforming a type pack into the same
> +// number of a different type. e.g.:
> +//   ExtractSecondType<Foo..., int>::type
> +template <typename Ty1, typename Ty2> struct ExtractSecondType {
> +  typedef Ty2 type;
> +};
> +
> +// TrailingObjectsImpl is somewhat complicated, because it is a
> +// recursively inheriting template, in order to handle the template
> +// varargs. Each level of inheritance picks off a single trailing type
> +// then recurses on the rest. The "Align", "BaseTy", and
> +// "TopTrailingObj" arguments are passed through unchanged through the
> +// recursion. "PrevTy" is, at each level, the type handled by the
> +// level right above it.
> +
> +template <int Align, typename BaseTy, typename TopTrailingObj, typename PrevTy,
> +          typename... MoreTys>
> +struct 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...>
> +    : 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;
> +
> +  static void verifyTrailingObjectsAssertions() {
> +    static_assert(llvm::AlignOf<PrevTy>::Alignment >=
> +                      llvm::AlignOf<NextTy>::Alignment,
> +                  "A trailing object requires more alignment than the previous "
> +                  "trailing object provides");
> +
> +    ParentType::verifyTrailingObjectsAssertions();
> +  }
> +
> +  // 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
> +  // the result of calling the function on the type to the
> +  // left. However, the function for the type to the left is
> +  // implemented by a *subclass* of this class, so we invoke it via
> +  // the TopTrailingObj, which is, via the
> +  // curiously-recurring-template-pattern, the most-derived type in
> +  // this recursion, and thus, contains all the overloads.
> +  static const NextTy *
> +  getTrailingObjectsImpl(const BaseTy *Obj,
> +                         TrailingObjectsBase::OverloadToken<NextTy>) {
> +    return reinterpret_cast<const NextTy *>(
> +        TopTrailingObj::getTrailingObjectsImpl(
> +            Obj, TrailingObjectsBase::OverloadToken<PrevTy>()) +
> +        TopTrailingObj::callNumTrailingObjects(
> +            Obj, TrailingObjectsBase::OverloadToken<PrevTy>()));
> +  }
> +
> +  static NextTy *
> +  getTrailingObjectsImpl(BaseTy *Obj,
> +                         TrailingObjectsBase::OverloadToken<NextTy>) {
> +    return reinterpret_cast<NextTy *>(
> +        TopTrailingObj::getTrailingObjectsImpl(
> +            Obj, TrailingObjectsBase::OverloadToken<PrevTy>()) +
> +        TopTrailingObj::callNumTrailingObjects(
> +            Obj, TrailingObjectsBase::OverloadToken<PrevTy>()));
> +  }
> +
> +  // Helper function for TrailingObjects::additionalSizeToAlloc: this
> +  // function recurses to superclasses, each of which requires one
> +  // fewer size_t argument, and adds its own size.
> +  static LLVM_CONSTEXPR size_t additionalSizeToAllocImpl(
> +      size_t Count1, typename ExtractSecondType<MoreTys, size_t>::type... MoreCounts) {
> +    return sizeof(NextTy) * Count1 + additionalSizeToAllocImpl(MoreCounts...);
> +  }
> +};
> +
> +// 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>
> +    : public TrailingObjectsAligner<Align> {
> +  // 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.
> +  static void getTrailingObjectsImpl();
> +
> +  static LLVM_CONSTEXPR size_t additionalSizeToAllocImpl() { return 0; }
> +
> +  static void verifyTrailingObjectsAssertions() {}
> +};
> +
> +} // end namespace trailing_objects_internal
> +
> +
> +// Finally, the main type defined in this file, the one intended for users...
> +
> +/// See the file comment for details on the usage of the
> +/// TrailingObjects type.
> +template <typename BaseTy, typename... TrailingTys>
> +class TrailingObjects : private trailing_objects_internal::TrailingObjectsImpl<
> +                            trailing_objects_internal::AlignmentCalcHelper<
> +                                TrailingTys...>::Alignment,
> +                            BaseTy, TrailingObjects<BaseTy, TrailingTys...>,
> +                            BaseTy, TrailingTys...> {
> +
> +  template <int A, typename B, typename T, typename P, typename... M>
> +  friend struct trailing_objects_internal::TrailingObjectsImpl;
> +
> +  template <typename... Tys> class Foo {};
> +
> +  typedef trailing_objects_internal::TrailingObjectsImpl<
> +      trailing_objects_internal::AlignmentCalcHelper<TrailingTys...>::Alignment,
> +      BaseTy, TrailingObjects<BaseTy, TrailingTys...>, BaseTy,
> +      TrailingTys...> ParentType;
> +  using TrailingObjectsBase = trailing_objects_internal::TrailingObjectsBase;
> +
> +  using ParentType::getTrailingObjectsImpl;
>  
> -/// This is the two-type version of the TrailingObjects template; see
> -/// file docstring for details.
> -template <typename BaseTy, typename TrailingTy1,
> -          typename TrailingTy2 = NoTrailingTypeArg>
> -class TrailingObjects : public TrailingObjectsBase {
> -private:
>    // Contains static_assert statements for the alignment of the
>    // types. Must not be at class-level, because BaseTy isn't complete
>    // at class instantiation time, but will be by the time this
> -  // function is instantiated.
> +  // function is instantiated. Recurses through the superclasses.
>    static void verifyTrailingObjectsAssertions() {
> -    static_assert(llvm::AlignOf<BaseTy>::Alignment >=
> -                      llvm::AlignOf<TrailingTy1>::Alignment,
> -                  "TrailingTy1 requires more alignment than BaseTy provides");
> -    static_assert(
> -        llvm::AlignOf<TrailingTy1>::Alignment >=
> -            llvm::AlignOf<TrailingTy2>::Alignment,
> -        "TrailingTy2 requires more alignment than TrailingTy1 provides");
> -
>  #ifdef LLVM_IS_FINAL
>      static_assert(LLVM_IS_FINAL(BaseTy), "BaseTy must be final.");
>  #endif
> +    ParentType::verifyTrailingObjectsAssertions();
>    }
>  
> -  // The next four functions are internal helpers for getTrailingObjects.
> -  static const TrailingTy1 *getTrailingObjectsImpl(const BaseTy *Obj,
> -                                                   OverloadToken<TrailingTy1>) {
> -    return reinterpret_cast<const TrailingTy1 *>(Obj + 1);
> +  // These two methods are the base of the recursion for this method.
> +  static const BaseTy *
> +  getTrailingObjectsImpl(const BaseTy *Obj,
> +                         TrailingObjectsBase::OverloadToken<BaseTy>) {
> +    return Obj;
>    }
>  
> -  static TrailingTy1 *getTrailingObjectsImpl(BaseTy *Obj,
> -                                             OverloadToken<TrailingTy1>) {
> -    return reinterpret_cast<TrailingTy1 *>(Obj + 1);
> +  static BaseTy *
> +  getTrailingObjectsImpl(BaseTy *Obj,
> +                         TrailingObjectsBase::OverloadToken<BaseTy>) {
> +    return Obj;
>    }
>  
> -  static const TrailingTy2 *getTrailingObjectsImpl(const BaseTy *Obj,
> -                                                   OverloadToken<TrailingTy2>) {
> -    return reinterpret_cast<const TrailingTy2 *>(
> -        getTrailingObjectsImpl(Obj, OverloadToken<TrailingTy1>()) +
> -        Obj->numTrailingObjects(OverloadToken<TrailingTy1>()));
> +  // callNumTrailingObjects simply calls numTrailingObjects on the
> +  // provided Obj -- except when the type being queried is BaseTy
> +  // itself. There is always only one of the base object, so that case
> +  // is handled here. (An additional benefit of indirecting through
> +  // this function is that consumers only say "friend
> +  // TrailingObjects", and thus, only this class itself can call the
> +  // numTrailingObjects function.)
> +  static size_t
> +  callNumTrailingObjects(const BaseTy *Obj,
> +                         TrailingObjectsBase::OverloadToken<BaseTy>) {
> +    return 1;
>    }
>  
> -  static TrailingTy2 *getTrailingObjectsImpl(BaseTy *Obj,
> -                                             OverloadToken<TrailingTy2>) {
> -    return reinterpret_cast<TrailingTy2 *>(
> -        getTrailingObjectsImpl(Obj, OverloadToken<TrailingTy1>()) +
> -        Obj->numTrailingObjects(OverloadToken<TrailingTy1>()));
> +  template <typename T>
> +  static size_t callNumTrailingObjects(const BaseTy *Obj,
> +                                       TrailingObjectsBase::OverloadToken<T>) {
> +    return Obj->numTrailingObjects(TrailingObjectsBase::OverloadToken<T>());
>    }
>  
> -protected:
> +public:
> +  // make this (privately inherited) class public.
> +  using TrailingObjectsBase::OverloadToken;
> +
>    /// Returns a pointer to the trailing object array of the given type
>    /// (which must be one of those specified in the class template). The
>    /// array may have zero or more elements in it.
>    template <typename T> const T *getTrailingObjects() const {
>      verifyTrailingObjectsAssertions();
>      // Forwards to an impl function with overloads, since member
>      // function templates can't be specialized.
> -    return getTrailingObjectsImpl(static_cast<const BaseTy *>(this),
> -                                  OverloadToken<T>());
> +    return this->getTrailingObjectsImpl(
> +        static_cast<const BaseTy *>(this),
> +        TrailingObjectsBase::OverloadToken<T>());
>    }
>  
>    /// Returns a pointer to the trailing object array of the given type
> @@ -146,83 +305,34 @@
>      verifyTrailingObjectsAssertions();
>      // Forwards to an impl function with overloads, since member
>      // function templates can't be specialized.
> -    return getTrailingObjectsImpl(static_cast<BaseTy *>(this),
> -                                  OverloadToken<T>());
> +    return this->getTrailingObjectsImpl(
> +        static_cast<BaseTy *>(this), TrailingObjectsBase::OverloadToken<T>());
>    }
>  
>    /// Returns the size of the trailing data, if an object were
>    /// allocated with the given counts (The counts are in the same order
>    /// as the template arguments). This does not include the size of the
>    /// base object.  The template arguments must be the same as those
>    /// used in the class; they are supplied here redundantly only so
>    /// that it's clear what the counts are counting in callers.
> -  template <typename Ty1, typename Ty2,
> -            typename std::enable_if<std::is_same<Ty1, TrailingTy1>::value &&
> -                                        std::is_same<Ty2, TrailingTy2>::value,
> -                                    int>::type = 0>
> -  static LLVM_CONSTEXPR size_t additionalSizeToAlloc(size_t Count1, size_t Count2) {
> -    return sizeof(TrailingTy1) * Count1 + sizeof(TrailingTy2) * Count2;
> +  template <typename... Tys>
> +  static LLVM_CONSTEXPR typename std::enable_if<
> +      std::is_same<Foo<TrailingTys...>, Foo<Tys...>>::value, size_t>::type
> +      additionalSizeToAlloc(typename trailing_objects_internal::ExtractSecondType<
> +                            TrailingTys, size_t>::type... Counts) {
> +    return ParentType::additionalSizeToAllocImpl(Counts...);
>    }
>  
>    /// Returns the total size of an object if it were allocated with the
>    /// given trailing object counts. This is the same as
>    /// additionalSizeToAlloc, except it *does* include the size of the base
>    /// object.
> -  template <typename Ty1, typename Ty2>
> -  static LLVM_CONSTEXPR size_t totalSizeToAlloc(size_t Count1, size_t Count2) {
> -    return sizeof(BaseTy) + additionalSizeToAlloc<Ty1, Ty2>(Count1, Count2);
> -  }
> -};
> -
> -/// This is the one-type version of the TrailingObjects template. See
> -/// the two-type version for more documentation.
> -template <typename BaseTy, typename TrailingTy1>
> -class TrailingObjects<BaseTy, TrailingTy1, NoTrailingTypeArg>
> -    : public TrailingObjectsBase {
> -private:
> -  static void verifyTrailingObjectsAssertions() {
> -    static_assert(llvm::AlignOf<BaseTy>::Alignment >=
> -                      llvm::AlignOf<TrailingTy1>::Alignment,
> -                  "TrailingTy1 requires more alignment than BaseTy provides");
> -
> -#ifdef LLVM_IS_FINAL
> -    static_assert(LLVM_IS_FINAL(BaseTy), "BaseTy must be final.");
> -#endif
> -  }
> -
> -  static const TrailingTy1 *getTrailingObjectsImpl(const BaseTy *Obj,
> -                                                   OverloadToken<TrailingTy1>) {
> -    return reinterpret_cast<const TrailingTy1 *>(Obj + 1);
> -  }
> -
> -  static TrailingTy1 *getTrailingObjectsImpl(BaseTy *Obj,
> -                                             OverloadToken<TrailingTy1>) {
> -    return reinterpret_cast<TrailingTy1 *>(Obj + 1);
> -  }
> -
> -protected:
> -  template <typename T> const T *getTrailingObjects() const {
> -    verifyTrailingObjectsAssertions();
> -    return getTrailingObjectsImpl(static_cast<const BaseTy *>(this),
> -                                  OverloadToken<T>());
> -  }
> -
> -  template <typename T> T *getTrailingObjects() {
> -    verifyTrailingObjectsAssertions();
> -    return getTrailingObjectsImpl(static_cast<BaseTy *>(this),
> -                                  OverloadToken<T>());
> -  }
> -
> -  template <typename Ty1,
> -            typename std::enable_if<std::is_same<Ty1, TrailingTy1>::value,
> -                                    int>::type = 0>
> -  static LLVM_CONSTEXPR size_t additionalSizeToAlloc(size_t Count1) {
> -    return sizeof(TrailingTy1) * Count1;
> -  }
> -
> -  template <typename Ty1>
> -  static LLVM_CONSTEXPR size_t totalSizeToAlloc(size_t Count1) {
> -    return sizeof(BaseTy) + additionalSizeToAlloc<Ty1>(Count1);
> +  template <typename... Tys>
> +  static LLVM_CONSTEXPR typename std::enable_if<
> +      std::is_same<Foo<TrailingTys...>, Foo<Tys...>>::value, size_t>::type
> +      totalSizeToAlloc(typename trailing_objects_internal::ExtractSecondType<
> +                       TrailingTys, size_t>::type... Counts) {
> +    return sizeof(BaseTy) + ParentType::additionalSizeToAllocImpl(Counts...);
>    }
>  };
>  
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list