[PATCH] D81356: [ObjectYAML] Add support for error handling in DWARFYAML. NFC.

Xing GUO via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 8 06:31:05 PDT 2020


Higuoxing added inline comments.


================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:862
+  } else
     llvm_unreachable("unexpected emitDWARF() call");
 
----------------
jhenderson wrote:
> Higuoxing wrote:
> > grimar wrote:
> > > My concern this starts to look a bit bulky. Perhaps something like the following would be better?
> > > 
> > > ```
> > > uint64_t BeginOffset = OS.tell();
> > > Error Err = Error::success();
> > > 
> > > if (Name == ".debug_str")
> > >   Err = DWARFYAML::EmitDebugStr(OS, DWARF);
> > > else if (Name == ".debug_aranges")
> > >   Err = DWARFYAML::EmitDebugAranges(OS, DWARF);
> > > ...
> > > 
> > > if (Err)
> > >   return std::move(Err);
> > > ```
> > Thanks a lot!
> Using `joinErrors` here and below doesn't make much sense, since there is only a single error to worry about. However, I believe you can't just assign to `Err` here either, as the original `Err` will be unchecked (even `Error::success()` needs to be checked before being overwritten/destroyed).
> 
> I think the thing to do is close to what @grimar suggested, but with one additional line:
> ```
> Error Err = Error::success();
> cantFail(Err); // Mark as checked before assigning.
> 
> if (Name == ".debug_str")
>   Err = DWARFYAML::EmitDebugStr(OS, DWARF);
> else if (Name == ".debug_aranges")
>   Err = DWARFYAML::EmitDebugAranges(OS, DWARF);
> ...
> 
> if (Err)
>   return std::move(Err);
> ```
Oh, thanks a lot. At first, I don't know how to make the `Error::success()` get checked. So I use `joinErrors()` everywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81356





More information about the llvm-commits mailing list