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

Mon Jun 8 01:35:29 PDT 2020

Higuoxing marked an inline comment as done.
Higuoxing added a comment.

In D81356#2078973 <https://reviews.llvm.org/D81356#2078973>, @jhenderson wrote:

> To silence all the clang-tidy warnings, I'd be happy if you want to do a bulk renaming of the functions (to `emitDebug*`) as a separate commit before this patch. Since you're going to modify all those lines in this patch anyway, it'll not make git blame any worse, which is usually the motivation behind not renaming things.

Sure, I will do it later.

> That being said, do you actually need to make all the functions return `Error`? Do you anticipate them all getting reporting some form of error? If you don't, making them return `Error` complicates the usage and adds some code paths (i.e. a function returning a failed Error) we can't test, because they are dead. Perhaps it would be better to only modify the signature of `EmitDebugRanges`, since that's where you need the error reported from.

Yes, I need to, but I'm not sure if I really need to. There's a helper function called `EmitDebugSectionImpl` that takes `EmitFuncType` as the parameter, where `EmitFuncType` can be `EmitDebugStrings`, `EmitDebugARanges` etc. The `EmitDebugXXX` functions have to have same return type (See my inline comment).

================
Comment at: llvm/lib/ObjectYAML/DWARFEmitter.cpp:333

-using EmitFuncType = void (*)(raw_ostream &, const DWARFYAML::Data &);
+using EmitFuncType = Error (*)(raw_ostream &, const DWARFYAML::Data &);

----------------
The `EmitDebugXXXX` functions have to have same type.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81356