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

[Hans Wennborg via Phabricator via llvm-commits]

hans added inline comments.


================
Comment at: clang/include/clang/Driver/ToolChain.h:331
+  /// If LinkerIsLLD is non-nullptr, it is set to true if the returned linker
+  /// is LLD. If it's set, it can be assumed that the linker iis LLD built
+  /// at the same revision as clang, and clang can make assumptions about
----------------
nit: iis


================
Comment at: clang/lib/Driver/ToolChain.cpp:606
+        if (UseLinker == "lld" || UseLinker == "lld.darwinnew")
+          *LinkerIsLLD = true;
       return LinkerPath;
----------------
It seems nicer for the caller if *LinkerIsLLD is always set to either true or false, so the caller doesn't have to initialize it.

For example:

```
if (LinkerIsLLD)
  *LinkerIsLLD = (UseLinker == "lld" || UseLinker == "lld.darwinnew");
```


================
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:696
 
-  const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath());
   std::unique_ptr<Command> Cmd = std::make_unique<Command>(
----------------
Why is this removed?


================
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"))
----------------
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.


================
Comment at: lld/MachO/Options.td:1109
      Group<grp_undocumented>;
 def demangle : Flag<["-"], "demangle">,
+     HelpText<"Demangle symbol names in diagnostics">;
----------------
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?


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

https://reviews.llvm.org/D91884