bulbazord added a comment. I think breaking it out of the Clang-specific class makes sense if we want LLDB to be more language-agnostic. Do you have an idea of what bits of `DWARFASTParserClang` can be moved out other than `ParseChildArrayInfo` and `GetAccessTypeFromDWARF` (from the patch on top of this)? What is your end-goal with this decoupling? I assume you want to work towards supporting languages non-clang-based languages but I'm curious about the motivation.
================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp:1 +#include "DWARFASTParser.h" +#include "DWARFAttribute.h" ---------------- This file will need a license header. ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp:77 + default: + case DW_AT_abstract_origin: + case DW_AT_accessibility: ---------------- 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. 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
