[PATCH] D119050: [llvm-objdump/mac] Add new function starts print mode

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai accepted this revision.
smeenai added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/tools/llvm-objdump/MachODump.cpp:1107
+      if (It != SymbolNames.end())
+        outs() << It->second << "\n";
+    } else {
----------------
keith wrote:
> smeenai wrote:
> > So if we can't find the address associated with a symbol, we don't print anything for it? Does that match dyldinfo's behavior? Should we have a test for it?
> So turns out that dyldinfo prints a `?` beside the address in this case. I've mirrored that here and added a test, but in the case you only ask for names I stilled opted to print nothing since it doesn't seem like a ton of `?`'s w/o any other info would be very useful output, and if you have _some_ symbols it might distract you from those. Interested to hear thoughts on this since names only not a mode dyldinfo has.
Yeah, that sounds reasonable to me.


================
Comment at: llvm/tools/llvm-objdump/MachODump.h:37
 
+enum FunctionStartsMode { FSAddrs, FSNames, FSBoth, FSNone };
+
----------------
smeenai wrote:
> Nit: I think an `enum class` would be nicer here.
Sorry for being nitty again, but you can remove the `FS` prefix now that it's an `enum class` (which was the motivation for my suggestion).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119050