[PATCH] D83305: [ADT] Fix join_impl using the wrong size when calculating total length

Nathan James via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 29 15:30:06 PST 2020


njames93 added a comment.

In D83305#2474470 <https://reviews.llvm.org/D83305#2474470>, @dblaikie wrote:

> In D83305#2474466 <https://reviews.llvm.org/D83305#2474466>, @njames93 wrote:
>
>> In D83305#2474378 <https://reviews.llvm.org/D83305#2474378>, @dblaikie wrote:
>>
>>> Any chance of testing this? Could test the 'capacity' after join to check its the right size?
>>
>> How do you suppose I do that, maybe expensive checks to get the capacity after reserve and after all items have been added and ensure they are the same, otherwise the string grew during appending.
>
> Oh, nah, nothing that'd require expensive checks - you could add an assert that size == Len at the end of the function (would have to have some way of avoiding the case for short results that are below the starting capacity, though... perhaps checking if capacity grew over the reserve call, and only asserting in that case)). But I was thinking more along the lines of a unit test that tests join and checks the capacity isn't bigger than the size for a few hardcoded samples.

Oh that assert is fine, the unit test won't be a good test. As we don't control std string, it's capacity for calls to reserve will be implementation defined


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83305/new/

https://reviews.llvm.org/D83305



More information about the llvm-commits mailing list