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

[Alexey Lapshin via Phabricator via llvm-commits]

avl accepted this revision.
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:
> avl wrote:
> > 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? 
> Exactly, I would only commit it as is once D143397 is approved
If the behavior matched then it is probably not necessary to make it conditional(until someone need it directly).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143458