This is an archive of the discontinued LLVM Phabricator instance.

[DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use
ClosedPublic

Authored by labath on Jun 1 2018, 6:16 AM.

Details

Summary

This patch is pretty much a big noop. It adds the ability to create a
DebugNamesDWARFIndex class, but the class itself is not implemented in
any useful way. The I am putting it up this way is because of the
setting to control whether it gets used. My idea for it was the
following:

  • while the feature is under development (which hopefully won't take much longer), it will default to off. Tests will override it to true.
  • once the feature is complete and we are reasonably certain it works, we flip the switch to "on" while keeping the ability to turn it off for troubleshooting purposes.
  • after it has been on for a release or two and it hasn't blown up into anyone's face, we remove the setting altogether.

Diff Detail

Repository
rL LLVM

Event Timeline

labath created this revision.Jun 1 2018, 6:16 AM
JDevlieghere accepted this revision.Jun 4 2018, 1:15 PM
This revision is now accepted and ready to land.Jun 4 2018, 1:15 PM
clayborg added inline comments.Jun 4 2018, 4:10 PM
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
122 ↗(On Diff #149443)

Would it be better to use an inverse setting like "always-manually-index"? That way if there is something wrong with any of the indexes that are generated, then we can fall back to manually indexing? Otherwise we will need to remove this later.

labath added inline comments.Jun 5 2018, 2:33 AM
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
122 ↗(On Diff #149443)

In the interests of non-proliferation of settings I have deliberately chosen a path that does not make the setting permanent. However, this does sound like a thing that could conceivably be useful as a setting in general, so I can do that too if you think it's better.

Would you then also make the setting non-experimental?

labath updated this revision to Diff 149964.Jun 5 2018, 5:52 AM

This makes the setting non-experimental and renames it to "ignore-file-indexes".
I've also wired up the index dump methods to Module::Dump so that I could write
a test that apple indexes are indeed being used after this.

clayborg accepted this revision.Jun 5 2018, 9:38 AM

I like this new setting much better. Looks good.

This revision was automatically updated to reflect the committed changes.