[PATCH] D19770: Add FixedSizeStorage to TrailingObjects; NFC

[Aaron Ballman via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

I think this LGTM (with the const overload), but you should poke Richard to see what his thoughts are before committing.


================
Comment at: include/llvm/Support/TrailingObjects.h:353
@@ +352,3 @@
+  ///
+  /// \code{.cpp}
+  ///
----------------
hubert.reinterpretcast wrote:
> aaron.ballman wrote:
> > That's novel (we use it in two other places from what I can tell).
> This is a documented Doxygen command:
> https://www.stack.nl/~dimitri/doxygen/manual/commands.html#cmdcode
> 
> The presence of the file extension in the existing use is mostly harmless to older versions of Doxygen:
> http://www.llvm.org/docs/doxygen/html/classllvm_1_1TypeBuilder.html#details
> 
> Since this is present in a .h header, I am not sure we will get C++ (as opposed to C) syntax coloring reliably otherwise.
> 
Huh, interesting. I'm not opposed to it, though I prefer consistency. I say leave it, though.

================
Comment at: include/llvm/Support/TrailingObjects.h:378
@@ +377,3 @@
+
+    BaseTy *get() { return p; }
+
----------------
hubert.reinterpretcast wrote:
> aaron.ballman wrote:
> > Should we have a const overload for this?
> I can add one. It will return a pointer-to-const since the semantics of this class is that it owns the memory.
> 
That's what I was expecting, so it sounds good to me.


http://reviews.llvm.org/D19770