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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
