[PATCH] D88113: [llvm-objcopy][NFC] refactor error handling. part 1.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp:361
+    return ObjOrErr.takeError();
+  assert(*ObjOrErr && "unable to deserialize MachO object");
+
----------------
I think you don't need the asser. A lot of our code base don't bother asserting that something is not null, because it's not really any different from a crash in that situation.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOReader.cpp:89
     if (!SecRef)
-      reportError(MachOObj.getFileName(), SecRef.takeError());
+      return createStringError(errc::invalid_argument,
+                               "'" + MachOObj.getFileName() +
----------------
Can you use the error code associated with `SecRef` instead of `errc::invalid_argument` here?


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOReader.cpp:131-137
+      Expected<std::vector<std::unique_ptr<Section>>> SectionsOrErr =
+          extractSections<MachO::section, MachO::segment_command>(
+              LoadCmd, MachOObj, NextSectionIndex);
+      if (!SectionsOrErr)
+        return SectionsOrErr.takeError();
+
+      LC.Sections = std::move(*SectionsOrErr);
----------------
Here and below in these `case` statements, due to the scoping issue, I'm okay if you want to go with your previous version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88113