[Lldb-commits] [PATCH] D76758: Augment lldb's symbol table with external symbols in Mach-O's dyld trie

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda marked 2 inline comments as done.
jasonmolenda added a comment.

Thanks Adrian, great comments, incorporated most of them.  The use of the block to search the trie entries when eliminating duplication between nlist/tries was a good idea, much cleaner now.



================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:4500
+              external_sym_trie_entries.end(), sort_by_address);
+    for (uint32_t i = 0; i < sym_idx; i++) {
+      TrieEntryWithOffset ent(0);
----------------
aprantl wrote:
> Does `for (auto &current_sym : sym)` work here?
This idiom is used everywhere else in ParseSymtab, just for consistency I want to stick with the older style.


================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:4566
+
+    Address symbol_addr;
+    if (module_sp->ResolveFileAddress(e.entry.address, symbol_addr)) {
----------------
aprantl wrote:
> High-level comment what this block is doing?
Added.  It was largely the same thing as the function starts symbol table entries - we compute which section the address is in, to set the type of the symbol in the symtab.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76758