r346652 - Make clang-based tools find libc++ on MacOS
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 29 02:43:33 PST 2019
This change got reverted, but the follow-up r348365 might be causing the
issues.
I wonder why the Driver can't detect install dir on its own, will take a
look, thanks.
On Mon, Jan 28, 2019 at 7:20 PM Nico Weber <thakis at chromium.org> wrote:
> This seems to fix it, but I don't know if it's the right fix:
>
> diff --git a/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
> b/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
> index b62416ffd9e..d933f8c1b70 100644
> --- a/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
> +++ b/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
> @@ -47,6 +47,10 @@ std::unique_ptr<CompilerInvocation>
> clang::createInvocationFromCommandLine(
> driver::Driver TheDriver(Args[0], llvm::sys::getDefaultTargetTriple(),
> *Diags, VFS);
>
> + StringRef InstalledPathParent(llvm::sys::path::parent_path(Args[0]));
> + if (llvm::sys::fs::exists(InstalledPathParent))
> + TheDriver.setInstalledDir(InstalledPathParent);
> +
> // Don't check that inputs exist, they may have been remapped.
> TheDriver.setCheckInputsExist(false);
>
>
>
> On Mon, Jan 28, 2019 at 10:08 AM Nico Weber <thakis at chromium.org> wrote:
>
>> I think c-index-test has the same problem:
>>
>> $ cat test.cc
>> #include <iostream>
>> $ out/gn/bin/c-index-test -write-pch foo.pch test.cc
>> test.cc:1:10: fatal error: 'iostream' file not found
>>
>> Since https://reviews.llvm.org/rL350714 was a driver-side fix and
>> c-index-test doesn't use lib/Driver, it didn't do anything here. Can you
>> take a look at c-index-test too?
>>
>> On Wed, Jan 9, 2019 at 8:15 AM Ilya Biryukov <ibiryukov at google.com>
>> wrote:
>>
>>> Glad to help. The fix has landed. Let me know if the problem persists
>>> after it's integrated.
>>>
>>> On Tue, Jan 8, 2019 at 7:36 PM Nico Weber <thakis at chromium.org> wrote:
>>>
>>>> That looks like it should help. Thanks for the quick fix!
>>>>
>>>> On Tue, Jan 8, 2019 at 1:11 PM Ilya Biryukov <ibiryukov at google.com>
>>>> wrote:
>>>>
>>>>> Hi Nico,
>>>>>
>>>>> This is clearly a bug, it's supposed to search in a sibling directory.
>>>>> Are you running clang as './clang' in the scripts? The code seems to
>>>>> break in that case, https://reviews.llvm.org/D56446 should fix this.
>>>>>
>>>>> On Tue, Jan 8, 2019 at 5:12 PM Nico Weber <thakis at chromium.org> wrote:
>>>>>
>>>>>> 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?
>>>>>>
>>>>>> As-is, this seems to break chromium's clang ability to find libc++
>>>>>> headers (https://crbug.com/919761) 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
>>>>>> https://codereview.chromium.org/281753002).
>>>>>>
>>>>>> On Mon, Nov 12, 2018 at 8:58 AM Ilya Biryukov via cfe-commits <
>>>>>> cfe-commits at lists.llvm.org> wrote:
>>>>>>
>>>>>>> Author: ibiryukov
>>>>>>> Date: Mon Nov 12 05:55:55 2018
>>>>>>> New Revision: 346652
>>>>>>>
>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=346652&view=rev
>>>>>>> Log:
>>>>>>> Make clang-based tools find libc++ on MacOS
>>>>>>>
>>>>>>> Summary:
>>>>>>> When they read compiler args from compile_commands.json.
>>>>>>> This change allows to run clang-based tools, like clang-tidy or
>>>>>>> clangd,
>>>>>>> built from head using the compile_commands.json file produced for
>>>>>>> XCode
>>>>>>> toolchains.
>>>>>>>
>>>>>>> On MacOS clang can find the C++ standard library relative to the
>>>>>>> compiler installation dir.
>>>>>>>
>>>>>>> The logic to do this was based on resource dir as an approximation of
>>>>>>> where the compiler is installed. This broke the tools that read
>>>>>>> 'compile_commands.json' and don't ship with the compiler, as they
>>>>>>> typically change resource dir.
>>>>>>>
>>>>>>> To workaround this, we now use compiler install dir detected by the
>>>>>>> driver
>>>>>>> to better mimic the behavior of the original compiler when replaying
>>>>>>> the
>>>>>>> compilations using other tools.
>>>>>>>
>>>>>>> Reviewers: sammccall, arphaman, EricWF
>>>>>>>
>>>>>>> Reviewed By: sammccall
>>>>>>>
>>>>>>> Subscribers: ioeric, christof, kadircet, cfe-commits
>>>>>>>
>>>>>>> Differential Revision: https://reviews.llvm.org/D54310
>>>>>>>
>>>>>>> Added:
>>>>>>> cfe/trunk/test/Tooling/Inputs/mock-libcxx/
>>>>>>> cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/
>>>>>>> cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/
>>>>>>> cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/v1/
>>>>>>>
>>>>>>> cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector
>>>>>>> cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp
>>>>>>> cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp
>>>>>>> Modified:
>>>>>>> cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
>>>>>>> cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
>>>>>>> cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
>>>>>>> cfe/trunk/lib/Tooling/Tooling.cpp
>>>>>>>
>>>>>>> Modified: cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearchOptions.h?rev=346652&r1=346651&r2=346652&view=diff
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/include/clang/Lex/HeaderSearchOptions.h (original)
>>>>>>> +++ cfe/trunk/include/clang/Lex/HeaderSearchOptions.h Mon Nov 12
>>>>>>> 05:55:55 2018
>>>>>>> @@ -108,6 +108,13 @@ public:
>>>>>>> /// etc.).
>>>>>>> std::string ResourceDir;
>>>>>>>
>>>>>>> + /// Compiler install dir as detected by the Driver.
>>>>>>> + /// This is typically the directory that contains the clang
>>>>>>> executable, i.e.
>>>>>>> + /// the 'bin/' subdir of a clang distribution.
>>>>>>> + /// Only used to add include dirs for libc++ on Darwin. Please
>>>>>>> avoid relying
>>>>>>> + /// on this field for other purposes.
>>>>>>> + std::string InstallDir;
>>>>>>> +
>>>>>>> /// The directory used for the module cache.
>>>>>>> std::string ModuleCachePath;
>>>>>>>
>>>>>>>
>>>>>>> Modified: cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp?rev=346652&r1=346651&r2=346652&view=diff
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
>>>>>>> (original)
>>>>>>> +++ cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp Mon
>>>>>>> Nov 12 05:55:55 2018
>>>>>>> @@ -11,17 +11,18 @@
>>>>>>> //
>>>>>>>
>>>>>>> //===----------------------------------------------------------------------===//
>>>>>>>
>>>>>>> -#include "clang/Frontend/Utils.h"
>>>>>>> #include "clang/Basic/DiagnosticOptions.h"
>>>>>>> +#include "clang/Driver/Action.h"
>>>>>>> #include "clang/Driver/Compilation.h"
>>>>>>> #include "clang/Driver/Driver.h"
>>>>>>> -#include "clang/Driver/Action.h"
>>>>>>> #include "clang/Driver/Options.h"
>>>>>>> #include "clang/Driver/Tool.h"
>>>>>>> #include "clang/Frontend/CompilerInstance.h"
>>>>>>> #include "clang/Frontend/FrontendDiagnostic.h"
>>>>>>> +#include "clang/Frontend/Utils.h"
>>>>>>> #include "llvm/Option/ArgList.h"
>>>>>>> #include "llvm/Support/Host.h"
>>>>>>> +#include "llvm/Support/Path.h"
>>>>>>> using namespace clang;
>>>>>>> using namespace llvm::opt;
>>>>>>>
>>>>>>> @@ -102,5 +103,8 @@ std::unique_ptr<CompilerInvocation> clan
>>>>>>> CCArgs.size(),
>>>>>>> *Diags))
>>>>>>> return nullptr;
>>>>>>> + // Patch up the install dir, so we find the same standard library
>>>>>>> as the
>>>>>>> + // original compiler on MacOS.
>>>>>>> + CI->getHeaderSearchOpts().InstallDir =
>>>>>>> TheDriver.getInstalledDir();
>>>>>>> return CI;
>>>>>>> }
>>>>>>>
>>>>>>> Modified: cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitHeaderSearch.cpp?rev=346652&r1=346651&r2=346652&view=diff
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/lib/Frontend/InitHeaderSearch.cpp (original)
>>>>>>> +++ cfe/trunk/lib/Frontend/InitHeaderSearch.cpp Mon Nov 12 05:55:55
>>>>>>> 2018
>>>>>>> @@ -476,14 +476,9 @@ void InitHeaderSearch::AddDefaultInclude
>>>>>>> if (triple.isOSDarwin()) {
>>>>>>> // On Darwin, libc++ may be installed alongside the
>>>>>>> compiler in
>>>>>>> // include/c++/v1.
>>>>>>> - if (!HSOpts.ResourceDir.empty()) {
>>>>>>> - // Remove version from foo/lib/clang/version
>>>>>>> - StringRef NoVer =
>>>>>>> llvm::sys::path::parent_path(HSOpts.ResourceDir);
>>>>>>> - // Remove clang from foo/lib/clang
>>>>>>> - StringRef Lib = llvm::sys::path::parent_path(NoVer);
>>>>>>> - // Remove lib from foo/lib
>>>>>>> - SmallString<128> P = llvm::sys::path::parent_path(Lib);
>>>>>>> -
>>>>>>> + if (!HSOpts.InstallDir.empty()) {
>>>>>>> + // Get from foo/bin to foo.
>>>>>>> + SmallString<128>
>>>>>>> P(llvm::sys::path::parent_path(HSOpts.InstallDir));
>>>>>>> // Get foo/include/c++/v1
>>>>>>> llvm::sys::path::append(P, "include", "c++", "v1");
>>>>>>> AddUnmappedPath(P, CXXSystem, false);
>>>>>>>
>>>>>>> Modified: cfe/trunk/lib/Tooling/Tooling.cpp
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?rev=346652&r1=346651&r2=346652&view=diff
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/lib/Tooling/Tooling.cpp (original)
>>>>>>> +++ cfe/trunk/lib/Tooling/Tooling.cpp Mon Nov 12 05:55:55 2018
>>>>>>> @@ -327,6 +327,9 @@ bool ToolInvocation::run() {
>>>>>>> Invocation->getPreprocessorOpts().addRemappedFile(It.getKey(),
>>>>>>>
>>>>>>> Input.release());
>>>>>>> }
>>>>>>> + // Patch up the install dir, so we find the same standard library
>>>>>>> as the
>>>>>>> + // original compiler on MacOS.
>>>>>>> + Invocation->getHeaderSearchOpts().InstallDir =
>>>>>>> Driver->getInstalledDir();
>>>>>>> return runInvocation(BinaryName, Compilation.get(),
>>>>>>> std::move(Invocation),
>>>>>>> std::move(PCHContainerOps));
>>>>>>> }
>>>>>>>
>>>>>>> Added:
>>>>>>> cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c%2B%2B/v1/mock_vector?rev=346652&view=auto
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> ---
>>>>>>> cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector (added)
>>>>>>> +++
>>>>>>> cfe/trunk/test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector Mon
>>>>>>> Nov 12 05:55:55 2018
>>>>>>> @@ -0,0 +1 @@
>>>>>>> +class vector {};
>>>>>>>
>>>>>>> Added: cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp?rev=346652&view=auto
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp (added)
>>>>>>> +++ cfe/trunk/test/Tooling/clang-check-mac-libcxx-abspath.cpp Mon
>>>>>>> Nov 12 05:55:55 2018
>>>>>>> @@ -0,0 +1,17 @@
>>>>>>> +// Clang on MacOS can find libc++ living beside the installed
>>>>>>> compiler.
>>>>>>> +// This test makes sure our libTooling-based tools emulate this
>>>>>>> properly.
>>>>>>> +//
>>>>>>> +// RUN: rm -rf %t
>>>>>>> +// RUN: mkdir %t
>>>>>>> +//
>>>>>>> +// Install the mock libc++ (simulates the libc++ directory
>>>>>>> structure).
>>>>>>> +// RUN: cp -r %S/Inputs/mock-libcxx %t/
>>>>>>> +//
>>>>>>> +// Pretend clang is installed beside the mock library that we
>>>>>>> provided.
>>>>>>> +// 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
>>>>>>> +// RUN: cp "%s" "%t/test.cpp"
>>>>>>> +// clang-check will produce an error code if the mock library is
>>>>>>> not found.
>>>>>>> +// RUN: clang-check -p "%t" "%t/test.cpp"
>>>>>>> +
>>>>>>> +#include <mock_vector>
>>>>>>> +vector v;
>>>>>>>
>>>>>>> Added: cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp
>>>>>>> URL:
>>>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp?rev=346652&view=auto
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp (added)
>>>>>>> +++ cfe/trunk/test/Tooling/clang-check-mac-libcxx-relpath.cpp Mon
>>>>>>> Nov 12 05:55:55 2018
>>>>>>> @@ -0,0 +1,17 @@
>>>>>>> +// Clang on MacOS can find libc++ living beside the installed
>>>>>>> compiler.
>>>>>>> +// This test makes sure our libTooling-based tools emulate this
>>>>>>> properly.
>>>>>>> +//
>>>>>>> +// RUN: rm -rf %t
>>>>>>> +// RUN: mkdir %t
>>>>>>> +//
>>>>>>> +// Install the mock libc++ (simulates the libc++ directory
>>>>>>> structure).
>>>>>>> +// RUN: cp -r %S/Inputs/mock-libcxx %t/
>>>>>>> +//
>>>>>>> +// Pretend clang is installed beside the mock library that we
>>>>>>> provided.
>>>>>>> +// 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
>>>>>>> +// RUN: cp "%s" "%t/test.cpp"
>>>>>>> +// clang-check will produce an error code if the mock library is
>>>>>>> not found.
>>>>>>> +// RUN: clang-check -p "%t" "%t/test.cpp"
>>>>>>> +
>>>>>>> +#include <mock_vector>
>>>>>>> +vector v;
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> cfe-commits mailing list
>>>>>>> cfe-commits at lists.llvm.org
>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>>>
>>>>>>
>>>>>
>>>>> --
>>>>> Regards,
>>>>> Ilya Biryukov
>>>>>
>>>>
>>>
>>> --
>>> Regards,
>>> Ilya Biryukov
>>>
>>
--
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190129/93b1b4b7/attachment-0001.html>
More information about the cfe-commits
mailing list