[PATCH] D88213: [llvm-objcopy][NFC] refactor error handling. part 2.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/COFF/COFFObjcopy.cpp:57
   support::endian::write32le(Data.data() + CRCPos, CRC32);
-  return Data;
+  return std::move(Data);
 }
----------------
avl wrote:
> grimar wrote:
> > You don't need this move I believe. Move constructor of `Expected<>` will be called anyways.
> It looks like in gcc case it does not work  - https://reviews.llvm.org/rG4da6927de47074f56531c2e7e2eecc4d6a1f09ec
Have you checked this place? The commit message does not say what was the BB error.
But in LLVM code we have many places that don't have `std::move` and feel fine.

https://github.com/llvm/llvm-project/blob/master/llvm/lib/Object/ArchiveWriter.cpp#L510
https://github.com/llvm/llvm-project/blob/master/llvm/tools/llvm-exegesis/llvm-exegesis.cpp#L287
https://github.com/llvm/llvm-project/blob/master/llvm/tools/llvm-readobj/ELFDumper.cpp#L582

Perhaps, the issue was related to `std::unique_ptr`.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88213