Page MenuHomePhabricator

Unit test for Symtab::InitNameIndexes
ClosedPublic

Authored by sgraenitz on Jul 27 2018, 3:55 AM.

Details

Summary

In order to exploit the potential of LLVM's new ItaniumPartialDemangler for indexing in LLDB, we expect conceptual changes in the implementation of the InitNameIndexes function. Here is a unit test that aims at covering all relevant code paths in that function.

Diff Detail

Repository
rL LLVM

Event Timeline

sgraenitz created this revision.Jul 27 2018, 3:55 AM

I am wondering whether a test like this wouldn't be better of as a lit test instead of a unit test.

Right now, if you do a lldb-test symbols foo.o (and foo.o contains debug info), we will dump out a very similar index which is built from the information in the debug_info sections:

Manual DWARF index for (x86_64) 'foo.o':
Function basenames:
0x62a4e88: {0x00000000/0x0000005c} "__cxx_global_var_init"

Function fullnames:
0x6188e10: {0x00000000/0x0000012a} "_ZN1AC2Ez"
0x62a4e88: {0x00000000/0x0000005c} "__cxx_global_var_init"
...

I think it makes sense to extend this dump to include the index information from the Symtab class too. Then it should be possible to write this test the traditional LLVM way as yaml2obj | lldb-test | FileCheck.

What do you think?

Sounds very reasonable. I think unit tests are great for development, because turnaround times are fast and debugging easy. But I am happy to have a look on how to turn this into a lit test once that's done. Thanks for the remark.

JDevlieghere accepted this revision.Jul 31 2018, 3:24 AM

I agree with Pavel, but I also don't mind having this test in the meantime. LGTM if you add a FIXME with the direction we want to go.

This revision is now accepted and ready to land.Jul 31 2018, 3:24 AM

Nice. I created a ticket for me internally to turn this into a lit test.
For now I'd merge it once https://reviews.llvm.org/D50071 is done. (But I want to keep that one open until Jim had a look.)

Closed by commit rL338695: Unit test for Symtab::InitNameIndexes (authored by stefan.graenitz, committed by ). · Explain WhyAug 2 2018, 3:13 AM
This revision was automatically updated to reflect the committed changes.