[PATCH] D115443: [ADT] Weaken the initialization condition in SmallVector::resize_for_overwrite

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/include/llvm/ADT/SmallVector.h:601
     this->reserve(N);
-    for (auto I = this->end(), E = this->begin() + N; I != E; ++I)
-      if (ForOverwrite)
-        new (&*I) T;
-      else
+    if (!ForOverwrite || !is_trivially_copyable<T>::value)
+      for (auto I = this->end(), E = this->begin() + N; I != E; ++I)
----------------
dexonsmith wrote:
> dblaikie wrote:
> > Should this be `is_trivially_constructible<T>`?
> IIUC, the intent is:
> - Trivial to overwrite, for moving/copying values in right after.
> - Trivial to destroy, if followed by a truncate because `N` was bigger than necessary.
> - UB to read/use if not copied/overwritten.
> 
> This would allow it to skip constructors for types like `SourceLocation`, which is not trivially constructible, since it should be safe to overwrite or destroy an un-constructed value.
Ah, pity. :/ It'd be rather nice/simple/tidy if this only applied to trivially constructible things (where "new (x) T" is a no-op anyway) but I guess there's enough types we want to use this with that benefit from safe default constructibility in other places... :/ Ah well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115443