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

[Nathan James via Phabricator via llvm-commits]

njames93 added a comment.

In D93532#2464225 <https://reviews.llvm.org/D93532#2464225>, @dexonsmith wrote:

> 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?

If the call to `new (&*) T;` is tripping sanitisers up, I'm happy to keep that behaviour. After calling resize_for_overwrite it should be required that you write to any newly allocated items before you read them, Explicitly making it a no-op will hide that testing route.
For the record I can't seem to run gtest under msan, there seems to be a false-positive `use-of-uninitialized-value` occurring in basic_string::push_back, obviously not related to this change.



================
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);
----------------
dexonsmith wrote:
> 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?
Thats what I mean when I say an artefact from clang-format. I often format as I go.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93532