[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Pavel Kosov via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1aab5e653d2c: [LLDB] Provide target specific directories to 
libclang (authored by kpdev42).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110827/new/

https://reviews.llvm.org/D110827

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h
  lldb/unittests/Expression/CppModuleConfigurationTest.cpp

Index: lldb/unittests/Expression/CppModuleConfigurationTest.cpp
===
--- lldb/unittests/Expression/CppModuleConfigurationTest.cpp
+++ lldb/unittests/Expression/CppModuleConfigurationTest.cpp
@@ -58,7 +58,6 @@
   return std::string(resource_dir);
 }
 
-
 TEST_F(CppModuleConfigurationTest, Linux) {
   // Test the average Linux configuration.
 
@@ -69,12 +68,32 @@
 // 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 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.
 
@@ -85,7 +104,7 @@
 // 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));
@@ -101,7 +120,7 @@
 // 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));
@@ -119,12 +138,33 @@
 // 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 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"));
+  

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Raphael Isemann via Phabricator via lldb-commits]

teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for fixing this!




Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:86
 
-  // 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 inc_path;
+  // Target specific paths contains /usr/include, so we check them first

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110827/new/

https://reviews.llvm.org/D110827

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 updated this revision to Diff 388118.
kpdev42 added a comment.

Addressed review comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110827/new/

https://reviews.llvm.org/D110827

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h
  lldb/unittests/Expression/CppModuleConfigurationTest.cpp

Index: lldb/unittests/Expression/CppModuleConfigurationTest.cpp
===
--- lldb/unittests/Expression/CppModuleConfigurationTest.cpp
+++ lldb/unittests/Expression/CppModuleConfigurationTest.cpp
@@ -58,7 +58,6 @@
   return std::string(resource_dir);
 }
 
-
 TEST_F(CppModuleConfigurationTest, Linux) {
   // Test the average Linux configuration.
 
@@ -69,12 +68,32 @@
 // 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 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.
 
@@ -85,7 +104,7 @@
 // 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));
@@ -101,7 +120,7 @@
 // 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));
@@ -119,12 +138,33 @@
 // 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 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, 

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Raphael Isemann via Phabricator via lldb-commits]

teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

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).




Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:34
 
-bool CppModuleConfiguration::analyzeFile(const FileSpec ) {
+static void getTargetIncludePaths(const llvm::Triple ,
+  std::vector ) {

Could this just return a `llvm::SmallVector` ? Then you can 
just use it directly in the for-range below without any variable.

Some docs would be nice:
```
lang=c++
/// Returns the target-specific default include paths for libc.
```



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:46
+
+static bool guessIncludePath(llvm::StringRef pathToFile,
+ llvm::StringRef pattern, llvm::StringRef ) 
{

Could this just return `llvm::Optional`? 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)



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:46
+
+static bool guessIncludePath(llvm::StringRef pathToFile,
+ llvm::StringRef pattern, llvm::StringRef ) 
{

teemperor wrote:
> Could this just return `llvm::Optional`? 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)
```
lang=c++
/// Returns the include path matching the given pattern for the given file path 
(or None if the path doesn't match the pattern).
```



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:78
+
+posix_dir.consume_back("c++/v1");
+llvm::SmallVector tmp;

```
lang=c++
// Check if this is a target-specific libc++ include directory.
```



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp:81
+return m_std_target_inc.TrySet(
+(posix_dir + triple.str() + "/c++/v1").toStringRef(tmp));
   }

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



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h:46
+  /// If valid, the per-target include path used for the std module.
+  SetOncePath m_std_target_inc;
   /// If valid, the include path to the C library (e.g. /usr/include).

Please add a comment that this is optional unlike the other members:
```/// This is an optional path only required on some systems.```



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h:51
+  /// (e.g. /usr/include/x86_64-linux-gnu).
+  SetOncePath m_c_target_inc;
   /// The Clang resource include path for this configuration.

Same as above.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h:65
   /// Creates a configuration by analyzing the given list of used source files.
-  explicit CppModuleConfiguration(const FileSpecList _files);
+  explicit CppModuleConfiguration(const FileSpecList _files,
+  const llvm::Triple );

```
lang=c++
/// The triple (if valid) is used to search for target-specific include paths.
```



Comment at: lldb/unittests/Expression/CppModuleConfigurationTest.cpp:80
+   libcpp_target));
 }
 

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.



Comment at: lldb/unittests/Expression/CppModuleConfigurationTest.cpp:132
   EXPECT_THAT(config.GetIncludeDirs(),
-  testing::ElementsAre(libcpp, ResourceInc(), usr));
+  testing::ElementsAre(libcpp, ResourceInc(), usr, libcpp_target));
 }

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110827/new/

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110827/new/

https://reviews.llvm.org/D110827

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110827/new/

https://reviews.llvm.org/D110827

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110827/new/

https://reviews.llvm.org/D110827

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110827/new/

https://reviews.llvm.org/D110827

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110827/new/

https://reviews.llvm.org/D110827

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110827/new/

https://reviews.llvm.org/D110827

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110827/new/

https://reviews.llvm.org/D110827

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110827: [LLDB] Provide target specific directories to libclang

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 created this revision.
kpdev42 added reviewers: clayborg, granata.enrico, labath.
kpdev42 added a project: LLDB.
Herald added subscribers: JDevlieghere, pengfei.
kpdev42 requested review of this revision.
Herald added a subscriber: lldb-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110827

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CppModuleConfiguration.h
  lldb/unittests/Expression/CppModuleConfigurationTest.cpp

Index: lldb/unittests/Expression/CppModuleConfigurationTest.cpp
===
--- lldb/unittests/Expression/CppModuleConfigurationTest.cpp
+++ lldb/unittests/Expression/CppModuleConfigurationTest.cpp
@@ -63,16 +63,20 @@
   // 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::vector files = {// C library
-usr + "/stdio.h",
-// C++ library
-libcpp + "/vector",
-libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  std::string libcpp_target = "/usr/include/x86_64-unknown-linux-gnu/c++/v1";
+  std::vector 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));
+  testing::ElementsAre(libcpp, ResourceInc(), usr, usr_target,
+   libcpp_target));
 }
 
 TEST_F(CppModuleConfigurationTest, Sysroot) {
@@ -85,7 +89,7 @@
 // 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));
@@ -101,7 +105,7 @@
 // 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));
@@ -112,6 +116,8 @@
 
   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 files = {// C library
 usr + "/stdio.h",
 // unrelated library
@@ -119,10 +125,11 @@
 // C++ library
 libcpp + "/vector",
 libcpp + "/module.modulemap"};
-  CppModuleConfiguration config(makeFiles(files));
+  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));
+  testing::ElementsAre(libcpp, ResourceInc(), usr, libcpp_target));
 }
 
 TEST_F(CppModuleConfigurationTest, Xcode) {
@@ -139,7 +146,7 @@
   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));
@@ -154,7 +161,7 @@
 // C++ library
 libcpp + "/vector",
 libcpp + "/module.modulemap"};
-