ilya-biryukov created this revision.
ilya-biryukov added reviewers: JDevlieghere, arphaman.
Herald added a reviewer: EricWF.
Herald added subscribers: kadircet, christof, ioeric.

The intention is to make the tools replaying compilations from 
'compile_commands.json'
(clang-tidy, clangd, etc.) find the same standard library as the original 
compiler
specified in 'compile_commands.json'.

Previously, the library detection logic was in the frontend 
(InitHeaderSearch.cpp) and relied
on the value of resource dir as an approximation of the compiler install dir. 
The new logic
uses the actual compiler install dir and is performed in the driver. This is 
consistent with
the C++ standard library detection on other platforms and allows to override 
the resource dir
in the tools using the compile_commands.json without altering the
standard library detection mechanism. The tools have to override the resource 
dir to make sure
they use a consistent version of the builtin headers.

There is still logic in InitHeaderSearch that attemps to add the absolute 
includes for the
the C++ standard library, so we keep passing the -stdlib=libc++ from the driver 
to the frontend
via cc1 args to avoid breaking that. In the long run, we should move this logic 
to the driver too,
but it could potentially break the library detection on other systems, so we 
don't tackle it in this
patch to keep its scope manageable.

This is a second attempt to fix the issue, first one was commited in r346652 
and reverted in r346675.
The original fix relied on an ad-hoc propagation (bypassing the cc1 flags) of 
the install dir from the
driver to the frontend's HeaderSearchOptions. Unsurpisingly, the propagation 
was incomplete, it broke
the libc++ detection in clang itself, which caused LLDB tests to break.

The LLDB tests pass with new fix.


Repository:
  rC Clang

https://reviews.llvm.org/D54630

Files:
  
clang-tools-extra/test/clang-tidy/Inputs/mock-libcxx/include/c++/v1/mock_vector
  clang-tools-extra/test/clang-tidy/clang-tidy-mac-libcxx.cpp
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Darwin.h
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/test/Driver/darwin-stdlib.cpp
  clang/test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector
  clang/test/Tooling/clang-check-mac-libcxx-abspath.cpp
  clang/test/Tooling/clang-check-mac-libcxx-relpath.cpp

Index: clang/test/Tooling/clang-check-mac-libcxx-relpath.cpp
===================================================================
--- /dev/null
+++ clang/test/Tooling/clang-check-mac-libcxx-relpath.cpp
@@ -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;
Index: clang/test/Tooling/clang-check-mac-libcxx-abspath.cpp
===================================================================
--- /dev/null
+++ clang/test/Tooling/clang-check-mac-libcxx-abspath.cpp
@@ -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;
Index: clang/test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector
===================================================================
--- /dev/null
+++ clang/test/Tooling/Inputs/mock-libcxx/include/c++/v1/mock_vector
@@ -0,0 +1 @@
+class vector {};
Index: clang/test/Driver/darwin-stdlib.cpp
===================================================================
--- clang/test/Driver/darwin-stdlib.cpp
+++ clang/test/Driver/darwin-stdlib.cpp
@@ -2,19 +2,20 @@
 // than the platform default. (see https://llvm.org/bugs/show_bug.cgi?id=30548)
 // XFAIL: default-cxx-stdlib-set
 
-// RUN: %clang -target x86_64-apple-darwin -arch arm64 -miphoneos-version-min=7.0 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-LIBCXX
-// RUN: %clang -target x86_64-apple-darwin -mmacosx-version-min=10.8 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-LIBSTDCXX
-// RUN: %clang -target x86_64-apple-darwin -mmacosx-version-min=10.9 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-LIBCXX
-// RUN: %clang -target x86_64-apple-darwin -arch armv7s -miphoneos-version-min=6.1 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-LIBSTDCXX
-// RUN: %clang -target x86_64-apple-darwin -arch armv7s -miphoneos-version-min=7.0 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-LIBCXX
-// RUN: %clang -target x86_64-apple-darwin -arch armv7k %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-LIBCXX
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/ -arch arm64 -miphoneos-version-min=7.0 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-LIBCXX
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/ -mmacosx-version-min=10.8 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-LIBSTDCXX
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/ -mmacosx-version-min=10.9 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-LIBCXX
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/ -arch armv7s -miphoneos-version-min=6.1 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-LIBSTDCXX
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/ -arch armv7s -miphoneos-version-min=7.0 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-LIBCXX
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %S/Inputs/darwin_toolchain_tree/bin/ -arch armv7k %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-LIBCXX
 
 // The purpose of this test is that the libc++ headers should be found
-// properly. At the moment this is done by passing -stdlib=libc++ down to the
-// cc1 invocation. If and when we change to finding them in the driver this test
-// should reflect that.
+// properly. We also pass -stdlib=libc++ to make sure the logic to add the
+// optional absolute include for libc++ from InitHeaderSearch.cpp also fires.
 
-// CHECK-LIBCXX: -stdlib=libc++
+// CHECK-LIBCXX: "-stdlib=libc++"
+// CHECK-LIBCXX: "-internal-isystem" "{{[^"]*}}{{[/\\]}}Inputs{{[/\\]}}darwin_toolchain_tree{{[/\\]}}bin{{[/\\]}}include{{[/\\]}}c++{{[/\\]}}v1"
 
 // CHECK-LIBSTDCXX-NOT: -stdlib=libc++
 // CHECK-LIBSTDCXX-NOT: -stdlib=libstdc++
+// CHECK-LIBSTDCXX-NOT: -internal-isystem
Index: clang/lib/Frontend/InitHeaderSearch.cpp
===================================================================
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -473,22 +473,6 @@
   if (Lang.CPlusPlus && !Lang.AsmPreprocessor &&
       HSOpts.UseStandardCXXIncludes && HSOpts.UseStandardSystemIncludes) {
     if (HSOpts.UseLibcxx) {
-      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);
-
-          // Get foo/include/c++/v1
-          llvm::sys::path::append(P, "include", "c++", "v1");
-          AddUnmappedPath(P, CXXSystem, false);
-        }
-      }
       AddPath("/usr/include/c++/v1", CXXSystem, false);
     } else {
       AddDefaultCPlusPlusIncludePaths(Lang, triple, HSOpts);
Index: clang/lib/Driver/ToolChains/Darwin.h
===================================================================
--- clang/lib/Driver/ToolChains/Darwin.h
+++ clang/lib/Driver/ToolChains/Darwin.h
@@ -494,6 +494,10 @@
   void AddLinkRuntimeLibArgs(const llvm::opt::ArgList &Args,
                              llvm::opt::ArgStringList &CmdArgs) const override;
 
+  void AddClangCXXStdlibIncludeArgs(
+      const llvm::opt::ArgList &DriverArgs,
+      llvm::opt::ArgStringList &CC1Args) const override;
+
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
                            llvm::opt::ArgStringList &CmdArgs) const override;
 
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1676,6 +1676,38 @@
   }
 }
 
+void DarwinClang::AddClangCXXStdlibIncludeArgs(
+    const llvm::opt::ArgList &DriverArgs,
+    llvm::opt::ArgStringList &CC1Args) const {
+  // The implementation from a base class will pass through the -stdlib to
+  // CC1Args.
+  // FIXME: this should not be necessary, remove usages in the frontend
+  //        (e.g. HeaderSearchOptions::UseLibcxx) and don't pipe -stdlib.
+  ToolChain::AddClangCXXStdlibIncludeArgs(DriverArgs, CC1Args);
+
+  if (DriverArgs.hasArg(options::OPT_nostdlibinc) ||
+      DriverArgs.hasArg(options::OPT_nostdincxx))
+    return;
+
+  switch (GetCXXStdlibType(DriverArgs)) {
+  case ToolChain::CST_Libcxx: {
+    llvm::StringRef InstallDir = getDriver().getInstalledDir();
+    if (InstallDir.empty())
+      break;
+    // On Darwin, libc++ may be installed alongside the compiler in
+    // include/c++/v1.
+    // Get from 'foo/bin' to 'foo'.
+    SmallString<128> P = llvm::sys::path::parent_path(InstallDir);
+    // Get to 'foo/include/c++/v1'.
+    llvm::sys::path::append(P, "include", "c++", "v1");
+    addSystemInclude(DriverArgs, CC1Args, P);
+    break;
+  }
+  case ToolChain::CST_Libstdcxx:
+    // FIXME: should we do something about it?
+    break;
+  }
+}
 void DarwinClang::AddCXXStdlibLibArgs(const ArgList &Args,
                                       ArgStringList &CmdArgs) const {
   CXXStdlibType Type = GetCXXStdlibType(Args);
Index: clang-tools-extra/test/clang-tidy/clang-tidy-mac-libcxx.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/clang-tidy-mac-libcxx.cpp
@@ -0,0 +1,17 @@
+// Clang on MacOS can find libc++ living beside the installed compiler.
+// This test makes sure clang-tidy emulates 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"
+// RUN: clang-tidy -header-filter='.*' -system-headers -checks='-*,modernize-use-using'  "%t/test.cpp" | FileCheck %s
+// CHECK: mock_vector:{{[0-9]+}}:{{[0-9]+}}: warning: use 'using' instead of 'typedef'
+
+#include <mock_vector>
+typedef vector* vec_ptr;
Index: clang-tools-extra/test/clang-tidy/Inputs/mock-libcxx/include/c++/v1/mock_vector
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/Inputs/mock-libcxx/include/c++/v1/mock_vector
@@ -0,0 +1,2 @@
+class vector {};
+typedef vector* vector_ptr;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to