[clang-tools-extra] [clangd] Ensure `-isysroot` in the original command is respected (PR #75694)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Dec 16 00:47:21 PST 2023
https://github.com/kon72 created https://github.com/llvm/llvm-project/pull/75694
In CommandMangler, it guesses sysroot path of the host system and adds that through `-isysroot` flag only when there is no `--sysroot` or `-isysroot` flag beforehand.
Previously, it could not find `-isysroot<sysroot>` flag in the original compile command, resulting in duplicate `-isysroot` flags.
This commit fixes it and ensures the explicitly specified sysroot is respected. Also adds unit tests for this fix.
>From 1a6a38fdaa5722f6e3d0cba372459cd1e0feb69d Mon Sep 17 00:00:00 2001
From: kon72 <kinsei0916 at gmail.com>
Date: Sat, 16 Dec 2023 17:39:46 +0900
Subject: [PATCH] [clangd] Ensure `-isysroot` in the original command is
respected
In CommandMangler, it guesses sysroot path of the host system and adds that through `-isysroot` flag only when there is no `--sysroot` or `-isysroot` flag beforehand.
Previously, it could not find `-isysroot<sysroot>` flag in the original compile command, resulting in duplicate `-isysroot` flags.
This commit fixes it and ensures the explicitly specified sysroot is respected.
Also adds unit tests for this fix.
---
clang-tools-extra/clangd/CompileCommands.cpp | 23 +++---
.../clangd/unittests/CompileCommandsTests.cpp | 81 ++++++++++++++++++-
2 files changed, 91 insertions(+), 13 deletions(-)
diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp
index f43ce928463b90..a3c6f3f41118e4 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -328,13 +328,15 @@ void CommandMangler::operator()(tooling::CompileCommand &Command,
tooling::addTargetAndModeForProgramName(Cmd, Cmd.front());
- // Check whether the flag exists, either as -flag or -flag=*
- auto Has = [&](llvm::StringRef Flag) {
- for (llvm::StringRef Arg : Cmd) {
- if (Arg.consume_front(Flag) && (Arg.empty() || Arg[0] == '='))
- return true;
- }
- return false;
+ // Check whether the flag exists in the command.
+ auto HasExact = [&](llvm::StringRef Flag) {
+ return llvm::any_of(Cmd, [&](llvm::StringRef Arg) { return Arg == Flag; });
+ };
+
+ // Check whether the flag appears in the command as a prefix.
+ auto HasPrefix = [&](llvm::StringRef Flag) {
+ return llvm::any_of(
+ Cmd, [&](llvm::StringRef Arg) { return Arg.startswith(Flag); });
};
llvm::erase_if(Cmd, [](llvm::StringRef Elem) {
@@ -342,12 +344,13 @@ void CommandMangler::operator()(tooling::CompileCommand &Command,
});
std::vector<std::string> ToAppend;
- if (ResourceDir && !Has("-resource-dir"))
+ if (ResourceDir && !HasExact("-resource-dir") && !HasPrefix("-resource-dir="))
ToAppend.push_back(("-resource-dir=" + *ResourceDir));
// Don't set `-isysroot` if it is already set or if `--sysroot` is set.
// `--sysroot` is a superset of the `-isysroot` argument.
- if (Sysroot && !Has("-isysroot") && !Has("--sysroot")) {
+ if (Sysroot && !HasPrefix("-isysroot") && !HasExact("--sysroot") &&
+ !HasPrefix("--sysroot=")) {
ToAppend.push_back("-isysroot");
ToAppend.push_back(*Sysroot);
}
@@ -358,7 +361,7 @@ void CommandMangler::operator()(tooling::CompileCommand &Command,
}
if (!Cmd.empty()) {
- bool FollowSymlink = !Has("-no-canonical-prefixes");
+ bool FollowSymlink = !HasExact("-no-canonical-prefixes");
Cmd.front() =
(FollowSymlink ? ResolvedDrivers : ResolvedDriversNoFollow)
.get(Cmd.front(), [&, this] {
diff --git a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
index 28f0d85d332caa..cad135923f71c1 100644
--- a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -385,9 +385,8 @@ TEST(ArgStripperTest, OrderDependent) {
}
TEST(PrintArgvTest, All) {
- std::vector<llvm::StringRef> Args = {
- "one", "two", "thr ee", "f\"o\"ur", "fi\\ve", "$"
- };
+ std::vector<llvm::StringRef> Args = {"one", "two", "thr ee",
+ "f\"o\"ur", "fi\\ve", "$"};
const char *Expected = R"(one two "thr ee" "f\"o\"ur" "fi\\ve" $)";
EXPECT_EQ(Expected, printArgv(Args));
}
@@ -465,6 +464,82 @@ TEST(CommandMangler, PathsAsPositional) {
Mangler(Cmd, "a.cc");
EXPECT_THAT(Cmd.CommandLine, Contains("foo"));
}
+
+TEST(CommandMangler, RespectsOriginalResourceDir) {
+ auto Mangler = CommandMangler::forTests();
+ Mangler.ResourceDir = testPath("fake/resources");
+
+ {
+ tooling::CompileCommand Cmd;
+ Cmd.CommandLine = {"clang++", "-resource-dir", testPath("true/resources"),
+ "foo.cc"};
+ Mangler(Cmd, "foo.cc");
+ EXPECT_THAT(llvm::join(Cmd.CommandLine, " "),
+ HasSubstr("-resource-dir " + testPath("true/resources")));
+ EXPECT_THAT(llvm::join(Cmd.CommandLine, " "),
+ Not(HasSubstr(testPath("fake/resources"))));
+ }
+
+ {
+ tooling::CompileCommand Cmd;
+ Cmd.CommandLine = {"clang++", "-resource-dir=" + testPath("true/resources"),
+ "foo.cc"};
+ Mangler(Cmd, "foo.cc");
+ EXPECT_THAT(llvm::join(Cmd.CommandLine, " "),
+ HasSubstr("-resource-dir=" + testPath("true/resources")));
+ EXPECT_THAT(llvm::join(Cmd.CommandLine, " "),
+ Not(HasSubstr(testPath("fake/resources"))));
+ }
+}
+
+TEST(CommandMangler, RespectsOriginalSysroot) {
+ auto Mangler = CommandMangler::forTests();
+ Mangler.Sysroot = testPath("fake/sysroot");
+
+ {
+ tooling::CompileCommand Cmd;
+ Cmd.CommandLine = {"clang++", "-isysroot", testPath("true/sysroot"),
+ "foo.cc"};
+ Mangler(Cmd, "foo.cc");
+ EXPECT_THAT(llvm::join(Cmd.CommandLine, " "),
+ HasSubstr("-isysroot " + testPath("true/sysroot")));
+ EXPECT_THAT(llvm::join(Cmd.CommandLine, " "),
+ Not(HasSubstr(testPath("fake/sysroot"))));
+ }
+
+ {
+ tooling::CompileCommand Cmd;
+ Cmd.CommandLine = {"clang++", "-isysroot" + testPath("true/sysroot"),
+ "foo.cc"};
+ Mangler(Cmd, "foo.cc");
+ EXPECT_THAT(llvm::join(Cmd.CommandLine, " "),
+ HasSubstr("-isysroot" + testPath("true/sysroot")));
+ EXPECT_THAT(llvm::join(Cmd.CommandLine, " "),
+ Not(HasSubstr(testPath("fake/sysroot"))));
+ }
+
+ {
+ tooling::CompileCommand Cmd;
+ Cmd.CommandLine = {"clang++", "--sysroot", testPath("true/sysroot"),
+ "foo.cc"};
+ Mangler(Cmd, "foo.cc");
+ EXPECT_THAT(llvm::join(Cmd.CommandLine, " "),
+ HasSubstr("--sysroot " + testPath("true/sysroot")));
+ EXPECT_THAT(llvm::join(Cmd.CommandLine, " "),
+ Not(HasSubstr(testPath("fake/sysroot"))));
+ }
+
+ {
+ tooling::CompileCommand Cmd;
+ Cmd.CommandLine = {"clang++", "--sysroot=" + testPath("true/sysroot"),
+ "foo.cc"};
+ Mangler(Cmd, "foo.cc");
+ EXPECT_THAT(llvm::join(Cmd.CommandLine, " "),
+ HasSubstr("--sysroot=" + testPath("true/sysroot")));
+ EXPECT_THAT(llvm::join(Cmd.CommandLine, " "),
+ Not(HasSubstr(testPath("fake/sysroot"))));
+ }
+}
} // namespace
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list