[clang-tools-extra] [clangd] Forward --target to system include extraction (PR #65824)

[Matthew Mirvish via cfe-commits]

https://github.com/mincrmatt12 review_requested 
https://github.com/llvm/llvm-project/pull/65824
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Forward --target to system include extraction (PR #65824)

[Matthew Mirvish via cfe-commits]

https://github.com/mincrmatt12 created 
https://github.com/llvm/llvm-project/pull/65824:

Some clang multilib configurations (such as the one currently used in the [beta 
ARM LLVM 
toolchain](https://github.com/ARM-software/LLVM-embedded-toolchain-for-Arm)) 
wind up only reporting the C++ include paths properly if they get passed the 
correct target. This patch ensures the `--target` (or `-target`) arguments are 
correctly sent to the queried driver.

>From dac47fee4d165870732bda7cc91cefd68d6b14e6 Mon Sep 17 00:00:00 2001
From: Matthew Mirvish 
Date: Fri, 8 Sep 2023 19:59:58 -0400
Subject: [PATCH] [clangd] Forward --target to system include extraction

Some clang multilib configurations (such as the one currently used in
the beta ARM LLVM toolchain) wind up only reporting the C++ include paths
properly if they get passed the correct target.
---
 clang-tools-extra/clangd/SystemIncludeExtractor.cpp  | 12 ++--
 .../clangd/test/system-include-extractor.test|  5 +++--
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/clangd/SystemIncludeExtractor.cpp 
b/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
index ac99b220f6bd3f0..0c1ed810a37f7a4 100644
--- a/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
+++ b/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
@@ -87,12 +87,13 @@ struct DriverArgs {
   std::string Lang;
   std::string Sysroot;
   std::string ISysroot;
+  std::string Target;
 
   bool operator==(const DriverArgs &RHS) const {
 return std::tie(Driver, StandardIncludes, StandardCXXIncludes, Lang,
-Sysroot, ISysroot) ==
+Sysroot, ISysroot, Target) ==
std::tie(RHS.Driver, RHS.StandardIncludes, RHS.StandardCXXIncludes,
-RHS.Lang, RHS.Sysroot, ISysroot);
+RHS.Lang, RHS.Sysroot, ISysroot, Target);
   }
 
   DriverArgs(const tooling::CompileCommand &Cmd, llvm::StringRef File) {
@@ -130,6 +131,11 @@ struct DriverArgs {
   ISysroot = Cmd.CommandLine[I + 1];
 else
   ISysroot = Arg.str();
+  } else if (Arg.consume_front("--target=")) {
+Target = Arg.str();
+  } else if (Arg.consume_front("-target")) {
+if (Arg.empty() && I + 1 < E)
+  Target = Cmd.CommandLine[I + 1];
   }
 }
 
@@ -167,6 +173,8 @@ struct DriverArgs {
   Args.append({"--sysroot", Sysroot});
 if (!ISysroot.empty())
   Args.append({"-isysroot", ISysroot});
+if (!Target.empty())
+  Args.append({"-target", Target});
 return Args;
   }
 
diff --git a/clang-tools-extra/clangd/test/system-include-extractor.test 
b/clang-tools-extra/clangd/test/system-include-extractor.test
index 2a2c900316a9161..66882e424bb9216 100644
--- a/clang-tools-extra/clangd/test/system-include-extractor.test
+++ b/clang-tools-extra/clangd/test/system-include-extractor.test
@@ -17,6 +17,7 @@
 # RUN: echo '[ -z "${args##*"-nostdinc"*}" ] || exit' >> 
%t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"--sysroot /my/sysroot/path"*}" ] || exit' >> 
%t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"-isysroot /isysroot"*}" ] || exit' >> 
%t.dir/bin/my_driver.sh
+# RUN: echo '[ -z "${args##*"-target arm-linux-gnueabihf"*}" ] || exit' >> 
%t.dir/bin/my_driver.sh
 # RUN: echo 'echo line to ignore >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "Target: arm-linux-gnueabihf\r\n" >&2' >> 
%t.dir/bin/my_driver.sh
 # RUN: echo 'printf "#include <...> search starts here:\r\n" >&2' >> 
%t.dir/bin/my_driver.sh
@@ -36,7 +37,7 @@
 
 # Generate a compile_commands.json that will query the mock driver we've
 # created. Which should add a.h and b.h into include search path.
-# RUN: echo '[{"directory": "%/t.dir", "command": "my_driver.sh the-file.cpp 
-nostdinc --sysroot /my/sysroot/path -isysroot/isysroot", "file": 
"the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "my_driver.sh the-file.cpp 
--target=arm-linux-gnueabihf -nostdinc --sysroot /my/sysroot/path 
-isysroot/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
 
 # RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
 # On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
@@ -74,7 +75,7 @@
 {"jsonrpc":"2.0","method":"exit"}
 
 # Generate a different compile_commands.json which does not point to the mock 
driver
-# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp -nostdinc 
--sysroot /my/sysroot/path -isysroot/isysroot", "file": "the-file.cpp"}]' > 
%t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp 
--target=arm-linux-gnueabihf -nostdinc --sysroot /my/sysroot/path 
-isysroot/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
 
 # Generate a clangd config file which points to the mock driver instead
 # RUN: echo 'CompileFlags:' > %t.dir/.clangd

___
cfe-commits mailing list
cfe-commits@l

[clang-tools-extra] [clangd] Forward --target to system include extraction (PR #65824)

[Matthew Mirvish via cfe-commits]

https://github.com/mincrmatt12 updated 
https://github.com/llvm/llvm-project/pull/65824:

>From e5f74b064b303a53b267caa12738af381c17c65f Mon Sep 17 00:00:00 2001
From: Matthew Mirvish 
Date: Fri, 8 Sep 2023 19:59:58 -0400
Subject: [PATCH] [clangd] Forward --target to system include extraction

Some clang multilib configurations (such as the one currently used in
the beta ARM LLVM toolchain) wind up only reporting the C++ include paths
properly if they get passed the correct target.
---
 clang-tools-extra/clangd/SystemIncludeExtractor.cpp  | 12 ++--
 .../clangd/test/system-include-extractor.test|  5 +++--
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/clangd/SystemIncludeExtractor.cpp 
b/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
index ac99b220f6bd3f0..88df5b04ccb09f3 100644
--- a/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
+++ b/clang-tools-extra/clangd/SystemIncludeExtractor.cpp
@@ -87,12 +87,13 @@ struct DriverArgs {
   std::string Lang;
   std::string Sysroot;
   std::string ISysroot;
+  std::string Target;
 
   bool operator==(const DriverArgs &RHS) const {
 return std::tie(Driver, StandardIncludes, StandardCXXIncludes, Lang,
-Sysroot, ISysroot) ==
+Sysroot, ISysroot, Target) ==
std::tie(RHS.Driver, RHS.StandardIncludes, RHS.StandardCXXIncludes,
-RHS.Lang, RHS.Sysroot, ISysroot);
+RHS.Lang, RHS.Sysroot, RHS.ISysroot, RHS.Target);
   }
 
   DriverArgs(const tooling::CompileCommand &Cmd, llvm::StringRef File) {
@@ -130,6 +131,11 @@ struct DriverArgs {
   ISysroot = Cmd.CommandLine[I + 1];
 else
   ISysroot = Arg.str();
+  } else if (Arg.consume_front("--target=")) {
+Target = Arg.str();
+  } else if (Arg.consume_front("-target")) {
+if (Arg.empty() && I + 1 < E)
+  Target = Cmd.CommandLine[I + 1];
   }
 }
 
@@ -167,6 +173,8 @@ struct DriverArgs {
   Args.append({"--sysroot", Sysroot});
 if (!ISysroot.empty())
   Args.append({"-isysroot", ISysroot});
+if (!Target.empty())
+  Args.append({"-target", Target});
 return Args;
   }
 
diff --git a/clang-tools-extra/clangd/test/system-include-extractor.test 
b/clang-tools-extra/clangd/test/system-include-extractor.test
index 2a2c900316a9161..66882e424bb9216 100644
--- a/clang-tools-extra/clangd/test/system-include-extractor.test
+++ b/clang-tools-extra/clangd/test/system-include-extractor.test
@@ -17,6 +17,7 @@
 # RUN: echo '[ -z "${args##*"-nostdinc"*}" ] || exit' >> 
%t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"--sysroot /my/sysroot/path"*}" ] || exit' >> 
%t.dir/bin/my_driver.sh
 # RUN: echo '[ -z "${args##*"-isysroot /isysroot"*}" ] || exit' >> 
%t.dir/bin/my_driver.sh
+# RUN: echo '[ -z "${args##*"-target arm-linux-gnueabihf"*}" ] || exit' >> 
%t.dir/bin/my_driver.sh
 # RUN: echo 'echo line to ignore >&2' >> %t.dir/bin/my_driver.sh
 # RUN: echo 'printf "Target: arm-linux-gnueabihf\r\n" >&2' >> 
%t.dir/bin/my_driver.sh
 # RUN: echo 'printf "#include <...> search starts here:\r\n" >&2' >> 
%t.dir/bin/my_driver.sh
@@ -36,7 +37,7 @@
 
 # Generate a compile_commands.json that will query the mock driver we've
 # created. Which should add a.h and b.h into include search path.
-# RUN: echo '[{"directory": "%/t.dir", "command": "my_driver.sh the-file.cpp 
-nostdinc --sysroot /my/sysroot/path -isysroot/isysroot", "file": 
"the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "my_driver.sh the-file.cpp 
--target=arm-linux-gnueabihf -nostdinc --sysroot /my/sysroot/path 
-isysroot/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
 
 # RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
 # On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
@@ -74,7 +75,7 @@
 {"jsonrpc":"2.0","method":"exit"}
 
 # Generate a different compile_commands.json which does not point to the mock 
driver
-# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp -nostdinc 
--sysroot /my/sysroot/path -isysroot/isysroot", "file": "the-file.cpp"}]' > 
%t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp 
--target=arm-linux-gnueabihf -nostdinc --sysroot /my/sysroot/path 
-isysroot/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
 
 # Generate a clangd config file which points to the mock driver instead
 # RUN: echo 'CompileFlags:' > %t.dir/.clangd

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

[clang-tools-extra] [clangd] Forward --target to system include extraction (PR #65824)

[Matthew Mirvish via cfe-commits]

@@ -87,12 +87,13 @@ struct DriverArgs {
   std::string Lang;
   std::string Sysroot;
   std::string ISysroot;
+  std::string Target;
 
   bool operator==(const DriverArgs &RHS) const {
 return std::tie(Driver, StandardIncludes, StandardCXXIncludes, Lang,
-Sysroot, ISysroot) ==
+Sysroot, ISysroot, Target) ==
std::tie(RHS.Driver, RHS.StandardIncludes, RHS.StandardCXXIncludes,
-RHS.Lang, RHS.Sysroot, ISysroot);
+RHS.Lang, RHS.Sysroot, ISysroot, Target);

mincrmatt12 wrote:

Fixed in latest revision (both for `Target` and the existing bug)

https://github.com/llvm/llvm-project/pull/65824
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clangd] Forward --target to system include extraction (PR #65824)

[Matthew Mirvish via cfe-commits]

https://github.com/mincrmatt12 resolved 
https://github.com/llvm/llvm-project/pull/65824
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Driver] Ensure clang can construct toolchain target names (PR #115798)

[Matthew Mirvish via cfe-commits]

https://github.com/mincrmatt12 created 
https://github.com/llvm/llvm-project/pull/115798

This adds an extra validation step to 
`ToolChain::getTargetAndModeFromProgramName` to actually try constructing the 
target info for the triple it deduces, instead of just relying on the LLVM 
target registry.

Some targets, like `xtensa`, are supported by LLVM but are not supported by 
clang, so just checking the LLVM registry causes inconsistency.

This is based on the discussion from clangd/clangd#2184 (and indeed would solve 
that issue) - the approach here of "construct the target info and check for 
null" is taken from 
[clangd](https://github.com/llvm/llvm-project/blob/5716f836d25e93bf8f664a14fe55c70e07a369be/clang-tools-extra/clangd/SystemIncludeExtractor.cpp#L253).

>From f111040a50db8275b1d6aaee3cf5e4b8af2b08f5 Mon Sep 17 00:00:00 2001
From: Matthew Mirvish 
Date: Mon, 11 Nov 2024 20:20:20 -0500
Subject: [PATCH] [Clang][Driver] Ensure clang can construct toolchain target
 names

This adds an extra validation step to
`ToolChain::getTargetAndModeFromProgramName` to actually try
constructing the target info for the triple it deduces, instead of just
relying on the LLVM target registry.

Some targets, like `xtensa`, are supported by LLVM but are not supported
by clang, so just checking the LLVM registry causes inconsistency.
---
 clang/lib/Driver/ToolChain.cpp | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 646dbc0faf5a9f..e094a5fd0267f3 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -16,6 +16,8 @@
 #include "ToolChains/InterfaceStubs.h"
 #include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/Sanitizers.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Basic/TargetOptions.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Action.h"
 #include "clang/Driver/Driver.h"
@@ -496,8 +498,21 @@ ToolChain::getTargetAndModeFromProgramName(StringRef PN) {
   StringRef Prefix(ProgName);
   Prefix = Prefix.slice(0, LastComponent);
   std::string IgnoredError;
+
+  // First, check if the target is a supported LLVM target.
   bool IsRegistered =
   llvm::TargetRegistry::lookupTarget(std::string(Prefix), IgnoredError);
+  // If so, further check that clang is able to use it as a target.
+  if (IsRegistered) {
+std::shared_ptr TargetOpts(new TargetOptions);
+TargetOpts->Triple = Prefix.str();
+DiagnosticsEngine Diags(new DiagnosticIDs, new DiagnosticOptions,
+new IgnoringDiagConsumer);
+llvm::IntrusiveRefCntPtr Target =
+clang::TargetInfo::CreateTargetInfo(Diags, TargetOpts);
+if (!Target)
+  IsRegistered = false;
+  }
   return ParsedClangName{std::string(Prefix), ModeSuffix, DS->ModeFlag,
  IsRegistered};
 }

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