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

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:862
+  } else
     llvm_unreachable("unexpected emitDWARF() call");
 
----------------
grimar wrote:
> Higuoxing wrote:
> > 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.
> > 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 following code will invoke and trigger the original `Err` to be checked:
> 
> ```
>   /// Move-construct an error value. The newly constructed error is considered
>   /// unchecked, even if the source error had been checked. The original error
>   /// becomes a checked Success value, regardless of its original state.
>   Error(Error &&Other) {
>     setChecked(true);
>     *this = std::move(Other);
>   }
> ```
> I think the following code will invoke and trigger the original Err to be checked:

Am I wrong?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81356