[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");
 
----------------
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);
  }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81356