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
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
