ljmf00 added inline comments.
================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp:1 +#include "DWARFASTParser.h" +#include "DWARFAttribute.h" ---------------- ljmf00 wrote: > bulbazord wrote: > > This file will need a license header. > Yes, my bad. Thanks for noticing it. Added. ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp:77 + default: + case DW_AT_abstract_origin: + case DW_AT_accessibility: ---------------- clayborg wrote: > shafik wrote: > > 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? > Fine to remove these extra attributes that are in the default case as we > definitely don't have all of the attributes listed here. Done. 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
