[PATCH] D151353: [DebugInfo] Add error-handling to DWARFAbbreviationDeclarationSet

Tue May 30 00:34:30 PDT 2023

jhenderson accepted this revision.
jhenderson added a comment.

LGMT. There are a couple of pre-merge tests that failed that have something to do with DWARF. It's unlikely to be related to this patch, but it might be worth double-checking before landing this.

================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugAbbrev.cpp:127
     DWARFAbbreviationDeclarationSet AbbrDecls;
-    if (!AbbrDecls.extract(*Data, &Offset))
+    if (auto Err = AbbrDecls.extract(*Data, &Offset)) {
+      // FIXME: We should propagate the error upwards.
----------------
bulbazord wrote:
> jhenderson wrote:
> > Is there a particular reason you're using `auto` rather than `Error` here and in the similar code below? It doesn't seem to improve readability to me.
> In general, I use `auto` when I feel I can convey the type information through other means (the expression or name of variable), but I hold my opinion on the use of `auto` very weakly. I'll switch it over to use `Error`.
It's worth noting that LLVM has policies on `auto` usage documented in the coding standards document. I think it largely boils down to "use `auto` for iterators and where the type can be inferred from the right-hand-side of the expression", although there are some nuances to it beyond that.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151353