[Support] Forward rvalues instead of moving them

[gAlfonso-bit]

Summary

Some of these rvalues are moved conditionally, when they should be forwarded instead.

[MaskRay]

llvm/include/llvm/Support/raw_ostream.h

407 ↗

(On Diff #480660)

This && use seems wrong.

[gAlfonso-bit]

llvm/include/llvm/Support/raw_ostream.h

407 ↗

(On Diff #480660)

How, if I may ask?

[gAlfonso-bit]

This comment was removed by gAlfonso-bit.

[MaskRay]

Please spend more time on each call site instead of applying easy fixes from some static analyzer. For example, operator<<(OStream &&OS, const T &Value) { is for value only (see !std::is_reference). std::move is intended.

[gAlfonso-bit]

This comment was removed by gAlfonso-bit.

[gAlfonso-bit]

This comment was removed by gAlfonso-bit.

llvm/include/llvm/ExecutionEngine/Orc/ExecutorProcessControl.h

public:
   /// runInPlace function or a RunWithDispatch object.
   class IncomingWFRHandler {
     friend class ExecutorProcessControl;
   public:
     IncomingWFRHandler() = default;
     explicit operator bool() const { return !!H; }
     void operator()(shared::WrapperFunctionResult WFR) { H(std::move(WFR)); }
   private:
-    template <typename FnT> IncomingWFRHandler(FnT &&Fn)
-      : H(std::forward<FnT>(Fn)) {}
+    template <typename FnT>
+    IncomingWFRHandler(FnT &&Fn) : H(std::forward<FnT>(Fn)) {}
 
     unique_function<void(shared::WrapperFunctionResult)> H;
   };
 
   /// Constructs an IncomingWFRHandler from a function object that is callable
   /// as void(shared::WrapperFunctionResult). The function object will be called
   /// directly. This should be used with care as it may block listener threads
   /// in remote EPCs. It is only suitable for simple tasks (e.g. setting a
   /// future), or for performing some quick analysis before dispatching "real"
   /// work as a Task.
   class RunInPlace {
   public:
-    template <typename FnT>
-    IncomingWFRHandler operator()(FnT &&Fn) {
+    template <typename FnT> IncomingWFRHandler operator()(FnT &&Fn) {
       return IncomingWFRHandler(std::forward<FnT>(Fn));
     }
   };
 
   /// Constructs an IncomingWFRHandler from a function object by creating a new
   /// function object that dispatches the original using a TaskDispatcher,
   /// wrapping the original as a GenericNamedTask.
   ///
   /// This is the default approach for running WFR handlers.
   class RunAsTask {
   public:
     RunAsTask(TaskDispatcher &D) : D(D) {}
 
-    template <typename FnT>
-    IncomingWFRHandler operator()(FnT &&Fn) {
-      return IncomingWFRHandler(
-          [&D = this->D, Fn = std::move(Fn)]
-          (shared::WrapperFunctionResult WFR) mutable {
-              D.dispatch(
-                makeGenericNamedTask(
-                    [Fn = std::move(Fn), WFR = std::move(WFR)]() mutable {
-                      Fn(std::move(WFR));
-                    }, "WFR handler task"));
-          });
+    template <typename FnT> IncomingWFRHandler operator()(FnT &&Fn) {
+      return IncomingWFRHandler([&D = this->D, Fn = std::forward<FnT>(Fn)](
+                                    shared::WrapperFunctionResult WFR) mutable {
+        D.dispatch(makeGenericNamedTask(
+            },
+            "WFR handler task"));
     }
+
   private:
     TaskDispatcher &D;
   };
 
   /// APIs for manipulating memory in the target process.
   class MemoryAccess {
   public:
     /// Callback function for asynchronous writes.
public:
   /// runtime can use to call functions in the JIT.
   struct JITDispatchInfo {
     ExecutorAddr JITDispatchFunction;
     ExecutorAddr JITDispatchContext;
   };
 
   ExecutorProcessControl(std::shared_ptr<SymbolStringPool> SSP,
                          std::unique_ptr<TaskDispatcher> D)
       : SSP(std::move(SSP)), D(std::move(D)) {}
 
   virtual ~ExecutorProcessControl();
 
   /// Return the ExecutionSession associated with this instance.
   /// Not callable until the ExecutionSession has been associated.
   ExecutionSession &getExecutionSession() {
     assert(ES && "No ExecutionSession associated yet");
     return *ES;
virtual void callWrapperAsync(ExecutorAddr WrapperFnAddr,
                                 IncomingWFRHandler OnComplete,
                                 ArrayRef<char> ArgBuffer) = 0;
 
   /// Run a wrapper function in the executor using the given Runner to dispatch
   /// OnComplete when the result is ready.
   template <typename RunPolicyT, typename FnT>
   void callWrapperAsync(RunPolicyT &&Runner, ExecutorAddr WrapperFnAddr,
                         FnT &&OnComplete, ArrayRef<char> ArgBuffer) {
-    callWrapperAsync(
-        WrapperFnAddr, Runner(std::forward<FnT>(OnComplete)), ArgBuffer);
+    callWrapperAsync(WrapperFnAddr, Runner(std::forward<FnT>(OnComplete)),
+                     ArgBuffer);
   }
 
   /// Run a wrapper function in the executor. OnComplete will be dispatched
   /// as a GenericNamedTask using this instance's TaskDispatch object.
   template <typename FnT>
   void callWrapperAsync(ExecutorAddr WrapperFnAddr, FnT &&OnComplete,
                         ArrayRef<char> ArgBuffer) {
     callWrapperAsync(RunAsTask(*D), WrapperFnAddr,
                      std::forward<FnT>(OnComplete), ArgBuffer);
   }
 
   /// Run a wrapper function in the executor. The wrapper function should be
   /// callable as:
   ///
   /// \code{.cpp}
   ///   CWrapperFunctionResult fn(uint8_t *Data, uint64_t Size);
   /// \endcode{.cpp}
   shared::WrapperFunctionResult callWrapper(ExecutorAddr WrapperFnAddr,
                                             ArrayRef<char> ArgBuffer) {
     std::promise<shared::WrapperFunctionResult> RP;
     auto RF = RP.get_future();
     callWrapperAsync(
         RunInPlace(), WrapperFnAddr,
-        [&](shared::WrapperFunctionResult R) {
-          RP.set_value(std::move(R));
-        }, ArgBuffer);
+        [&](shared::WrapperFunctionResult R) { RP.set_value(std::move(R)); },
+        ArgBuffer);
     return RF.get();
   }
 
   /// Run a wrapper function using SPS to serialize the arguments and
   /// deserialize the results.
   template <typename SPSSignature, typename RunPolicyT, typename SendResultT,
             typename... ArgTs>
   void callSPSWrapperAsync(RunPolicyT &&Runner, ExecutorAddr WrapperFnAddr,
                            SendResultT &&SendResult, const ArgTs &...Args) {
     shared::WrapperFunction<SPSSignature>::callAsync(
-        [this, WrapperFnAddr, Runner = std::move(Runner)]
-        (auto &&SendResult, const char *ArgData, size_t ArgSize) mutable {
+        [this, WrapperFnAddr, Runner = std::forward<RunPolicyT>(Runner)](
+            auto &&SendResult, const char *ArgData, size_t ArgSize) mutable {
           this->callWrapperAsync(std::move(Runner), WrapperFnAddr,
-                                 std::move(SendResult),
+                                 std::forward<decltype(SendResult)>(SendResult),
                                  ArrayRef<char>(ArgData, ArgSize));
         },
         std::forward<SendResultT>(SendResult), Args...);
   }
 
   /// Run a wrapper function using SPS to serialize the arguments and
   /// deserialize the results.
   template <typename SPSSignature, typename SendResultT, typename... ArgTs>

llvm/include/llvm/ExecutionEngine/Orc/Shared/WrapperFunctionUtils.h

static void applyAsync(HandlerT &&H,
                          const char *ArgData, size_t ArgSize) {
     ArgTuple Args;
     if (!deserialize(ArgData, ArgSize, Args, ArgIndices{})) {
       SendWrapperFunctionResult(WrapperFunctionResult::createOutOfBandError(
           "Could not deserialize arguments for wrapper function call"));
       return;
     }
 
-    auto SendResult =
-        [SendWFR = std::move(SendWrapperFunctionResult)](auto Result) mutable {
-          using ResultT = decltype(Result);
-          SendWFR(ResultSerializer<ResultT>::serialize(std::move(Result)));
-        };
+    auto SendResult = [SendWFR = std::forward<SendWrapperFunctionResultT>(
+                           SendWrapperFunctionResult)](auto Result) mutable {
 
     callAsync(std::forward<HandlerT>(H), std::move(SendResult), std::move(Args),
               ArgIndices{});
   }
 
 private:
   template <std::size_t... I>
   static bool deserialize(const char *ArgData, size_t ArgSize, ArgTuple &Args,
                           std::index_sequence<I...>) {
     SPSInputBuffer IB(ArgData, ArgSize);
     return SPSArgList<SPSTagTs...>::deserialize(IB, std::get<I>(Args)...);
   }
 
   template <typename HandlerT, typename SerializeAndSendResultT,
             typename ArgTupleT, std::size_t... I>
   static void callAsync(HandlerT &&H,
                         SerializeAndSendResultT &&SerializeAndSendResult,
                         ArgTupleT Args, std::index_sequence<I...>) {
     (void)Args; // Silence a buggy GCC warning.
-    return std::forward<HandlerT>(H)(std::move(SerializeAndSendResult),
-                                     std::move(std::get<I>(Args))...);
+    return std::forward<HandlerT>(H)(
+        std::forward<SerializeAndSendResultT>(SerializeAndSendResult),
   }
 };
 
 // Map function pointers to function types.
 template <typename RetT, typename... ArgTs,
           template <typename> class ResultSerializer, typename... SPSTagTs>
 class WrapperFunctionAsyncHandlerHelper<RetT (*)(ArgTs...), ResultSerializer,
                                         SPSTagTs...>
auto ArgBuffer =
             Args...);
     if (auto *ErrMsg = ArgBuffer.getOutOfBandError()) {
       SendDeserializedResult(
           make_error<StringError>(ErrMsg, inconvertibleErrorCode()),
           detail::ResultDeserializer<SPSRetTagT, RetT>::makeValue());
       return;
     }
 
-    auto SendSerializedResult = [SDR = std::move(SendDeserializedResult)](
+    auto SendSerializedResult = [SDR = std::forward<SendDeserializedResultFn>(
+                                     SendDeserializedResult)](
                                     WrapperFunctionResult R) mutable {
       RetT RetVal = detail::ResultDeserializer<SPSRetTagT, RetT>::makeValue();
       detail::ResultDeserializer<SPSRetTagT, RetT>::makeSafe(RetVal);
 
       if (auto *ErrMsg = R.getOutOfBandError()) {
         SDR(make_error<StringError>(ErrMsg, inconvertibleErrorCode()),
             std::move(RetVal));
         return;
public:
 
   template <typename AsyncCallerFn, typename SendDeserializedResultFn,
             typename... ArgTs>
   static void callAsync(AsyncCallerFn &&Caller,
                         SendDeserializedResultFn &&SendDeserializedResult,
                         const ArgTs &...Args) {
     WrapperFunction<SPSEmpty(SPSTagTs...)>::callAsync(
         std::forward<AsyncCallerFn>(Caller),
-        [SDR = std::move(SendDeserializedResult)](Error SerializeErr,
-                                                  SPSEmpty E) mutable {
+        [SDR = std::forward<SendDeserializedResultFn>(SendDeserializedResult)](
+            Error SerializeErr, SPSEmpty E) mutable {
           SDR(std::move(SerializeErr));
         },
         Args...);
   }
 
   using WrapperFunction<SPSEmpty(SPSTagTs...)>::handle;
   using WrapperFunction<SPSEmpty(SPSTagTs...)>::handleAsync;
 };

llvm/include/llvm/Support/MSVCErrorWorkarounds.h

public:
   }
 
   MSVCPExpected(Error Err) : Expected<T>(std::move(Err)) {}
 
   template <typename OtherT>
   MSVCPExpected(
       OtherT &&Val,
       std::enable_if_t<std::is_convertible<OtherT, T>::value> * = nullptr)
-      : Expected<T>(std::move(Val)) {}
+      : Expected<T>(std::forward<OtherT>(Val)) {}
 
   template <class OtherT>
   MSVCPExpected(
       Expected<OtherT> &&Other,
       std::enable_if_t<std::is_convertible<OtherT, T>::value> * = nullptr)
       : Expected<T>(std::move(Other)) {}
 
   template <class OtherT>

llvm/include/llvm/Support/TaskQueue.h

public:
   /// used to wait for the task (and all previous tasks that have not yet
   /// completed) to finish.
   template <typename Callable>
   std::future<std::invoke_result_t<Callable>> async(Callable &&C) {
 #if !LLVM_ENABLE_THREADS
     static_assert(false,
                   "TaskQueue requires building with LLVM_ENABLE_THREADS!");
 #endif
-    Task<Callable> T{std::move(C), *this};
+    Task<Callable> T{std::forward<Callable>(C), *this};
     using ResultTy = std::invoke_result_t<Callable>;
     std::future<ResultTy> F = T.P->get_future();
     {
       std::lock_guard<std::mutex> Lock(QueueLock);
       // If there's already a task in flight, just queue this one up.  If
       // there is not a task in flight, bypass the queue and schedule this
       // task immediately.
       if (IsTaskInFlight)