[Lldb-commits] [PATCH] D114668: [lldb][NFC] Move generic DWARFASTParser code out of Clang-specific code

Tue Nov 30 08:07:02 PST 2021

shafik added a subscriber: clayborg.
shafik added inline comments.

================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp:77
+          default:
+          case DW_AT_abstract_origin:
+          case DW_AT_accessibility:
----------------
ljmf00 wrote:
> bulbazord wrote:
> > ljmf00 wrote:
> > > Why we are including just these specific attributes? Maybe we should add a comment explaining it. According to the DWARF standard, any attribute is eligible for any tag.
> > I'm not sure why. Possibly they were added to make sure the switch was fully covered (potentially to silence a warning)? You could add a `FIXME` or a `TODO` if you feel that these attributes should have functionality associated with them like the ones above.
> I don't think it is to mark it as fully covered since there are much more attributes, the default label will address it anyway, and according to the DWARF standard, any attribute can be in a type tag (realistically, any tag). We can take the example of `DW_AT_description` which is just a description associated with the symbol. I feel like this can be safely deleted but I'm afraid to do it in favour of some other rationale I'm not seeing.
@clayborg it looks like this has been this way since you put this in: https://github.com/llvm/llvm-project/commit/261ac3f4b5b98d02dd8718078015a92cf07df736

Do you agree this is dead code or is there something we are missing?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114668