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

James Y Knight via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 17 11:37:52 PST 2015


On Wed, Dec 16, 2015 at 6:02 PM, Justin Bogner <mail at justinbogner.com>
wrote:

> 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


Yeah, I don't like recursive templates either -- which is why I avoided
writing it that way in the first place -- but it seemed the less bad
solution than manually duplicating it a few more times. Some of the uses
I'm adding in clang use more than two trailing types.

> +// 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?


It's indirectly asserted via the test on sizeof(Class2), but a direct test
is be better, I'll add.


> 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).
>

Will add that too.

Thanks for the review!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151217/71bcf4b2/attachment.html>


More information about the llvm-commits mailing list