[PATCH] D28429: Remove the restriction of ten types on AligedCharArrayUnion

[David Blaikie via llvm-commits]

On Fri, Jan 6, 2017 at 6:38 PM Davide Italiano via Phabricator <
reviews at reviews.llvm.org> wrote:

> davide requested changes to this revision.
> davide added a reviewer: davide.
> davide added a comment.
> This revision now requires changes to proceed.
>
> Can you add an unittest to make sure we handle correctly > 10 arguments?
>

It's sort of an interesting question, isn't it - with the new
implementation, 10 arguments isn't special so should we bother testing it?
(if we test it with 2, there's no reason to believe it'd be different for
10 or 20)

I don't really know the right answer to this sort of question - in some
cases like this (it comes up semi-often - you refactor some code to address
a bug, but the bug/test isn't really relevant to the new implementation)
I've included the test, sometimes not.

Open to opinions/perspectives here, for sure.

- Dave


>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D28429
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170107/633c7317/attachment.html>