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

[Alexey Lapshin via Phabricator via llvm-commits]

avl added inline comments.


================
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);
----------------
Michael137 wrote:
> JDevlieghere wrote:
> > 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.  
> I interpreted it as being optional to include an imported declaration (since imported declaration != namespace, although for our purposes that is the case). That said, we haven't put a version check on the codegen side either, so probably not necessary here
this patch makes the behavior to be similar to the behavior of AsmPrinter(after D143397 AsmPrinter will put imported declaration into the apple_namespaces and/or debug_names), correct? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143458