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

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
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");
----------------
avl wrote:
> 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
While these argument seem contradictory, the main point in this argument here, is that to implement what this patch sets out to do, you don't need to touch these lines at all, they already do exactly what they need to.

If you want to rename unrelated variables for consistency, that's a separate change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88213