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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
