[PATCH] D27835: Add support to llvm::Twine for storing formatv objects

[David Blaikie via llvm-commits]

On Thu, Dec 15, 2016 at 6:38 PM Zachary Turner via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Tbh i was surprised there weren't more tests. It seemed kind of arbitrary
> that only the various string cases had concatenation coverage.
>
> Is it safe to assume that if concatenation with empty works for each type,
> and a+b+c works for a single choice of a, b, and c that it works for any
> choice?
>

I think that's probably reasonable, yes - if the codepaths generally agree
with that (it's possible there are quirks in the design that make that
assumed orthogonality not actually orthogonal, for sure - in which case
more detailed testing would be warranted).


>
> Also yes the laziness check should be easy to write a test for
> On Thu, Dec 15, 2016 at 6:15 PM David Blaikie via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
> dblaikie added a comment.
>
> Could you explain what makes each of the test cases interesting? (what
> they're testing that other test cases aren't) - it seems like a bit more
> testing than I'd expect, but I'm probably missing something about the
> mechanics of Twine.
>
> Also - would it be worth adding a test case to demonstrate that it truly
> is lazy? If you introduce a type with a custom printer that has a side
> effect (such as modifying the object as it prints) - then create a Twine of
> a formatv, observe the side effect hasn't occurred, then stringify the
> Twine and observe that the side effect has occurred.
>
>
> https://reviews.llvm.org/D27835
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161216/a97d0220/attachment.html>