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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

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.

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.



================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:890-892
+        handleAllErrors(
+            std::move(ShSizeOrErr.takeError()),
+            [&](const ErrorInfoBase &Err) { reportError(Err.message()); });
----------------
Rather than just writing `handleAllErrors` here, I'd move this into a helper function alongside `reportError`, probably as a `reportError` overload that takes `Error`. That would be useful going forwards.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81356