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

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 15 18:37:52 PST 2016


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?

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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161216/3859e53c/attachment.html>


More information about the llvm-commits mailing list