[PATCH] D27835: Add support to llvm::Twine for storing formatv objects
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 16 08:04:50 PST 2016
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
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.
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits