<div dir="ltr">That looks like it should help. Thanks for the quick fix!</div><br><div class="gmail_quote"><div dir="ltr">On Tue, Jan 8, 2019 at 1:11 PM Ilya Biryukov <<a href="mailto:ibiryukov@google.com">ibiryukov@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr">Hi Nico,<div><br></div><div>This is clearly a bug, it's supposed to search in a sibling directory.</div><div>Are you running clang as './clang' in the scripts? The code seems to break in that case, <a href="https://reviews.llvm.org/D56446" target="_blank">https://reviews.llvm.org/D56446</a> should fix this.</div></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Jan 8, 2019 at 5:12 PM Nico Weber <<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">It looks like clang now looks for libc++ headers in -internal-isystem Release+Asserts/bin/include/c++/v1 , compared to -internal-isystem Release+Asserts/include/c++/v1. `make install` puts the libc++ headers in Release+Asserts/include, the old location. Was this an intentional change?</div><div dir="ltr"><br></div><div>As-is, this seems to break chromium's clang ability to find libc++ headers (<a href="https://crbug.com/919761" target="_blank">https://crbug.com/919761</a>) because we bundle libc++ headers in an "include" directory that's a sibling to the "bin" directory (and have been doing so for 4.5 years, since <a href="https://codereview.chromium.org/281753002" target="_blank">https://codereview.chromium.org/281753002</a>).</div></div></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov 12, 2018 at 8:58 AM Ilya Biryukov via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: ibiryukov<br>
Date: Mon Nov 12 05:55:55 2018<br>
New Revision: 346652<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=346652&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=346652&view=rev</a><br>
Log:<br>
Make clang-based tools find libc++ on MacOS<br>
<br>
Summary:<br>
When they read compiler args from compile_commands.json.<br>
This change allows to run clang-based tools, like clang-tidy or clangd,<br>
built from head using the compile_commands.json file produced for XCode<br>
toolchains.<br>
<br>
On MacOS clang can find the C++ standard library relative to the<br>
compiler installation dir.<br>
<br>
The logic to do this was based on resource dir as an approximation of<br>
where the compiler is installed. This broke the tools that read<br>
'compile_commands.json' and don't ship with the compiler, as they<br>
typically change resource dir.<br>
<br>
To workaround this, we now use compiler install dir detected by the driver<br>
to better mimic the behavior of the original compiler when replaying the<br>
compilations using other tools.<br>
<br>
Reviewers: sammccall, arphaman, EricWF<br>
<br>
Reviewed By: sammccall<br>
<br>
Subscribers: ioeric, christof, kadircet, cfe-commits<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D54310" rel="noreferrer" target="_blank">https://reviews.llvm.org/D54310</a><br>
<br>
Added:<br>
cfe/trunk/test/Tooling/Inputs/mock-libcxx/<br>
cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/<br>
cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/<br>
cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/v1/<br>
cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector<br>
cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp<br>
cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp<br>
Modified:<br>
cfe/trunk/include/clang/Lex/HeaderSearchOptions.h<br>
cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp<br>
cfe/trunk/lib/Frontend/InitHeaderSearch.cpp<br>
cfe/trunk/lib/Tooling/Tooling.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Lex/HeaderSearchOptions.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearchOptions.h?rev=346652&r1=346651&r2=346652&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearchOptions.h?rev=346652&r1=346651&r2=346652&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Lex/HeaderSearchOptions.h (original)<br>
+++ cfe/trunk/include/clang/Lex/HeaderSearchOptions.h Mon Nov 12 05:55:55 2018<br>
@@ -108,6 +108,13 @@ public:<br>
/// etc.).<br>
std::string ResourceDir;<br>
<br>
+ /// Compiler install dir as detected by the Driver.<br>
+ /// This is typically the directory that contains the clang executable, i.e.<br>
+ /// the 'bin/' subdir of a clang distribution.<br>
+ /// Only used to add include dirs for libc++ on Darwin. Please avoid relying<br>
+ /// on this field for other purposes.<br>
+ std::string InstallDir;<br>
+<br>
/// The directory used for the module cache.<br>
std::string ModuleCachePath;<br>
<br>
<br>
Modified: cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp?rev=346652&r1=346651&r2=346652&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp?rev=346652&r1=346651&r2=346652&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp Mon Nov 12 05:55:55 2018<br>
@@ -11,17 +11,18 @@<br>
//<br>
//===----------------------------------------------------------------------===//<br>
<br>
-#include "clang/Frontend/Utils.h"<br>
#include "clang/Basic/DiagnosticOptions.h"<br>
+#include "clang/Driver/Action.h"<br>
#include "clang/Driver/Compilation.h"<br>
#include "clang/Driver/Driver.h"<br>
-#include "clang/Driver/Action.h"<br>
#include "clang/Driver/Options.h"<br>
#include "clang/Driver/Tool.h"<br>
#include "clang/Frontend/CompilerInstance.h"<br>
#include "clang/Frontend/FrontendDiagnostic.h"<br>
+#include "clang/Frontend/Utils.h"<br>
#include "llvm/Option/ArgList.h"<br>
#include "llvm/Support/Host.h"<br>
+#include "llvm/Support/Path.h"<br>
using namespace clang;<br>
using namespace llvm::opt;<br>
<br>
@@ -102,5 +103,8 @@ std::unique_ptr<CompilerInvocation> clan<br>
CCArgs.size(),<br>
*Diags))<br>
return nullptr;<br>
+ // Patch up the install dir, so we find the same standard library as the<br>
+ // original compiler on MacOS.<br>
+ CI->getHeaderSearchOpts().InstallDir = TheDriver.getInstalledDir();<br>
return CI;<br>
}<br>
<br>
Modified: cfe/trunk/lib/Frontend/InitHeaderSearch.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitHeaderSearch.cpp?rev=346652&r1=346651&r2=346652&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitHeaderSearch.cpp?rev=346652&r1=346651&r2=346652&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Frontend/InitHeaderSearch.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/InitHeaderSearch.cpp Mon Nov 12 05:55:55 2018<br>
@@ -476,14 +476,9 @@ void InitHeaderSearch::AddDefaultInclude<br>
if (triple.isOSDarwin()) {<br>
// On Darwin, libc++ may be installed alongside the compiler in<br>
// include/c++/v1.<br>
- if (!HSOpts.ResourceDir.empty()) {<br>
- // Remove version from foo/lib/clang/version<br>
- StringRef NoVer = llvm::sys::path::parent_path(HSOpts.ResourceDir);<br>
- // Remove clang from foo/lib/clang<br>
- StringRef Lib = llvm::sys::path::parent_path(NoVer);<br>
- // Remove lib from foo/lib<br>
- SmallString<128> P = llvm::sys::path::parent_path(Lib);<br>
-<br>
+ if (!HSOpts.InstallDir.empty()) {<br>
+ // Get from foo/bin to foo.<br>
+ SmallString<128> P(llvm::sys::path::parent_path(HSOpts.InstallDir));<br>
// Get foo/include/c++/v1<br>
llvm::sys::path::append(P, "include", "c++", "v1");<br>
AddUnmappedPath(P, CXXSystem, false);<br>
<br>
Modified: cfe/trunk/lib/Tooling/Tooling.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?rev=346652&r1=346651&r2=346652&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?rev=346652&r1=346651&r2=346652&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Tooling/Tooling.cpp (original)<br>
+++ cfe/trunk/lib/Tooling/Tooling.cpp Mon Nov 12 05:55:55 2018<br>
@@ -327,6 +327,9 @@ bool ToolInvocation::run() {<br>
Invocation->getPreprocessorOpts().addRemappedFile(It.getKey(),<br>
Input.release());<br>
}<br>
+ // Patch up the install dir, so we find the same standard library as the<br>
+ // original compiler on MacOS.<br>
+ Invocation->getHeaderSearchOpts().InstallDir = Driver->getInstalledDir();<br>
return runInvocation(BinaryName, Compilation.get(), std::move(Invocation),<br>
std::move(PCHContainerOps));<br>
}<br>
<br>
Added: cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c%2B%2B/v1/mock_vector?rev=346652&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c%2B%2B/v1/mock_vector?rev=346652&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector (added)<br>
+++ cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector Mon Nov 12 05:55:55 2018<br>
@@ -0,0 +1 @@<br>
+class vector {};<br>
<br>
Added: cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp?rev=346652&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp?rev=346652&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp (added)<br>
+++ cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp Mon Nov 12 05:55:55 2018<br>
@@ -0,0 +1,17 @@<br>
+// Clang on MacOS can find libc++ living beside the installed compiler.<br>
+// This test makes sure our libTooling-based tools emulate this properly.<br>
+//<br>
+// RUN: rm -rf %t<br>
+// RUN: mkdir %t<br>
+//<br>
+// Install the mock libc++ (simulates the libc++ directory structure).<br>
+// RUN: cp -r %S/Inputs/mock-libcxx %t/<br>
+//<br>
+// Pretend clang is installed beside the mock library that we provided.<br>
+// RUN: echo '[{"directory":"%t","command":"%t/mock-libcxx/bin/clang++ -stdlib=libc++ -target x86_64-apple-darwin -c test.cpp","file":"test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json<br>
+// RUN: cp "%s" "%t/test.cpp"<br>
+// clang-check will produce an error code if the mock library is not found.<br>
+// RUN: clang-check -p "%t" "%t/test.cpp"<br>
+<br>
+#include <mock_vector><br>
+vector v;<br>
<br>
Added: cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp?rev=346652&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp?rev=346652&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp (added)<br>
+++ cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp Mon Nov 12 05:55:55 2018<br>
@@ -0,0 +1,17 @@<br>
+// Clang on MacOS can find libc++ living beside the installed compiler.<br>
+// This test makes sure our libTooling-based tools emulate this properly.<br>
+//<br>
+// RUN: rm -rf %t<br>
+// RUN: mkdir %t<br>
+//<br>
+// Install the mock libc++ (simulates the libc++ directory structure).<br>
+// RUN: cp -r %S/Inputs/mock-libcxx %t/<br>
+//<br>
+// Pretend clang is installed beside the mock library that we provided.<br>
+// RUN: echo '[{"directory":"%t","command":"mock-libcxx/bin/clang++ -stdlib=libc++ -target x86_64-apple-darwin -c test.cpp","file":"test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json<br>
+// RUN: cp "%s" "%t/test.cpp"<br>
+// clang-check will produce an error code if the mock library is not found.<br>
+// RUN: clang-check -p "%t" "%t/test.cpp"<br>
+<br>
+#include <mock_vector><br>
+vector v;<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail-m_-7391980193987889540gmail-m_-6573800847359305280gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Regards,</div><div>Ilya Biryukov</div></div></div></div></div>
</blockquote></div>