[PATCH] D83902: [ADT] Add a range-based version of std::move

Nathan James via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 17 15:38:45 PDT 2020


njames93 marked 5 inline comments as done.
njames93 added inline comments.


================
Comment at: llvm/include/llvm/ADT/STLExtras.h:1550
+OutputIt move(R &&Range, OutputIt Out) {
+  return std::move(adl_begin(Range), adl_end(Range), Out);
+}
----------------
ychen wrote:
> Why std::forward is not used?
Forwarding the Range or the Output iterator. Forwarding the range to adl_begin and adl_end is not a good idea as if you pass an rvalue e.g. `llvm::move(std::move(vec), output)`, You're gonna have a bad time, I guess the output iterator could be forwarded, not sure how much would be gained from that.


================
Comment at: llvm/unittests/ADT/STLExtrasTest.cpp:10
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/Twine.h"
 #include "gtest/gtest.h"
----------------
gamesh411 wrote:
> Why is Twine necessary? Maybe just an unintended addition?
Must be a clangd IWYU inclusion, I never added it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83902





More information about the llvm-commits mailing list