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

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:1990
         import_name[0]) {
+      // add symbols that are reexport symbols with a valid import name
+      add_this_entry = true;
----------------
aprantl wrote:
> Super nit-picky nit pick:
> 
> `// Add symbols that are reexport symbols with a valid import name.`
Mega nit-picky nit pick: comment should be sentences and start with a capital.



================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:1926
+  bool IsSymbolAlreadyPresent() {
+    if (flags == LLDB_INVALID_ADDRESS)
+      return true;
----------------
`return flags == LLDB_INVALID_ADDRESS`


================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:4508
+        return it;
+      else
+        return entries.end();
----------------
You don't need the else after a return. 


================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:4569
+      }
+      if (symbol_is_thumb && func_start_entry->addr == (e.entry.address & 1)) {
+        func_start_entry->data = true;
----------------
I guess this could be an `else if`


================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:4598
+        uint32_t symbol_flags = 0;
+        if (symbol_is_thumb)
+          symbol_flags = MACHO_NLIST_ARM_SYMBOL_IS_THUMB;
----------------
This could make a great ternary operator ;-) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76758