[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 Jan 12 15:56:48 PST 2021


njames93 added a comment.

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

> The spec seems to be a bit more precise than that (using cppreference, which isn't authoritative, but more readable): "If new_cap is greater than the current capacity(), new storage is allocated, and capacity() is made equal or greater than new_cap."
>
> So a unit test that joins some large strings (if it wanted to be fancy, it could even generate a string intentionally larger than std::string's default capacity, whatever it happens to be) and if the resulting string has a larger capacity than std::string's default capacity, verify that the capacity is exactly equal to the string length.

That still sounds like it'll cause some issues. The main thing we want to test (which is the purpose of this patch) is that the string didn't grow while we are appending all the pieces. It's impossible to tell externally if that happened as reserve may choose to over allocate.
Putting an assert in the function that the capacity before hasn't changed should be more than enough, WDYT?


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