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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits