[PATCH] D94739: ADT: Fix reference invalidation in SmallVector::emplace_back and the size+value version of SmallVector::assign

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith created this revision.
dexonsmith added reviewers: njames93, dblaikie.
Herald added subscribers: ributzka, hiraditya.
dexonsmith requested review of this revision.
Herald added a project: LLVM.

Fix the final (I think?) reference invalidation in `SmallVector` that
we need to fix to align with `std::vector`. (There is still some left in
the range insert / append / assign, but the standard calls that UB for
`std::vector` so I think we don't care?)

Avoid reference invalidation during `SmallVector::emplace_back` when one
of the arguments is an internal reference to storage and the vector has
to grow.

For POD-like types, reimplement `emplace_back` in terms of `push_back`.

For other types, split the grow operation in three and construct the new
element in the middle.

Also reimplement `SmallVector:assign(size_type, const T&)` to avoid
invalidating the argument during `clear()` when it's an internal
reference.

There is a minor semantic change when not growing. Previously, assign
would start with `clear()`, and so the old elements were destructed and
all elements of the new vector were copy-constructed.  The new
implementation skips destruction and uses copy-assignment for the prefix
of the new vector less than `size()`. The new semantics match what
libc++ does for `std::vector::assign`.

Note that the following is another possible implementation:

  void assign(size_type NumElts, ValueParamT Elt) {
    std::fill_n(this->begin(), std::min(NumElts, this->size()), Elt);
    this->resize(NumElts, Elt);
  }

The downside of this is that if the vector has to grow there will be
`size()` redundant copy operations.

I should probably split this patch up into three for committing:

1. NFC refactoring to split up `grow()`.
2. Fix `emplace_back()`.
3. Fix `assign()`.

... but I wanted to start the review all together since the patches
will be interdependent.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94739

Files:
  llvm/include/llvm/ADT/SmallVector.h
  llvm/lib/Support/SmallVector.cpp
  llvm/unittests/ADT/SmallVectorTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D94739.316827.patch
Type: text/x-patch
Size: 17604 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210115/c0875658/attachment.bin>