[PATCH] D91884: clang+lld: Improve clang+ld.darwinnew.lld interaction, pass -demangle

[Hans Wennborg via Phabricator via llvm-commits]

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

lgtm, probably check with maskray that he's happy first though.



================
Comment at: lld/Common/Strings.cpp:29
   // result for a C symbol that happens to match a mangled type name.
-  if (!name.startswith("_Z"))
+  if (!name.startswith("_Z") && !name.startswith("__Z") &&
+      !name.startswith("___Z") && !name.startswith("____Z"))
----------------
thakis wrote:
> hans wrote:
> > Does the comment above need updating now that this is checking not just for "_Z", but also multiple leading underscores?
> > Actually I found the comment pretty confusing in its current state too.
> > The idea here is to only call the demangler for C++ symbols, right? Maybe the comment could just say that.
> Removed some bits, added some bits, added an example.
Much nicer, thanks!


================
Comment at: lld/MachO/Options.td:1109
      Group<grp_undocumented>;
 def demangle : Flag<["-"], "demangle">,
+     HelpText<"Demangle symbol names in diagnostics">;
----------------
thakis wrote:
> hans wrote:
> > All options seem to belong to a Group and also be ordered in the .td file according to group. Now that it's no longer in grp_undocumented, maybe it should move into another group?
> Options without explicit end up in "OPTIONS:" in `lld -flavor darwinnew --help`, which seems the correct group for this flag. Do you think it should be in a different group? A new group?
Are there any other options that are not in Group?
Perhaps grp_rare would fit? It seems like a bit of a catch-all, e.g. -v is there.

In any case, the .td file seems quite strictly organized around these groups, so if we take this out of grp_undocumented I think definition should at least move out of this part of the file -- either to a different group, or somewhere around the top/bottom if it doesn't get any group.


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

https://reviews.llvm.org/D91884