[PATCH] D143458: [llvm][dsymutil] Add DW_TAG_imported_declaration to accelerator table

Wed Feb 8 19:39:00 PST 2023

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM

================
Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:1592
     Unit.addNamespaceAccelerator(Die, AttrInfo.Name);
+  } else if (Tag == dwarf::DW_TAG_imported_declaration && AttrInfo.Name) {
+    Unit.addNamespaceAccelerator(Die, AttrInfo.Name);
----------------
aprantl wrote:
> Now that DWARFLinker.cpp has been moved from tools/dsymutil into lib/DWARFLinker do we also have access to the debugger tuning settings?
> My understanding is that @avl is working on a DWARFLinker for ELF so if that flag is exposed here, it might be nice to make this conditional to match the AsmPrinter behavior. If it's not exposed, I think leaving it as is is probably okay.
> 
The DWARFLinker has options that allow it to be configured differently between users (like dsymutil). That said, looking at the DWARF spec:

> The name index must contain an entry for each debugging information entry that defines a named subprogram, label, variable, type, or namespace, subject to the following rules: [..]

Nothing in the later stated "rules" excludes `DW_TAG_imported_declaration` from being part of the index. So following the wording of the spec, we "must" include it.  

================
Comment at: llvm/test/tools/dsymutil/Inputs/accel-imported-declaration.cpp:1
+// Compiled on MacOS using:
+// clang++ -c -std=c++2a -gdwarf-4 -O0 -o accel-imported-declaration.macho-arm64.o
----------------

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143458/new/

https://reviews.llvm.org/D143458