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

Mon Nov 29 14:51:12 PST 2021

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