[PATCH] D91837: ADT: Fix reference invalidation in SmallVector APIs that pass in a value

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

dexonsmith added inline comments.


================
Comment at: llvm/include/llvm/ADT/SmallVector.h:694
+    // update the pointer values in insert_one_impl.
+    return insert_one_impl(I, T(Elt));
+  }
----------------
njames93 wrote:
> dexonsmith wrote:
> > njames93 wrote:
> > > Shouldn't this use the move constructor `T(std::move(Elt))` Otherwise this adds the unnecessary requirement that T is copyable when calling the xvalue version of insert.
> > The `enable_if` restricts this to when `TakesParamByValue`, which already requires that `T` is copyable (see note above). Do you think an expanded comment would be a good idea?
> > 
> > Or, maybe I've missed your point, let me know if I didn't understand. 
> No I missed the point, disregard it.
Just to confirm, do you think this is good to commit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91837