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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
