[PATCH] D93779: ADT: Fix reference invalidation in SmallVector::push_back and single-element insert

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

dexonsmith added inline comments.


================
Comment at: llvm/include/llvm/ADT/SmallVector.h:235
+    }
+    This->grow();
+    return ReferencesStorage ? This->begin() + Index : const_cast<T *>(&Elt);
----------------
dexonsmith wrote:
> `reserveForAndGetAddressImpl` is used in `SmallVectorTemplateBase<T>` and needs to call `grow()` here, but the text of the implementation does not depend on whether `T` is trivially copyable.
> 
> https://reviews.llvm.org/D91837#inline-875946 suggested using CRTP, but that seemed to require yet more names for `SmallVector` base classes and it seemed like a bit of a mess.
> 
> Actually... now I realize there is another option I'd missed: just add a CRTP parameter to `SmallVectorTemplateCommon` (no need for a new name). If you think that's worth trying I can try it and post an update.
Ah, no, it's not that easy... we can't easily add new parameters to `SmallVectorTemplateCommon`.
```
/// [...] The extra dummy template argument is used by ArrayRef
/// to avoid unnecessarily requiring T to be complete.
template <typename T, typename = void>
class SmallVectorTemplateCommon
```
I suspect adding a `DerivedT` template parameter would defeat this compile-time optimization.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93779