kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Pushes input for the compile action to the end while separating with a
`--` before applying other manglings. This ensures edits that effect only the
arguments that come after them works, like changing parse language via -x.
Fixes https://github.com/clangd/clangd/issues/555.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D106527
Files:
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
Index: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -11,6 +11,7 @@
#include "TestFS.h"
#include "support/Context.h"
+#include "clang/Tooling/ArgumentsAdjusters.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/FileSystem.h"
@@ -199,13 +200,15 @@
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", "-fsyntax-only"));
+ // Edits are applied in given order and before other mangling and they always
+ // go before filename.
+ EXPECT_THAT(Cmd, ElementsAre(_, "--hello", "-fsyntax-only", "--", "FOO.CC"));
}
static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) {
Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -133,8 +133,9 @@
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));
Index: clang-tools-extra/clangd/test/did-change-configuration-params.test
===================================================================
--- clang-tools-extra/clangd/test/did-change-configuration-params.test
+++ 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"}
---
Index: clang-tools-extra/clangd/CompileCommands.cpp
===================================================================
--- clang-tools-extra/clangd/CompileCommands.cpp
+++ clang-tools-extra/clangd/CompileCommands.cpp
@@ -9,9 +9,12 @@
#include "CompileCommands.h"
#include "Config.h"
#include "support/Logger.h"
+#include "support/Trace.h"
#include "clang/Driver/Options.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Tooling/ArgumentsAdjusters.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Option/ArgList.h"
#include "llvm/Option/Option.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/Debug.h"
@@ -185,23 +188,36 @@
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 std::string &S : Cmd)
+ OriginalArgs.push_back(S.c_str());
+ // 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);
+
+ // 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);
- // 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;
- };
-
// clangd should not write files to disk, including dependency files
// requested on the command line.
Cmd = tooling::getClangStripDependencyFileAdjuster()(Cmd, "");
@@ -212,12 +228,16 @@
Cmd = tooling::getClangSyntaxOnlyAdjuster()(Cmd, "");
std::vector<std::string> ToAppend;
- if (ResourceDir && !Has("-resource-dir"))
+ if (ResourceDir &&
+ !ArgList.hasArgNoClaim(driver::options::OPT_resource_dir,
+ driver::options::OPT_resource_dir_EQ))
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 && !ArgList.hasArgNoClaim(driver::options::OPT__sysroot,
+ driver::options::OPT__sysroot_EQ,
+ driver::options::OPT_isysroot)) {
ToAppend.push_back("-isysroot");
ToAppend.push_back(*Sysroot);
}
@@ -228,7 +248,8 @@
}
if (!Cmd.empty()) {
- bool FollowSymlink = !Has("-no-canonical-prefixes");
+ bool FollowSymlink =
+ !ArgList.hasArgNoClaim(driver::options::OPT_no_canonical_prefixes);
Cmd.front() =
(FollowSymlink ? ResolvedDrivers : ResolvedDriversNoFollow)
.get(Cmd.front(), [&, this] {
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
