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

Thu May 25 13:30:33 PDT 2023

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)) &&
----------------
arphaman wrote:
> ldionne wrote:
> > 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?
> That makes sense to me. I'm not sure why specifically do we need to check if the dylib exists when determining the header search path. Is there a specific reason we couldn't mix the local headers and the system's dylib in that case?
> 
We definitely don't want to mix the local headers and the system dylib. They can be configured differently (e.g. different ABI flags amongst other things). However it shouldn't really matter since when we build libc++, we build both the dylib and the headers so either both should be picked up or neither should be. I wouldn't bother checking. If we really wanted to check, we could actually error-out if one (but not both) are there, since that would point out to a mismatch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148266