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

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOReader.cpp:131
     case MachO::LC_SEGMENT:
-      LC.Sections = extractSections<MachO::section, MachO::segment_command>(
-          LoadCmd, MachOObj, NextSectionIndex);
+      if (Expected<std::vector<std::unique_ptr<Section>>> SectionsOrErr =
+              extractSections<MachO::section, MachO::segment_command>(
----------------
the same as above - the name SectionsOrErr appears to be slightly misleading, I'd probably just call it Sections.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOReader.cpp:89
     if (!SecRef)
-      reportError(MachOObj.getFileName(), SecRef.takeError());
+      return createStringError(errc::invalid_argument,
+                               "'" + MachOObj.getFileName() +
----------------
jhenderson wrote:
> avl wrote:
> > jhenderson wrote:
> > > Can you use the error code associated with `SecRef` instead of `errc::invalid_argument` here?
> > As far as I understand that could be done in that way:
> > 
> > 
> > ```
> > inline Error errorWithContext(StringRef Context, Error E) {
> >   assert(E);
> >   std::string Buf;
> >   raw_string_ostream OS(Buf);
> >   Optional<std::error_code> EC;
> >   handleAllErrors(std::move(E), [&](const ErrorInfoBase &EI) {
> >     OS << "'" + Context + "': ";
> >     if (!EC)
> >       EC = EI.convertToErrorCode();
> >     EI.log(OS);
> >     OS << "\n";
> >   });
> >   OS.flush();
> > 
> >   return createStringError(*EC, Buf);
> > }
> > ```
> > 
> > but you`ve asked me to not create a special routine for that.
> I misremembered how the interface worked. There's an `errorToErrorCode()` function, but that consumes the input `Error`, which we don't want. Leave it as-is.
I'm wondering - why not simply bubble up the error ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88113