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

[Luís Ferreira via Phabricator via lldb-commits]

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