[LLDB] Provide target specific directories to libclang

[kpdev42]

Summary

On Linux some C++ and C include files reside in target specific directories, like /usr/include/x86_64-linux-gnu.
Patch adds them to libclang, so LLDB jitter has more chances to compile expression.

OS Laboratory. Huawei Russian Research Institute. Saint-Petersburg

[teemperor]

Thanks for the patch! Out of curiosity: What distribution is using target-specific include paths?

I can take a look at this tomorrow, but just from scrolling over I think the general direction of this patch seems fine, so I think I only have a few nits here and there.

[clayborg]

Should we be testing if these directories exist before trying to use them? Might be nice to avoid compiler warnings if the compiler will emit warnings or errors if these directories don't exist.

LLDB also tests with compilers that were built, like when LLDB builds clang and uses that clang and clang++ that it built to run the test suite. If we had settings in LLDB that users could set, then the test suite would be able to use the include files for the compiler that is being used instead of always defaulting to the system headers.

[kpdev42]

In D110827#3034767, @clayborg wrote:

Should we be testing if these directories exist before trying to use them? Might be nice to avoid compiler warnings if the compiler will emit warnings or errors if these directories don't exist.

I think testing if these directories exists may be a little bit redundant because clang ignores non-existent include paths passed with -I

LLDB also tests with compilers that were built, like when LLDB builds clang and uses that clang and clang++ that it built to run the test suite. If we had settings in LLDB that users could set, then the test suite would be able to use the include files for the compiler that is being used instead of always defaulting to the system headers.

Could you please clarify: "LLDB builds clang" - here you mean clang which was build with LLDB? And I would like to mention that starting from https://reviews.llvm.org/D89013 libcxx puts __config_site header to target specific folder

[clayborg]

In D110827#3042820, @kpdev42 wrote:

In D110827#3034767, @clayborg wrote:

Should we be testing if these directories exist before trying to use them? Might be nice to avoid compiler warnings if the compiler will emit warnings or errors if these directories don't exist.

I think testing if these directories exists may be a little bit redundant because clang ignores non-existent include paths passed with -I

no worries then

LLDB also tests with compilers that were built, like when LLDB builds clang and uses that clang and clang++ that it built to run the test suite. If we had settings in LLDB that users could set, then the test suite would be able to use the include files for the compiler that is being used instead of always defaulting to the system headers.

Could you please clarify: "LLDB builds clang" - here you mean clang which was build with LLDB? And I would like to mention that starting from https://reviews.llvm.org/D89013 libcxx puts __config_site header to target specific folder

We often build the full clang compiler during LLDB builds and then use the clang we built as the compiler when running our test suite. So anything we can do to make sure what ever clang we use for building the test suite binaries has all of the headers that it was built with along with any support library headers (libcxx, etc) that would be great.

[kpdev42]

In D110827#3059120, @clayborg wrote:

In D110827#3042820, @kpdev42 wrote:

In D110827#3034767, @clayborg wrote:

LLDB also tests with compilers that were built, like when LLDB builds clang and uses that clang and clang++ that it built to run the test suite. If we had settings in LLDB that users could set, then the test suite would be able to use the include files for the compiler that is being used instead of always defaulting to the system headers.

Could you please clarify: "LLDB builds clang" - here you mean clang which was build with LLDB? And I would like to mention that starting from https://reviews.llvm.org/D89013 libcxx puts __config_site header to target specific folder

We often build the full clang compiler during LLDB builds and then use the clang we built as the compiler when running our test suite. So anything we can do to make sure what ever clang we use for building the test suite binaries has all of the headers that it was built with along with any support library headers (libcxx, etc) that would be great.

That's fine, but patch is about *libclang* seeing target specific headers, not clang frontend. While clang frontend obtains system header paths through Toolchain-derived classes (lib/Driver/Toolchain), one has to *explicitly* set those paths when calling libclang. That's exactly what lldb does in CppModuleConfiguration.cpp when evaluating expression.

[kpdev42]

Ping

[clayborg]

Looks good to me, but the expression parser isn't my main area of expertise. I would like to see someone with more knowledge in the expression parser to make the final ok. Raphael?

[teemperor]

Apologies for the delay, this slipped out of my review queue by accident.

The patch looks in general pretty much ready, I just have a few nits here and there.

(Also just to clarify what this code does. It scans the support files of some (LLDB) modules and then returns a list of include paths that are needed to compile the correct libc++/libc of the target. I fixed up the class docs a bit to make this clearer).

lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp

34–62

Could this just return a llvm::SmallVector<std::string, 2> ? Then you can just use it directly in the for-range below without any variable.

Some docs would be nice:

/// Returns the target-specific default include paths for libc.

46

Could this just return llvm::Optional<std::string>? Also the parameter should technically be path_to_file (this file uses LLDB's variable_naming_style instead of the usualNamingStyle, even though the existing code already has some code style issues where it deviated towards LLVM's style but oh well)

46

/// Returns the include path matching the given pattern for the given file path (or None if the path doesn't match the pattern).

80

// Check if this is a target-specific libc++ include directory.

83

.str() instead of .toStringRef(tmp) should also work.

lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h

46

Please add a comment that this is optional unlike the other members:

/// This is an optional path only required on some systems.

52

Same as above.

67–69

/// The triple (if valid) is used to search for target-specific include paths.

lldb/unittests/Expression/CppModuleConfigurationTest.cpp

75

Could you please duplicate this test and make the target-specific directory its own new test? E.g. TEST_F(CppModuleConfigurationTest, LinuxTargetSpecificInclude). Just to make sure we also still support the old behaviour.

144

Same as above: both the new and old behaviour should work so just copy this into a new test.

[kpdev42]

Addressed review comments

[teemperor]

LGTM, thanks for fixing this!

lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp

86

This can be moved to the assignments below (and then the double parenthesis can disappear too).

lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp

if (log && log->GetVerbose()) {
     for (const FileSpec &f : files)
       LLDB_LOGV(log, "[C++ module config] Analyzing support file: {0}",
                 f.GetPath());
   }
 
   // Try to create a configuration from the files. If there is no valid
   // configuration possible with the files, this just returns an invalid
   // configuration.
-  return CppModuleConfiguration(files);
+  return CppModuleConfiguration(files, target->GetArchitecture().GetTriple());
 }
 
 bool ClangUserExpression::PrepareForParsing(
     DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
     bool for_completion) {
   InstallContext(exe_ctx);
 
   if (!SetupPersistentState(diagnostic_manager, exe_ctx))

lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h

llvm::StringRef Get() const {
       return m_path;
     }
     /// Returns true iff this path was set exactly once so far.
     bool Valid() const { return m_valid; }
   };
 
   /// If valid, the include path used for the std module.
   SetOncePath m_std_inc;
+  /// If valid, the per-target include path used for the std module.
+  /// This is an optional path only required on some systems.
+  SetOncePath m_std_target_inc;
   /// If valid, the include path to the C library (e.g. /usr/include).
   SetOncePath m_c_inc;
+  /// If valid, the include path to target-specific C library files
+  /// (e.g. /usr/include/x86_64-linux-gnu).
+  /// This is an optional path only required on some systems.
+  SetOncePath m_c_target_inc;
   /// The Clang resource include path for this configuration.
   std::string m_resource_inc;
 
   std::vector<std::string> m_include_dirs;
   std::vector<std::string> m_imported_modules;
 
   /// Analyze a given source file to build the current configuration.
   /// Returns false iff there was a fatal error that makes analyzing any
   /// further files pointless as the configuration is now invalid.
-  bool analyzeFile(const FileSpec &f);
+  bool analyzeFile(const FileSpec &f, const llvm::Triple &triple);
 
 public:
   /// Creates a configuration by analyzing the given list of used source files.
-  explicit CppModuleConfiguration(const FileSpecList &support_files);
+  /// The triple (if valid) is used to search for target-specific include paths.
+  explicit CppModuleConfiguration(const FileSpecList &support_files,
+                                  const llvm::Triple &triple);
   /// Creates an empty and invalid configuration.
   CppModuleConfiguration() = default;
 
   /// Returns true iff this is a valid configuration that can be used to
   /// load and compile modules.
   bool hasValidConfig();
 
   /// Returns a list of include directories that should be used when using this

lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp

 //===-- CppModuleConfiguration.cpp ----------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "CppModuleConfiguration.h"
 
 #include "ClangHost.h"
 #include "lldb/Host/FileSystem.h"
+#include "llvm/ADT/Triple.h"
 
 using namespace lldb_private;
 
 bool CppModuleConfiguration::SetOncePath::TrySet(llvm::StringRef path) {
   // Setting for the first time always works.
   if (m_first) {
     m_path = path.str();
     m_valid = true;
     m_first = false;
     return true;
   }
   // Changing the path to the same value is fine.
   if (m_path == path)
     return true;
 
   // Changing the path after it was already set is not allowed.
   m_valid = false;
   return false;
 }
 
-bool CppModuleConfiguration::analyzeFile(const FileSpec &f) {
+static llvm::SmallVector<std::string, 2>
+getTargetIncludePaths(const llvm::Triple &triple) {
+  llvm::SmallVector<std::string, 2> paths;
+  if (!triple.str().empty()) {
+    paths.push_back("/usr/include/" + triple.str());
+    if (!triple.getArchName().empty() ||
+        triple.getOSAndEnvironmentName().empty())
+      paths.push_back(("/usr/include/" + triple.getArchName() + "-" +
+                       triple.getOSAndEnvironmentName())
+                          .str());
+  }
+  return paths;
+}
+
+/// Returns the include path matching the given pattern for the given file
+/// path (or None if the path doesn't match the pattern).
+static llvm::Optional<llvm::StringRef>
+guessIncludePath(llvm::StringRef path_to_file, llvm::StringRef pattern) {
+  if (pattern.empty())
+    return llvm::NoneType();
+  size_t pos = path_to_file.find(pattern);
+  if (pos == llvm::StringRef::npos)
+    return llvm::NoneType();
+
+  return path_to_file.substr(0, pos + pattern.size());
+}
+
+bool CppModuleConfiguration::analyzeFile(const FileSpec &f,
+                                         const llvm::Triple &triple) {
   using namespace llvm::sys::path;
   // Convert to slashes to make following operations simpler.
   std::string dir_buffer = convert_to_slash(f.GetDirectory().GetStringRef());
   llvm::StringRef posix_dir(dir_buffer);
 
   // Check for /c++/vX/ that is used by libc++.
   static llvm::Regex libcpp_regex(R"regex(/c[+][+]/v[0-9]/)regex");
   // If the path is in the libc++ include directory use it as the found libc++
   // path. Ignore subdirectories such as /c++/v1/experimental as those don't
   // need to be specified in the header search.
   if (libcpp_regex.match(f.GetPath()) &&
       parent_path(posix_dir, Style::posix).endswith("c++")) {
-    return m_std_inc.TrySet(posix_dir);
+    if (!m_std_inc.TrySet(posix_dir))
+      return false;
+    if (triple.str().empty())
+      return true;
+
+    posix_dir.consume_back("c++/v1");
+    // Check if this is a target-specific libc++ include directory.
+    return m_std_target_inc.TrySet(
+        (posix_dir + triple.str() + "/c++/v1").str());
   }
 
-  // Check for /usr/include. On Linux this might be /usr/include/bits, so
-  // we should remove that '/bits' suffix to get the actual include directory.
-  if (posix_dir.endswith("/usr/include/bits"))
-    posix_dir.consume_back("/bits");
-  if (posix_dir.endswith("/usr/include"))
-    return m_c_inc.TrySet(posix_dir);
+  llvm::Optional<llvm::StringRef> inc_path;
+  // Target specific paths contains /usr/include, so we check them first
+  for (auto &path : getTargetIncludePaths(triple)) {
+    if ((inc_path = guessIncludePath(posix_dir, path)))
+      return m_c_target_inc.TrySet(*inc_path);
+  }
+  if ((inc_path = guessIncludePath(posix_dir, "/usr/include")))
+    return m_c_inc.TrySet(*inc_path);
 
   // File wasn't interesting, continue analyzing.
   return true;
 }
 
 /// Utility function for just appending two paths.
 static std::string MakePath(llvm::StringRef lhs, llvm::StringRef rhs) {
   llvm::SmallString<256> result(lhs);
for (llvm::StringRef file_to_check : files_to_check) {
     if (!FileSystem::Instance().Exists(file_to_check))
       return false;
   }
 
   return true;
 }
 
 CppModuleConfiguration::CppModuleConfiguration(
-    const FileSpecList &support_files) {
+    const FileSpecList &support_files, const llvm::Triple &triple) {
   // Analyze all files we were given to build the configuration.
   bool error = !llvm::all_of(support_files,
                              std::bind(&CppModuleConfiguration::analyzeFile,
-                                       this, std::placeholders::_1));
+                                       this, std::placeholders::_1, triple));
   // If we have a valid configuration at this point, set the
   // include directories and module list that should be used.
   if (!error && hasValidConfig()) {
     // Calculate the resource directory for LLDB.
     llvm::SmallString<256> resource_dir;
     llvm::sys::path::append(resource_dir, GetClangResourceDir().GetPath(),
                             "include");
     m_resource_inc = std::string(resource_dir.str());
 
     // This order matches the way Clang orders these directories.
     m_include_dirs = {m_std_inc.Get().str(), m_resource_inc,
                       m_c_inc.Get().str()};
+    if (m_c_target_inc.Valid())
+      m_include_dirs.push_back(m_c_target_inc.Get().str());
+    if (m_std_target_inc.Valid())
+      m_include_dirs.push_back(m_std_target_inc.Get().str());
     m_imported_modules = {"std"};
   }
 }

lldb/unittests/Expression/CppModuleConfigurationTest.cpp

 /// Returns the Clang resource include directory.
 static std::string ResourceInc() {
   llvm::SmallString<256> resource_dir;
   llvm::sys::path::append(resource_dir, GetClangResourceDir().GetPath(),
                           "include");
   return std::string(resource_dir);
 }
 
-
 TEST_F(CppModuleConfigurationTest, Linux) {
   // Test the average Linux configuration.
 
   std::string usr = "/usr/include";
   std::string libcpp = "/usr/include/c++/v1";
   std::vector<std::string> files = {// C library
                                     usr + "/stdio.h",
                                     // C++ library
                                     libcpp + "/vector",
                                     libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
               testing::ElementsAre(libcpp, ResourceInc(), usr));
 }
 
+TEST_F(CppModuleConfigurationTest, LinuxTargetSpecificInclude) {
+  // Test the average Linux configuration.
+
+  std::string usr = "/usr/include";
+  std::string usr_target = "/usr/include/x86_64-linux-gnu";
+  std::string libcpp = "/usr/include/c++/v1";
+  std::string libcpp_target = "/usr/include/x86_64-unknown-linux-gnu/c++/v1";
+  std::vector<std::string> files = {
+      // C library
+      usr + "/stdio.h", usr_target + "/sys/cdefs.h",
+      // C++ library
+      libcpp + "/vector", libcpp + "/module.modulemap"};
+  CppModuleConfiguration config(makeFiles(files),
+                                llvm::Triple("x86_64-unknown-linux-gnu"));
+  EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
+  EXPECT_THAT(config.GetIncludeDirs(),
+              testing::ElementsAre(libcpp, ResourceInc(), usr, usr_target,
+                                   libcpp_target));
+}
+
 TEST_F(CppModuleConfigurationTest, Sysroot) {
   // Test that having a sysroot for the whole system works fine.
 
   std::string libcpp = "/home/user/sysroot/usr/include/c++/v1";
   std::string usr = "/home/user/sysroot/usr/include";
   std::vector<std::string> files = {// C library
                                     usr + "/stdio.h",
                                     // C++ library
                                     libcpp + "/vector",
                                     libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
               testing::ElementsAre(libcpp, ResourceInc(), usr));
 }
 
 TEST_F(CppModuleConfigurationTest, LinuxLocalLibCpp) {
   // Test that a locally build libc++ is detected.
 
   std::string usr = "/usr/include";
   std::string libcpp = "/home/user/llvm-build/include/c++/v1";
   std::vector<std::string> files = {// C library
                                     usr + "/stdio.h",
                                     // C++ library
                                     libcpp + "/vector",
                                     libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
               testing::ElementsAre(libcpp, ResourceInc(), usr));
 }
 
 TEST_F(CppModuleConfigurationTest, UnrelatedLibrary) {
   // Test that having an unrelated library in /usr/include doesn't break.
 
   std::string usr = "/usr/include";
   std::string libcpp = "/home/user/llvm-build/include/c++/v1";
   std::vector<std::string> files = {// C library
                                     usr + "/stdio.h",
                                     // unrelated library
                                     usr + "/boost/vector",
                                     // C++ library
                                     libcpp + "/vector",
                                     libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
               testing::ElementsAre(libcpp, ResourceInc(), usr));
 }
 
+TEST_F(CppModuleConfigurationTest, UnrelatedLibraryWithTargetSpecificInclude) {
+  // Test that having an unrelated library in /usr/include doesn't break.
+
+  std::string usr = "/usr/include";
+  std::string libcpp = "/home/user/llvm-build/include/c++/v1";
+  std::string libcpp_target =
+      "/home/user/llvm-build/include/x86_64-unknown-linux-gnu/c++/v1";
+  std::vector<std::string> files = {// C library
+                                    usr + "/stdio.h",
+                                    // unrelated library
+                                    usr + "/boost/vector",
+                                    // C++ library
+                                    libcpp + "/vector",
+                                    libcpp + "/module.modulemap"};
+  CppModuleConfiguration config(makeFiles(files),
+                                llvm::Triple("x86_64-unknown-linux-gnu"));
+  EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
+  EXPECT_THAT(config.GetIncludeDirs(),
+              testing::ElementsAre(libcpp, ResourceInc(), usr, libcpp_target));
+}
+
 TEST_F(CppModuleConfigurationTest, Xcode) {
   // Test detection of libc++ coming from Xcode with generic platform names.
 
   std::string p = "/Applications/Xcode.app/Contents/Developer/";
   std::string libcpp = p + "Toolchains/B.xctoolchain/usr/include/c++/v1";
   std::string usr =
       p + "Platforms/A.platform/Developer/SDKs/OSVers.sdk/usr/include";
   std::vector<std::string> files = {
       // C library
       usr + "/stdio.h",
       // C++ library
       libcpp + "/vector",
       libcpp + "/module.modulemap",
   };
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
               testing::ElementsAre(libcpp, ResourceInc(), usr));
 }
 
 TEST_F(CppModuleConfigurationTest, LibCppV2) {
   // Test that a "v2" of libc++ is still correctly detected.
 
   std::string libcpp = "/usr/include/c++/v2";
   std::vector<std::string> files = {// C library
                                     "/usr/include/stdio.h",
                                     // C++ library
                                     libcpp + "/vector",
                                     libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
               testing::ElementsAre("/usr/include/c++/v2", ResourceInc(),
                                    "/usr/include"));
 }
 
 TEST_F(CppModuleConfigurationTest, UnknownLibCppFile) {
   // Test that having some unknown file in the libc++ path doesn't break
   // anything.
 
   std::string libcpp = "/usr/include/c++/v1";
   std::vector<std::string> files = {// C library
                                     "/usr/include/stdio.h",
                                     // C++ library
                                     libcpp + "/non_existing_file",
                                     libcpp + "/module.modulemap",
                                     libcpp + "/vector"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre("std"));
   EXPECT_THAT(config.GetIncludeDirs(),
               testing::ElementsAre("/usr/include/c++/v1", ResourceInc(),
                                    "/usr/include"));
 }
 
 TEST_F(CppModuleConfigurationTest, MissingUsrInclude) {
   // Test that we don't load 'std' if we can't find the C standard library.
 
   std::string libcpp = "/usr/include/c++/v1";
   std::vector<std::string> files = {// C++ library
                                     libcpp + "/vector",
                                     libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre());
   EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre());
 }
 
 TEST_F(CppModuleConfigurationTest, MissingLibCpp) {
   // Test that we don't load 'std' if we don't have a libc++.
 
   std::string usr = "/usr/include";
   std::vector<std::string> files = {
       // C library
       usr + "/stdio.h",
   };
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre());
   EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre());
 }
 
 TEST_F(CppModuleConfigurationTest, IgnoreLibStdCpp) {
   // Test that we don't do anything bad when we encounter libstdc++ paths.
 
   std::string usr = "/usr/include";
   std::vector<std::string> files = {
       // C library
       usr + "/stdio.h",
       // C++ library
       usr + "/c++/8.0.1/vector",
   };
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre());
   EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre());
 }
 
 TEST_F(CppModuleConfigurationTest, AmbiguousCLib) {
   // Test that we don't do anything when we are not sure where the
   // right C standard library is.
 
   std::string usr1 = "/usr/include";
   std::string usr2 = "/usr/include/other/path";
   std::string libcpp = usr1 + "c++/v1";
   std::vector<std::string> files = {
       // First C library
       usr1 + "/stdio.h",
       // Second C library
       usr2 + "/stdio.h",
       // C++ library
       libcpp + "/vector",
       libcpp + "/module.modulemap",
   };
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre());
   EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre());
 }
 
 TEST_F(CppModuleConfigurationTest, AmbiguousLibCpp) {
   // Test that we don't do anything when we are not sure where the
   // right libc++ is.
 
   std::string usr = "/usr/include";
   std::string libcpp1 = usr + "c++/v1";
   std::string libcpp2 = usr + "c++/v2";
   std::vector<std::string> files = {
       // C library
       usr + "/stdio.h",
       // First C++ library
       libcpp1 + "/vector",
       libcpp1 + "/module.modulemap",
       // Second C++ library
       libcpp2 + "/vector",
       libcpp2 + "/module.modulemap",
   };
-  CppModuleConfiguration config(makeFiles(files));
+  CppModuleConfiguration config(makeFiles(files), llvm::Triple());
   EXPECT_THAT(config.GetImportedModules(), testing::ElementsAre());
   EXPECT_THAT(config.GetIncludeDirs(), testing::ElementsAre());
 }