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

[Alexey Lapshin via Phabricator via llvm-commits]

avl 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);
 }
----------------
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


================
Comment at: llvm/tools/llvm-objcopy/COFF/COFFObjcopy.cpp:282
+    return createFileError(Config.InputFilename, O.takeError());
+  Object *Obj = O->get();
   assert(Obj && "Unable to deserialize COFF object");
----------------
grimar wrote:
> avl wrote:
> > mstorsjo wrote:
> > > This bit seems like unrelated renamings?
> > During previous review it was asked to not to use such naming style for similar case - https://reviews.llvm.org/D88113#inline-818190 . So I changed it to match with that requirement.
> And `ObjOrErr` was probably a better name. It is common to name `Expected` variables as "<something>OrErr".
It was argued that such a naming match with  ErrorOr class. Since it is not the case here - it looks ambitions. https://reviews.llvm.org/D88113#inline-818190


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88213