rsmith added a comment. This looks like a good approach to me.
================ Comment at: include/llvm/Support/TrailingObjects.h:41-42 @@ +40,4 @@ +// +// TODO: Use a variadic template instead of multiple copies of the +// TrailingObjects class? +// ---------------- That sounds like a cleaner interface to me, even if you only implement it for the N = 1 and N = 2 cases. ================ Comment at: include/llvm/Support/TrailingObjects.h:76 @@ +75,3 @@ + // function is instantiated. + static void verifyTrailingObjectsAlignment() { + static_assert(llvm::AlignOf<BaseTy>::Alignment >= ---------------- Have you considered putting a `static_assert` in here for `std::is_final<BaseTy>()`? (We'd need to add the `final` to all the classes where we use this, but that's easier to do when rolling it out than after the fact.) ================ Comment at: include/llvm/Support/TrailingObjects.h:115 @@ +114,3 @@ + // array may have zero or more elements in it. + template <typename T> const T *getTrailingObjects() const { + verifyTrailingObjectsAlignment(); ---------------- I think it'd be useful to also provide an accessor that returns an `ArrayRef<T>`. Perhaps `ArrayRef<T> trailing<T>()` / `const T *trailing_begin<T>()` / `const T *trailing_end<T>()`? (Though maybe the longer names are more appropriate for an internal interface.) ================ Comment at: lib/IR/AttributeImpl.h:201-202 @@ -201,1 +200,4 @@ + // Helper fn for TrailingObjects class. + size_t numTrailingObjects(OverloadToken<IndexAttrPair>) { return NumAttrs; } + ---------------- Do you need this? It looks like you only ever grab the beginning of the trailing array from the base class. http://reviews.llvm.org/D11272 _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits