[PATCH] D19770: Add FixedSizeStorage to TrailingObjects; NFC

[Faisal Vali via llvm-commits]

faisalv added a comment.

Thanks for running with this! =)


================
Comment at: include/llvm/Support/TrailingObjects.h:352
@@ +351,3 @@
+    template <size_t... Counts>
+    using _ =
+        llvm::AlignedCharArray<llvm::AlignOf<BaseTy>::Alignment,
----------------
FWIW - while I see your point - the 'underscore' as a name seems out-of-style - as compared to what i'm used to reading within the llvm/clang code-base.  Personally I wouldn't mind calling it 'Type' or 'Ty'.

Also what are your thoughts about instead of making it a struct (and providing a more generic solution), just making it a template alias, constrained on the enclosing TrailingTypes pack (i.e. specialized for only being able to provide an answer for the enclosing type)?
For e.g - something along the lines of
template<size_t ... Counts> using FixedSizeStorageType = ...totalSizeToAlloc<TrailingTypes...>(Counts...)>;




================
Comment at: include/llvm/Support/TrailingObjects.h:359
@@ +358,3 @@
+  class FixedSizeStorageOwner {
+  public:
+    FixedSizeStorageOwner(BaseTy *p) : p(p) {}
----------------
What are your thoughts on having this guy be templated on <size_t ... Counts> and actually contain the Storage as a data member itself (as opposed to requiring folks to declare it - as you do for your Fixed TPL patch), and hiding the placement new under a forwarding templated constructor that forwards to BaseTy's ctors?

I imagine folks could just use it then as [If BaseTy = TemplateParameterList]:
TemplateParameterList::FixedSizeTrailingCounts<5> TPL(ctors-arguments);





http://reviews.llvm.org/D19770