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

[Alexey Lapshin via Phabricator via llvm-commits]

avl added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOReader.cpp:89
     if (!SecRef)
-      reportError(MachOObj.getFileName(), SecRef.takeError());
+      return createStringError(errc::invalid_argument,
+                               "'" + MachOObj.getFileName() +
----------------
alexshap wrote:
> 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 ?
My guess is that original intention is to add file name MachOObj.getFileName to have better reporting. If we would like to add filename then we could not bubble up original error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88113