[PATCH] D93532: [ADT] Add resize_for_overwrite method to SmallVector.

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 18 19:06:18 PST 2020


dexonsmith added a comment.

In D93532#2463995 <https://reviews.llvm.org/D93532#2463995>, @njames93 wrote:

> In D93532#2463881 <https://reviews.llvm.org/D93532#2463881>, @dexonsmith wrote:
>
>> I wonder if this can be tested, something like:
>>
>>   V.push_back(5);
>>   V.pop_back();
>>   V.resize_for_overwrite(V.size() + 1);
>>   EXPECT_EQ(5, V.back());
>>   V.pop_back();
>>   V.resize(V.size() + 1);
>>   EXPECT_EQ(0, V.back());
>
> Was thinking of a good way to test what is essentially undefined behaviour, how will this work under msan??

Given that we own `SmallVector` and `destroy_range` is a no-op, is it undefined behaviour?

If the problem is the call to `new (&*) T;`, then can we add a function in `SmallVectorTemplateCommon` (or whatever it is that's specialized for PODs) called `uninitialized_construct` or something that's definitely a no-op?



================
Comment at: llvm/include/llvm/ADT/SmallVector.h:466
     if (N < this->size()) {
-      this->destroy_range(this->begin()+N, this->end());
+      this->destroy_range(this->begin() + N, this->end());
       this->set_size(N);
----------------
njames93 wrote:
> dexonsmith wrote:
> > This whitespace change seems unrelated; can you commit separately?
> It's not strictly unrelated, but it was a clang format artefact 
Hmm... is it possible that git-diff gives clang-format a different diff than Phab is showing? Phab's version doesn't have this statement changing except for the whitespace... if clang-format is getting the same diff and formatting the line, that sounds like a bug in clang-format; alternatively, I wonder if you ran `clang-format` when the patch was different and at the time it looked like the statement had changed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93532



More information about the llvm-commits mailing list