[PATCH] D54630: Move detection of libc++ include dirs to Driver on MacOS

Alex Lorenz via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 26 13:07:06 PST 2018


arphaman added a comment.

In D54630#1302566 <https://reviews.llvm.org/D54630#1302566>, @ilya-biryukov wrote:

> In D54630#1301283 <https://reviews.llvm.org/D54630#1301283>, @arphaman wrote:
>
> > I don't think we want to move the logic to add a libc++ path to the driver. `-cc1` with `-resource-dir` and `-stdlib=libc++` should still work as before. In this case the previous patch is better, except it should not set `InstalledDir` except when needed (e.g. for tooling when working with a CDB that has an absolute path to the compiler), so when `InstalledDir` is empty it should fallback to the current logic in `InitHeaderSearch.cpp`. That should solve the issues we had.
>
>
> Why not?
>  @ldionne made a good point: there is value in keeping the frontend separate from the logic to detect system libraries and other toolchain-specific things.
>
> This is how other toolchains do it (Linux, etc.). It's more straightforward to the user (one can inspect the -cc1 args produced by the driver), this is where the logic to detect linker dependencies is. It seems that was also the plan of some developers looking in that area: the test I've edited suggests this should be moved to the driver (`darwin-stdlib.cpp`, it says: If and **when** we change to finding them in the driver ...), there's a bug corresponding bug to do this: http://llvm.org/PR30548. 
>  I might not have full information, but my understanding is that `-resource-dir` has a pretty well-defined semantics: it should be used to find the `compiler resources`, i.e. only the buit-in headers and not the C++ standard library.
>
> What are the reasons we need to keep the old behavior?


Sounds convincing.
@dexonsmith What do you think?

@ilya-biryukov I'm going to do some internal testing to see if we uncover any issues.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54630





More information about the cfe-commits mailing list