[PATCH] D148266: [clang][driver] Linking to just-built libc++.dylib when bootstrapping libc++ with clang

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a subscriber: arphaman.
ldionne added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:431
+  // libc++.dylib in the toolchain.
+  if ((!Args.hasArg(options::OPT_nostdinc, options::OPT_nostdlibinc,
+                    options::OPT_nostdincxx)) &&
----------------
The more I look at this, the more I think it seems kind of weird to me that a linker argument would be impacted by `-nostdinc` & friends, which control header search paths. IMO we should use the simplest behavior and only check for `libc++.dylib` here, and not check for `libc++.dylib` when we determine the header search paths.

@arphaman Do you have an opinion?


================
Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:432
   if (sysroot != "") {
     CmdArgs.push_back("-syslibroot");
     CmdArgs.push_back(C.getArgs().MakeArgString(sysroot));
----------------
ldionne wrote:
> Where do we set the usual `<sysroot>/usr/lib` search path? I think it might make sense to move the code you added to that place.
What I mean: there's a place where we must be adding `-L <sysroot>/usr/lib` in the code, and it isn't in `darwin::Linker::AddLinkArgs`. Where is it? Would it make sense to move the code you added to that place instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148266