[Orc] Fix copy elision warning in RPCUtils

[sgraenitz]

Summary

The callB() template function always moved errors on return, because in the majority of cases its return type is an Expected<T> and the error must be moved into the implicit ctor.
For the special case of a void result, however, the ResultTraits class is specialized and the return type is a raw Error. Some build bots complain, that in favor of NRVO errors should not be moved in this case.

llvm/include/llvm/ExecutionEngine/Orc/Shared/RPCUtils.h:1513:27:
llvm/include/llvm/ExecutionEngine/Orc/Shared/RPCUtils.h:1519:27:
llvm/include/llvm/ExecutionEngine/Orc/Shared/RPCUtils.h:1526:29:
  warning: moving a local object in a return statement prevents copy elision [-Wpessimizing-move]

The warning is reasonable from a type-system point of view. For performance it's entirely insignificant.

[sgraenitz]

I actually didn't manage to reproduce the warning with GCC 10 on Debian, so I couldn't really test this. @thakis Do you want to run a pre-test? Otherwise, we could land it right away and see if it is effective.

@lhames Correct me if I am wrong, but I think consumeAbandoned() should have been called on detail::ResultTraits<AltRetT> and not on detail::ResultTraits<typename Func::ReturnType> in the first place. I simplified it now with RTraits analog to appendCallNB(). Hope it's fine.

[thakis]

Landing and checking is easier. I saw the warnings on some bot; it's easier to check bot logs after this landed :)

[sgraenitz]

Make returnError() static (oops). Rename RTraits -> AltRetTraits. Turn some autos into concrete typenames to aid readability.

[lhames]

LGTM. Thanks very much for fixing this Stefan!

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

#endif
   static ErrorReturnType createBlankErrorReturnValue() {
     return ErrorReturnType(RetT());
   }
 
   // Consume an abandoned ErrorReturnType.
   static void consumeAbandoned(ErrorReturnType RetOrErr) {
     consumeError(RetOrErr.takeError());
   }
+
+  static ErrorReturnType returnError(Error Err) { return std::move(Err); }
 };
 
 // ResultTraits specialization for void functions.
 template <> class ResultTraits<void> {
 public:
   // For void functions, ErrorReturnType is llvm::Error.
   using ErrorReturnType = Error;
 
#endif
   static ErrorReturnType createBlankErrorReturnValue() {
     return ErrorReturnType::success();
   }
 
   // Consume an abandoned ErrorReturnType.
   static void consumeAbandoned(ErrorReturnType Err) {
     consumeError(std::move(Err));
   }
+
+  static ErrorReturnType returnError(Error Err) { return Err; }
 };
 
 // ResultTraits<Error> is equivalent to ResultTraits<void>. This allows
 // handlers for void RPC functions to return either void (in which case they
 // implicitly succeed) or Error (in which case their error return is
 // propagated). See usage in HandlerTraits::runHandlerHelper.
 template <> class ResultTraits<Error> : public ResultTraits<void> {};
 
addAsyncHandler<Func>(
         detail::MemberFnWrapper<ClassT, RetT, ArgTs...>(Object, Method));
   }
 
   template <typename Func, typename... ArgTs,
             typename AltRetT = typename Func::ReturnType>
   typename detail::ResultTraits<AltRetT>::ErrorReturnType
   callB(const ArgTs &...Args) {
     bool ReceivedResponse = false;
-    using ResultType = typename detail::ResultTraits<AltRetT>::ErrorReturnType;
-    auto Result = detail::ResultTraits<AltRetT>::createBlankErrorReturnValue();
+    using AltRetTraits = detail::ResultTraits<AltRetT>;
+    using ResultType = typename AltRetTraits::ErrorReturnType;
+    ResultType Result = AltRetTraits::createBlankErrorReturnValue();
 
     // We have to 'Check' result (which we know is in a success state at this
     // point) so that it can be overwritten in the async handler.
     (void)!!Result;
 
-    if (auto Err = this->template appendCallAsync<Func>(
+    if (Error Err = this->template appendCallAsync<Func>(
             [&](ResultType R) {
               Result = std::move(R);
               ReceivedResponse = true;
               return Error::success();
             },
             Args...)) {
-      detail::ResultTraits<typename Func::ReturnType>::consumeAbandoned(
-          std::move(Result));
-      return std::move(Err);
+      AltRetTraits::consumeAbandoned(std::move(Result));
+      return AltRetTraits::returnError(std::move(Err));
     }
 
-    if (auto Err = this->C.send()) {
-      detail::ResultTraits<typename Func::ReturnType>::consumeAbandoned(
-          std::move(Result));
-      return std::move(Err);
+    if (Error Err = this->C.send()) {
+      AltRetTraits::consumeAbandoned(std::move(Result));
+      return AltRetTraits::returnError(std::move(Err));
     }
 
     while (!ReceivedResponse) {
-      if (auto Err = this->handleOne()) {
-        detail::ResultTraits<typename Func::ReturnType>::consumeAbandoned(
-            std::move(Result));
-        return std::move(Err);
+      if (Error Err = this->handleOne()) {
+        AltRetTraits::consumeAbandoned(std::move(Result));
+        return AltRetTraits::returnError(std::move(Err));
       }
     }
 
     return Result;
   }
 };
 
 /// Asynchronous dispatch for a function on an RPC endpoint.