[PATCH] D112753: [llvm] [Support] Add CURL HTTP Client.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/Support/HTTPClient.cpp:112
+  CurlHTTPRequest(HTTPResponseHandler &Handler) : Handler(Handler) {}
+  void storeError(Error &Err) {
+    ErrorState = joinErrors(std::move(Err), std::move(ErrorState));
----------------
Generally this function should accept Error by value (or rvalue reference) and callers should pass with std::move.


================
Comment at: llvm/lib/Support/HTTPClient.cpp:179-181
+  if (Error Err = Handler.handleStatusCode(Code))
+    return Err;
+  return std::move(CurlRequest.ErrorState);
----------------
I /think/ the `ErrorState` should be tested/returned first, otherwise if the `Code` codepath is hit (or the `CurlRes` codepath above) will hit an assertion due to the `ErrorState` being unchecked.


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

https://reviews.llvm.org/D112753