[Lldb-commits] [PATCH] D103652: [lldb][NFC] Refactor name to index maps in Symtab

[Raphael Isemann via Phabricator via lldb-commits]

teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

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



================
Comment at: lldb/include/lldb/Symbol/Symtab.h:177
   FileRangeToIndexMap m_file_addr_to_index;
-  UniqueCStringMap<uint32_t> m_name_to_index;
-  UniqueCStringMap<uint32_t> m_basename_to_index;
-  UniqueCStringMap<uint32_t> m_method_to_index;
-  UniqueCStringMap<uint32_t> m_selector_to_index;
+  std::map<lldb::FunctionNameType, UniqueCStringMap<uint32_t>>
+      m_functype_to_map;
----------------
`/// Maps function names to symbol indices (grouped by FunctionNameTypes)`
and maybe let's call this variable:
`m_name_to_symbol_indizes`


================
Comment at: lldb/source/Symbol/Symtab.cpp:1031
   std::vector<uint32_t> symbol_indexes;
+  std::vector<UniqueCStringMap<uint32_t>> maps_to_search;
 
----------------
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.

```
lang=c++
for (lldb::FunctionNameType type : {lldb::eFunctionNameTypeBase, lldb::eFunctionNameTypeMethod, lldb::eFunctionNameTypeSelector}) {
  if (name_type_mask & type) {
    auto &map = GetFunctypeMap(type);
    [search map]
  }
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103652