This is an archive of the discontinued LLVM Phabricator instance.

[lldb][NFC] Refactor name to index maps in Symtab
ClosedPublic

Authored by bulbazord on Jun 3 2021, 3:11 PM.

Details

Summary

The various maps in Symtab lead to some repetative code. This should
improve the situation somewhat.

Diff Detail

Event Timeline

bulbazord created this revision.Jun 3 2021, 3:11 PM
bulbazord requested review of this revision.Jun 3 2021, 3:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 3 2021, 3:11 PM
teemperor requested changes to this revision.Jun 4 2021, 3:06 AM

Only have some comments about the way FindFunctionSymbols is now implemented, but otherwise this LGTM.

lldb/include/lldb/Symbol/Symtab.h
177–180

/// Maps function names to symbol indices (grouped by FunctionNameTypes)
and maybe let's call this variable:
m_name_to_symbol_indizes

lldb/source/Symbol/Symtab.cpp
1038

Those large maps seem rather expensive to copy around. Could you just make this a for loop over the few types we care about? Then we can delete all the duplicated code.

for (lldb::FunctionNameType type : {lldb::eFunctionNameTypeBase, lldb::eFunctionNameTypeMethod, lldb::eFunctionNameTypeSelector}) {
  if (name_type_mask & type) {
    auto &map = GetFunctypeMap(type);
    [search map]
  }
}
This revision now requires changes to proceed.Jun 4 2021, 3:06 AM
bulbazord updated this revision to Diff 350404.Jun 7 2021, 1:33 PM

Address feedback

bulbazord updated this revision to Diff 350405.Jun 7 2021, 1:35 PM

Remove unused line of code that I forgot to remove previously

teemperor accepted this revision.Jun 7 2021, 2:43 PM

Some small rename I forgot to point out, but LGTM modulo that rename. Thanks for cleaning this up!

lldb/include/lldb/Symbol/Symtab.h
186

GetNameToSymbolIndexMap might be more accurate now (I should have pointed out when changing the variable name).

This revision is now accepted and ready to land.Jun 7 2021, 2:43 PM
bulbazord updated this revision to Diff 350685.Jun 8 2021, 12:23 PM

Rename function :)

This revision was landed with ongoing or failed builds.Jun 8 2021, 12:38 PM
This revision was automatically updated to reflect the committed changes.