[PATCH] D84412: [clang][Driver] Don't hardcode --as-needed/--no-as-needed on Illumos

[Rainer Orth via Phabricator via cfe-commits]

ro marked 3 inline comments as done.
ro added a comment.

In D84412#2193600 <https://reviews.llvm.org/D84412#2193600>, @MaskRay wrote:

> #clang <https://reviews.llvm.org/tag/clang/> does not have many people. You might need to add people explicitly..

I always found finding reviewers sort of a black art: sometimes the people from the `CODE_OWNERS.TXT` files work, sometimes people that
recently reviewed changes to the same files are willing to do so again, while at others only repeated nagging works...



================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:633
 
+static const char *getAsNeededOption(const ToolChain &TC, bool ignore) {
+  // FIXME: Need to handle GNU ld on Solaris.
----------------
MaskRay wrote:
> `ignore` seems strange. How about `start`?
I'd used `ignore` based no the description of the option in the `ld` man page:
```
       -z ignore | record
       --as-needed | --no-as-needed

           Ignores, or records, shared object dependencies that are not refer-
           enced as part  of  the  link-edit.  These  options  are  positional
```
`start` doesn't tell me much either, so I've gone for `as_needed`.


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:634
+static const char *getAsNeededOption(const ToolChain &TC, bool ignore) {
+  // FIXME: Need to handle GNU ld on Solaris.
+  if (TC.getTriple().isOSSolaris())
----------------
MaskRay wrote:
> Can you improve the comment to mention that this is to work around illumnos ld?
I hope it's better now.  I've removed the GNU `ld` reference.
You can see my current hack to make `clang` work with `gld` on Solaris in D85309.


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:636
+  if (TC.getTriple().isOSSolaris())
+    return ignore ? "-zignore" : "-zrecord";
+  else
----------------
MaskRay wrote:
> I'll assume that `-zignore` has semantics similar to --as-needed.
Right: the two are identical semantically.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84412