[PATCH] D63280: [llvm-objdump] Use <first-symbol>-<offset> as the section start symbol

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:348
 
+static bool shouldKeepDisassemble(object::SectionRef S) {
+  if (!shouldKeep(S) || !S.getSize())
----------------
It's not clear from the function name what this function is supposed to be doing. Please at least add a comment, and also consider renaming the function. Perhaps `shouldKeepForDisassembly`?


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1126
+  // these symbols are needed for branch target resolving.
+  StringSet<> StartSymbols;
+  for (const SectionRef &Section : DisassembleSectionFilter(*Obj)) {
----------------
Perhaps worth calling this StartProxySymbols (or just ProxySymbols), since they aren't real symbols.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63280