[PATCH] D54310: Make clang-based tools find libc++ on MacOS

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 9 09:27:00 PST 2018

ilya-biryukov added a comment.

In https://reviews.llvm.org/D54310#1292924, @sammccall wrote:

> If you know of a good reviewer for this, you may want a second opinion!

This would definitely be nice, but I also don't know who'd be the proper person to review this.
I'll take a pause until Monday, maybe @arphaman or someone else from the community can suggest a better way of fixing this or proper reviewers for this change.

Comment at: include/clang/Lex/HeaderSearchOptions.h:111
+  /// Compiler install dir as detected by the Driver.
+  /// Only used to add include dirs for libc++ on Darwin. Please avoid relying
sammccall wrote:
> Could you add "typically contains bin/clang, lib/libclang_rt, and include/<builtins>"?
This is actually the 'bin' subdirectory, I've added a comment about that but you see a better way to communicate this, please let me know! E.g. actually mentioning the parallel lib/ and include/ subdirs. 

Comment at: lib/Frontend/CompilerInvocation.cpp:1776
   Opts.ResourceDir = Args.getLastArgValue(OPT_resource_dir);
+  Opts.InstallDir = Opts.ResourceDir;
sammccall wrote:
> (is this needed? I guess you fall back to this if you don't create the CI from the command line?)
Thanks for noticing this!
This is a leftover from a previous iteration, it's actually incorrect now since we rely on `InstallDir` pointing to a different place from the resource dir.
Removed it.

  rC Clang


More information about the cfe-commits mailing list