[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