[clang-tools-extra] 2aa0cf1 - Revert D106562 "[clangd] Get rid of arg adjusters in CommandMangler"
Fangrui Song via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 23 09:50:55 PDT 2021
Author: Fangrui Song
Date: 2021-07-23T09:50:43-07:00
New Revision: 2aa0cf19e7fe17c9eb5eb2555e10184061b933f1
URL: https://github.com/llvm/llvm-project/commit/2aa0cf19e7fe17c9eb5eb2555e10184061b933f1
DIFF: https://github.com/llvm/llvm-project/commit/2aa0cf19e7fe17c9eb5eb2555e10184061b933f1.diff
LOG: Revert D106562 "[clangd] Get rid of arg adjusters in CommandMangler"
This reverts commit 1c0d0085bcaaf27cc8d9492eb3c5c05058e54b8e.
This commit made unittest BuildCompilerInvocation.DropsPlugins crash.
Added:
Modified:
clang-tools-extra/clangd/CompileCommands.cpp
clang-tools-extra/clangd/CompileCommands.h
clang-tools-extra/clangd/Compiler.cpp
clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
clang-tools-extra/clangd/unittests/CompilerTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp
index ffc66303c9fc..e749720b83a1 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -12,8 +12,6 @@
#include "clang/Driver/Options.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Tooling/ArgumentsAdjusters.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringRef.h"
#include "llvm/Option/Option.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/Debug.h"
@@ -22,7 +20,6 @@
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Program.h"
-#include <iterator>
#include <string>
#include <vector>
@@ -205,9 +202,14 @@ void CommandMangler::adjust(std::vector<std::string> &Cmd) const {
return false;
};
- llvm::erase_if(Cmd, [](llvm::StringRef Elem) {
- return Elem.startswith("--save-temps") || Elem.startswith("-save-temps");
- });
+ // clangd should not write files to disk, including dependency files
+ // requested on the command line.
+ Cmd = tooling::getClangStripDependencyFileAdjuster()(Cmd, "");
+ // Strip plugin related command line arguments. Clangd does
+ // not support plugins currently. Therefore it breaks if
+ // compiler tries to load plugins.
+ Cmd = tooling::getStripPluginsAdjuster()(Cmd, "");
+ Cmd = tooling::getClangSyntaxOnlyAdjuster()(Cmd, "");
std::vector<std::string> ToAppend;
if (ResourceDir && !Has("-resource-dir"))
@@ -221,8 +223,8 @@ void CommandMangler::adjust(std::vector<std::string> &Cmd) const {
}
if (!ToAppend.empty()) {
- Cmd.insert(llvm::find(Cmd, "--"), std::make_move_iterator(ToAppend.begin()),
- std::make_move_iterator(ToAppend.end()));
+ Cmd = tooling::getInsertArgumentAdjuster(
+ std::move(ToAppend), tooling::ArgumentInsertPosition::END)(Cmd, "");
}
if (!Cmd.empty()) {
diff --git a/clang-tools-extra/clangd/CompileCommands.h b/clang-tools-extra/clangd/CompileCommands.h
index 759472413fda..6e958d271c87 100644
--- a/clang-tools-extra/clangd/CompileCommands.h
+++ b/clang-tools-extra/clangd/CompileCommands.h
@@ -25,10 +25,6 @@ namespace clangd {
// - forcing the use of clangd's builtin headers rather than clang's
// - resolving argv0 as cc1 expects
// - injecting -isysroot flags on mac as the system clang does
-// FIXME: This is currently not used in all code paths that create invocations.
-// Make use of these adjusters and buildCompilerInvocation in clangd-indexer as
-// well. It should be possible to hook it up by overriding RunInvocation in
-// FrontendActionFactory.
struct CommandMangler {
// Absolute path to clang.
llvm::Optional<std::string> ClangPath;
diff --git a/clang-tools-extra/clangd/Compiler.cpp b/clang-tools-extra/clangd/Compiler.cpp
index f2fb4489f105..a7e48934ddaf 100644
--- a/clang-tools-extra/clangd/Compiler.cpp
+++ b/clang-tools-extra/clangd/Compiler.cpp
@@ -64,7 +64,6 @@ buildCompilerInvocation(const ParseInputs &Inputs, clang::DiagnosticConsumer &D,
// our compiler invocation set-up doesn't seem to work with it (leading
// assertions in VerifyDiagnosticConsumer).
CI->getDiagnosticOpts().VerifyDiagnostics = false;
- CI->getDiagnosticOpts().ShowColors = false;
// Disable any dependency outputting, we don't want to generate files or write
// to stdout/stderr.
@@ -91,12 +90,6 @@ buildCompilerInvocation(const ParseInputs &Inputs, clang::DiagnosticConsumer &D,
CI->getHeaderSearchOpts().ModuleFormat =
PCHContainerOperations().getRawReader().getFormat().str();
- CI->getFrontendOpts().Plugins.clear();
- CI->getFrontendOpts().AddPluginActions.clear();
- CI->getFrontendOpts().PluginArgs.clear();
- CI->getFrontendOpts().ProgramAction = frontend::ParseSyntaxOnly;
- CI->getFrontendOpts().ActionName.clear();
-
return CI;
}
diff --git a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
index f5727305b465..7fb2ae7664fa 100644
--- a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -41,9 +41,11 @@ TEST(CommandMangler, Everything) {
Mangler.ClangPath = testPath("fake/clang");
Mangler.ResourceDir = testPath("fake/resources");
Mangler.Sysroot = testPath("fake/sysroot");
- std::vector<std::string> Cmd = {"clang++", "--", "foo.cc"};
+ std::vector<std::string> Cmd = {"clang++", "-Xclang", "-load",
+ "-Xclang", "plugin", "-MF",
+ "dep", "--", "foo.cc"};
Mangler.adjust(Cmd);
- EXPECT_THAT(Cmd, ElementsAre(testPath("fake/clang++"),
+ EXPECT_THAT(Cmd, ElementsAre(testPath("fake/clang++"), "-fsyntax-only",
"-resource-dir=" + testPath("fake/resources"),
"-isysroot", testPath("fake/sysroot"), "--",
"foo.cc"));
@@ -67,6 +69,38 @@ TEST(CommandMangler, Sysroot) {
HasSubstr("-isysroot " + testPath("fake/sysroot")));
}
+TEST(CommandMangler, StripPlugins) {
+ auto Mangler = CommandMangler::forTests();
+ std::vector<std::string> Cmd = {"clang++", "-Xclang", "-load",
+ "-Xclang", "plugin", "foo.cc"};
+ Mangler.adjust(Cmd);
+ for (const char* Stripped : {"-Xclang", "-load", "plugin"})
+ EXPECT_THAT(Cmd, Not(Contains(Stripped)));
+}
+
+TEST(CommandMangler, StripOutput) {
+ auto Mangler = CommandMangler::forTests();
+ std::vector<std::string> Cmd = {"clang++", "-MF", "dependency", "-c",
+ "foo.cc"};
+ Mangler.adjust(Cmd);
+ for (const char* Stripped : {"-MF", "dependency"})
+ EXPECT_THAT(Cmd, Not(Contains(Stripped)));
+}
+
+TEST(CommandMangler, StripShowIncludes) {
+ auto Mangler = CommandMangler::forTests();
+ std::vector<std::string> Cmd = {"clang-cl", "/showIncludes", "foo.cc"};
+ Mangler.adjust(Cmd);
+ EXPECT_THAT(Cmd, Not(Contains("/showIncludes")));
+}
+
+TEST(CommandMangler, StripShowIncludesUser) {
+ auto Mangler = CommandMangler::forTests();
+ std::vector<std::string> Cmd = {"clang-cl", "/showIncludes:user", "foo.cc"};
+ Mangler.adjust(Cmd);
+ EXPECT_THAT(Cmd, Not(Contains("/showIncludes:user")));
+}
+
TEST(CommandMangler, ClangPath) {
auto Mangler = CommandMangler::forTests();
Mangler.ClangPath = testPath("fake/clang");
@@ -171,7 +205,7 @@ TEST(CommandMangler, ConfigEdits) {
Mangler.adjust(Cmd);
}
// Edits are applied in given order and before other mangling.
- EXPECT_THAT(Cmd, ElementsAre(_, "FOO.CC", "--hello"));
+ EXPECT_THAT(Cmd, ElementsAre(_, "FOO.CC", "--hello", "-fsyntax-only"));
}
static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) {
diff --git a/clang-tools-extra/clangd/unittests/CompilerTests.cpp b/clang-tools-extra/clangd/unittests/CompilerTests.cpp
index 7350422b6801..45e9bcd9d534 100644
--- a/clang-tools-extra/clangd/unittests/CompilerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CompilerTests.cpp
@@ -8,8 +8,6 @@
#include "Compiler.h"
#include "TestTU.h"
-#include "clang/Frontend/DependencyOutputOptions.h"
-#include "clang/Frontend/FrontendOptions.h"
#include "clang/Lex/PreprocessorOptions.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -58,50 +56,6 @@ TEST(BuildCompilerInvocation, PragmaDebugCrash) {
TU.build(); // no-crash
}
-TEST(BuildCompilerInvocation, DropsShowIncludes) {
- MockFS FS;
- IgnoreDiagnostics Diags;
- TestTU TU;
-
- TU.ExtraArgs = {"-Xclang", "--show-includes"};
- EXPECT_THAT(buildCompilerInvocation(TU.inputs(FS), Diags)
- ->getDependencyOutputOpts()
- .ShowIncludesDest,
- ShowIncludesDestination::None);
-
- TU.ExtraArgs = {"/showIncludes", "--driver-mode=cl"};
- EXPECT_THAT(buildCompilerInvocation(TU.inputs(FS), Diags)
- ->getDependencyOutputOpts()
- .ShowIncludesDest,
- ShowIncludesDestination::None);
-
- TU.ExtraArgs = {"/showIncludes:user", "--driver-mode=cl"};
- EXPECT_THAT(buildCompilerInvocation(TU.inputs(FS), Diags)
- ->getDependencyOutputOpts()
- .ShowIncludesDest,
- ShowIncludesDestination::None);
-}
-
-TEST(BuildCompilerInvocation, DropsPlugins) {
- MockFS FS;
- IgnoreDiagnostics Diags;
- TestTU TU;
-
- TU.ExtraArgs = {"-Xclang", "-load",
- "-Xclang", "plugins.so",
- "-Xclang", "-plugin",
- "-Xclang", "my_plugin",
- "-Xclang", "-plugin-arg-my_plugin",
- "-Xclang", "foo=bar",
- "-Xclang", "-add-plugin",
- "-Xclang", "my_plugin2"};
- auto &Opts = buildCompilerInvocation(TU.inputs(FS), Diags)->getFrontendOpts();
- EXPECT_THAT(Opts.Plugins, IsEmpty());
- EXPECT_THAT(Opts.PluginArgs, IsEmpty());
- EXPECT_THAT(Opts.AddPluginActions, IsEmpty());
- EXPECT_EQ(Opts.ProgramAction, frontend::ActionKind::ParseSyntaxOnly);
- EXPECT_TRUE(Opts.ActionName.empty());
-}
} // namespace
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list