[clang-tools-extra] ab714ba - Revert "Revert "[clangd] Canonicalize compile flags before applying edits""
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 27 05:55:30 PDT 2021
Author: Kadir Cetinkaya
Date: 2021-07-27T14:49:53+02:00
New Revision: ab714ba056c14bce00ab67cc10e34678f9d77b5a
URL: https://github.com/llvm/llvm-project/commit/ab714ba056c14bce00ab67cc10e34678f9d77b5a
DIFF: https://github.com/llvm/llvm-project/commit/ab714ba056c14bce00ab67cc10e34678f9d77b5a.diff
LOG: Revert "Revert "[clangd] Canonicalize compile flags before applying edits""
Set driver mode before parsing arglist.
Depends on D106789.
Differential Revision: https://reviews.llvm.org/D106794
Added:
Modified:
clang-tools-extra/clangd/CompileCommands.cpp
clang-tools-extra/clangd/test/did-change-configuration-params.test
clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp
index ffc66303c9fc4..ec0e2160abbad 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -9,11 +9,17 @@
#include "CompileCommands.h"
#include "Config.h"
#include "support/Logger.h"
+#include "support/Trace.h"
+#include "clang/Driver/Driver.h"
#include "clang/Driver/Options.h"
+#include "clang/Driver/ToolChain.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Tooling/ArgumentsAdjusters.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Option/ArgList.h"
#include "llvm/Option/Option.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/Debug.h"
@@ -188,11 +194,43 @@ CommandMangler CommandMangler::detect() {
return Result;
}
-CommandMangler CommandMangler::forTests() {
- return CommandMangler();
-}
+CommandMangler CommandMangler::forTests() { return CommandMangler(); }
void CommandMangler::adjust(std::vector<std::string> &Cmd) const {
+ trace::Span S("AdjustCompileFlags");
+ auto &OptTable = clang::driver::getDriverOptTable();
+ // OriginalArgs needs to outlive ArgList.
+ llvm::SmallVector<const char *, 16> OriginalArgs;
+ OriginalArgs.reserve(Cmd.size());
+ for (const auto &S : Cmd)
+ OriginalArgs.push_back(S.c_str());
+ bool IsCLMode =
+ !OriginalArgs.empty() &&
+ driver::IsClangCL(driver::getDriverMode(
+ OriginalArgs[0], llvm::makeArrayRef(OriginalArgs).slice(1)));
+ // ParseArgs propagates missig arg/opt counts on error, but preserves
+ // everything it could parse in ArgList. So we just ignore those counts.
+ unsigned IgnoredCount;
+ auto ArgList = OptTable.ParseArgs(
+ OriginalArgs, IgnoredCount, IgnoredCount,
+ /*FlagsToInclude=*/
+ IsCLMode ? (driver::options::CLOption | driver::options::CoreOption)
+ : /*everything*/ 0,
+ /*FlagsToExclude=*/driver::options::NoDriverOption |
+ (IsCLMode ? 0 : driver::options::CLOption));
+
+ // Move the inputs to the end, separated via `--` from flags. This ensures
+ // modifications done in the following steps apply in more cases (like setting
+ // -x, which only affects inputs that come after it).
+ if (!ArgList.hasArgNoClaim(driver::options::OPT__DASH_DASH)) {
+ // In theory there might be more than one input, but clangd can't deal with
+ // them anyway.
+ if (auto *Input = ArgList.getLastArg(driver::options::OPT_INPUT)) {
+ Cmd.insert(Cmd.end(), {"--", Input->getAsString(ArgList)});
+ Cmd.erase(Cmd.begin() + Input->getIndex());
+ }
+ }
+
for (auto &Edit : Config::current().CompileFlags.Edits)
Edit(Cmd);
diff --git a/clang-tools-extra/clangd/test/did-change-configuration-params.test b/clang-tools-extra/clangd/test/did-change-configuration-params.test
index 19d41d0812e23..e90e26c0d3a7a 100644
--- a/clang-tools-extra/clangd/test/did-change-configuration-params.test
+++ b/clang-tools-extra/clangd/test/did-change-configuration-params.test
@@ -48,7 +48,7 @@
#
# ERR: ASTWorker building file {{.*}}foo.c version 0 with command
# ERR: [{{.*}}clangd-test2]
-# ERR: clang -c foo.c -Wall -Werror
+# ERR: clang -c -Wall -Werror {{.*}} -- foo.c
---
{"jsonrpc":"2.0","id":5,"method":"shutdown"}
---
diff --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
index de8ff2b4a14e1..79da2f059a8c2 100644
--- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -133,8 +133,9 @@ TEST_F(BackgroundIndexTest, Config) {
Opts.ContextProvider = [](PathRef P) {
Config C;
if (P.endswith("foo.cpp"))
- C.CompileFlags.Edits.push_back(
- [](std::vector<std::string> &Argv) { Argv.push_back("-Done=two"); });
+ C.CompileFlags.Edits.push_back([](std::vector<std::string> &Argv) {
+ Argv = tooling::getInsertArgumentAdjuster("-Done=two")(Argv, "");
+ });
if (P.endswith("baz.cpp"))
C.Index.Background = Config::BackgroundPolicy::Skip;
return Context::current().derive(Config::Key, std::move(C));
diff --git a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
index f5727305b465c..15b5dddadd262 100644
--- a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -11,6 +11,8 @@
#include "TestFS.h"
#include "support/Context.h"
+#include "clang/Tooling/ArgumentsAdjusters.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/FileSystem.h"
@@ -165,13 +167,15 @@ TEST(CommandMangler, ConfigEdits) {
for (char &C : Arg)
C = llvm::toUpper(C);
});
- Cfg.CompileFlags.Edits.push_back(
- [](std::vector<std::string> &Argv) { Argv.push_back("--hello"); });
+ Cfg.CompileFlags.Edits.push_back([](std::vector<std::string> &Argv) {
+ Argv = tooling::getInsertArgumentAdjuster("--hello")(Argv, "");
+ });
WithContextValue WithConfig(Config::Key, std::move(Cfg));
Mangler.adjust(Cmd);
}
- // Edits are applied in given order and before other mangling.
- EXPECT_THAT(Cmd, ElementsAre(_, "FOO.CC", "--hello"));
+ // Edits are applied in given order and before other mangling and they always
+ // go before filename.
+ EXPECT_THAT(Cmd, ElementsAre(_, "--hello", "--", "FOO.CC"));
}
static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) {
@@ -342,6 +346,27 @@ TEST(PrintArgvTest, All) {
EXPECT_EQ(Expected, printArgv(Args));
}
+TEST(CommandMangler, InputsAfterDashDash) {
+ const auto Mangler = CommandMangler::forTests();
+ {
+ std::vector<std::string> Args = {"clang", "/Users/foo.cc"};
+ Mangler.adjust(Args);
+ EXPECT_THAT(llvm::makeArrayRef(Args).take_back(2),
+ ElementsAre("--", "/Users/foo.cc"));
+ EXPECT_THAT(llvm::makeArrayRef(Args).drop_back(2),
+ Not(Contains("/Users/foo.cc")));
+ }
+ // In CL mode /U triggers an undef operation, hence `/Users/foo.cc` shouldn't
+ // be interpreted as a file.
+ {
+ std::vector<std::string> Args = {"clang", "--driver-mode=cl", "bar.cc",
+ "/Users/foo.cc"};
+ Mangler.adjust(Args);
+ EXPECT_THAT(llvm::makeArrayRef(Args).take_back(2),
+ ElementsAre("--", "bar.cc"));
+ EXPECT_THAT(llvm::makeArrayRef(Args).drop_back(2), Not(Contains("bar.cc")));
+ }
+}
} // namespace
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list