[PATCH] D87987: [llvm-objcopy][NFC] refactor error handling. part 3.

Alexey Lapshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 1 06:15:55 PDT 2020


avl added a comment.

@grimar

> I've noticed that the code tries to address 2 a bit different situations currently:
>
>   Cases that fix the unwrapOrError: like unwrapOrError(HeadersFile.program_headers()) which seems are trying to fix places related to >broken inputs which were never tested before I think.
>   Cases that are tested and report errors, like: error("program header with offset 0x" + Twine::utohexstr(Phdr.p_offset) + .....
>   
>   I think for start we should probably focus on (B) and probably can keep unwrapOrError untouched until we convert them and add tests.

Probably, we could leave unwrapOrError expanded by this patch?
This patch does not change the logic existed with unwrapOrError.
It just removes the function and put the code inplace.
Before this patch:

  for (const auto &Phdr : unwrapOrError(HeadersFile.program_headers()))

after this patch:

  Expected<typename ELFFile<ELFT>::Elf_Phdr_Range> Headers =
      HeadersFile.program_headers();
  if (!Headers)
    return Headers.takeError();
  
  for (const typename ELFFile<ELFT>::Elf_Phdr &Phdr : *Headers)

The unwrapOrError was defined as :

  template <class T> T unwrapOrError(Expected<T> EO) {
    if (EO)
      return *EO;
    std::string Buf;
    raw_string_ostream OS(Buf);
    logAllUnhandledErrors(EO.takeError(), OS);
    OS.flush();
    error(Buf);
  }

i.e. it either continues execution, either report the error and die.
After unwrapOrError() is expanded the behavior is not changed: it either 
continues execution, either report Error which is handled in llvm-objcopy.cpp and stop.

WDYT, Would it be OK to remove unwrapOrError() by this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87987



More information about the llvm-commits mailing list