ljmf00 added inline comments.
================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp:1 +#include "DWARFASTParser.h" +#include "DWARFAttribute.h" ---------------- bulbazord wrote: > This file will need a license header. Yes, my bad. Thanks for noticing it. ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp:77 + default: + case DW_AT_abstract_origin: + case DW_AT_accessibility: ---------------- 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. 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
