Author: Kadir Cetinkaya Date: 2021-07-27T14:49:53+02:00 New Revision: 259e365deaa3a6920b30f49e3d03d3508f1d4900
URL: https://github.com/llvm/llvm-project/commit/259e365deaa3a6920b30f49e3d03d3508f1d4900 DIFF: https://github.com/llvm/llvm-project/commit/259e365deaa3a6920b30f49e3d03d3508f1d4900.diff LOG: Revert "Revert "[clangd] Adjust compile flags to contain only the requested file as input"" This reverts commit 04e21fbc44c145d5599ef8db9aaf66b159107f33. Added: Modified: clang-tools-extra/clangd/CompileCommands.cpp clang-tools-extra/clangd/CompileCommands.h clang-tools-extra/clangd/GlobalCompilationDatabase.cpp clang-tools-extra/clangd/test/did-change-configuration-params.test clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp clang-tools-extra/clangd/unittests/TestTU.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp index ec0e2160abbad..032bf7354d770 100644 --- a/clang-tools-extra/clangd/CompileCommands.cpp +++ b/clang-tools-extra/clangd/CompileCommands.cpp @@ -196,7 +196,8 @@ CommandMangler CommandMangler::detect() { CommandMangler CommandMangler::forTests() { return CommandMangler(); } -void CommandMangler::adjust(std::vector<std::string> &Cmd) const { +void CommandMangler::adjust(std::vector<std::string> &Cmd, + llvm::StringRef File) const { trace::Span S("AdjustCompileFlags"); auto &OptTable = clang::driver::getDriverOptTable(); // OriginalArgs needs to outlive ArgList. @@ -211,8 +212,10 @@ void CommandMangler::adjust(std::vector<std::string> &Cmd) const { // ParseArgs propagates missig arg/opt counts on error, but preserves // everything it could parse in ArgList. So we just ignore those counts. unsigned IgnoredCount; + // Drop the executable name, as ParseArgs doesn't expect it. This means + // indices are actually of by one between ArgList and OriginalArgs. auto ArgList = OptTable.ParseArgs( - OriginalArgs, IgnoredCount, IgnoredCount, + llvm::makeArrayRef(OriginalArgs).drop_front(), IgnoredCount, IgnoredCount, /*FlagsToInclude=*/ IsCLMode ? (driver::options::CLOption | driver::options::CoreOption) : /*everything*/ 0, @@ -223,12 +226,17 @@ void CommandMangler::adjust(std::vector<std::string> &Cmd) const { // 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()); - } + // Drop all the inputs and only add one for the current file. + llvm::SmallVector<unsigned, 1> IndicesToDrop; + for (auto *Input : ArgList.filtered(driver::options::OPT_INPUT)) + IndicesToDrop.push_back(Input->getIndex()); + llvm::sort(IndicesToDrop); + llvm::for_each(llvm::reverse(IndicesToDrop), + // +1 to account for the executable name in Cmd[0] that + // doesn't exist in ArgList. + [&Cmd](unsigned Idx) { Cmd.erase(Cmd.begin() + Idx + 1); }); + Cmd.push_back("--"); + Cmd.push_back(File.str()); } for (auto &Edit : Config::current().CompileFlags.Edits) @@ -278,7 +286,7 @@ CommandMangler::operator clang::tooling::ArgumentsAdjuster() && { return [Mangler = std::make_shared<CommandMangler>(std::move(*this))]( const std::vector<std::string> &Args, llvm::StringRef File) { auto Result = Args; - Mangler->adjust(Result); + Mangler->adjust(Result, File); return Result; }; } diff --git a/clang-tools-extra/clangd/CompileCommands.h b/clang-tools-extra/clangd/CompileCommands.h index 759472413fdaf..50c1a3571ec42 100644 --- a/clang-tools-extra/clangd/CompileCommands.h +++ b/clang-tools-extra/clangd/CompileCommands.h @@ -12,6 +12,7 @@ #include "clang/Tooling/ArgumentsAdjusters.h" #include "clang/Tooling/CompilationDatabase.h" #include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringRef.h" #include <deque> #include <string> #include <vector> @@ -46,7 +47,7 @@ struct CommandMangler { // - on mac, find clang and isysroot by querying the `xcrun` launcher static CommandMangler detect(); - void adjust(std::vector<std::string> &Cmd) const; + void adjust(std::vector<std::string> &Cmd, llvm::StringRef File) const; explicit operator clang::tooling::ArgumentsAdjuster() &&; private: diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp index cfc46131496d1..85b0826e6d8b0 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp @@ -763,7 +763,7 @@ OverlayCDB::getCompileCommand(PathRef File) const { if (!Cmd) return llvm::None; if (ArgsAdjuster) - Cmd->CommandLine = ArgsAdjuster(Cmd->CommandLine, Cmd->Filename); + Cmd->CommandLine = ArgsAdjuster(Cmd->CommandLine, File); return Cmd; } @@ -773,7 +773,7 @@ tooling::CompileCommand OverlayCDB::getFallbackCommand(PathRef File) const { Cmd.CommandLine.insert(Cmd.CommandLine.end(), FallbackFlags.begin(), FallbackFlags.end()); if (ArgsAdjuster) - Cmd.CommandLine = ArgsAdjuster(Cmd.CommandLine, Cmd.Filename); + Cmd.CommandLine = ArgsAdjuster(Cmd.CommandLine, File); return 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 e90e26c0d3a7a..29cf1786a835a 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 -Wall -Werror {{.*}} -- foo.c +# ERR: clang -c -Wall -Werror {{.*}} -- {{.*}}foo.c --- {"jsonrpc":"2.0","id":5,"method":"shutdown"} --- diff --git a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp index 15b5dddadd262..175ceb4552740 100644 --- a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp +++ b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp @@ -44,7 +44,7 @@ TEST(CommandMangler, Everything) { Mangler.ResourceDir = testPath("fake/resources"); Mangler.Sysroot = testPath("fake/sysroot"); std::vector<std::string> Cmd = {"clang++", "--", "foo.cc"}; - Mangler.adjust(Cmd); + Mangler.adjust(Cmd, "foo.cc"); EXPECT_THAT(Cmd, ElementsAre(testPath("fake/clang++"), "-resource-dir=" + testPath("fake/resources"), "-isysroot", testPath("fake/sysroot"), "--", @@ -55,7 +55,7 @@ TEST(CommandMangler, ResourceDir) { auto Mangler = CommandMangler::forTests(); Mangler.ResourceDir = testPath("fake/resources"); std::vector<std::string> Cmd = {"clang++", "foo.cc"}; - Mangler.adjust(Cmd); + Mangler.adjust(Cmd, "foo.cc"); EXPECT_THAT(Cmd, Contains("-resource-dir=" + testPath("fake/resources"))); } @@ -64,7 +64,7 @@ TEST(CommandMangler, Sysroot) { Mangler.Sysroot = testPath("fake/sysroot"); std::vector<std::string> Cmd = {"clang++", "foo.cc"}; - Mangler.adjust(Cmd); + Mangler.adjust(Cmd, "foo.cc"); EXPECT_THAT(llvm::join(Cmd, " "), HasSubstr("-isysroot " + testPath("fake/sysroot"))); } @@ -74,19 +74,19 @@ TEST(CommandMangler, ClangPath) { Mangler.ClangPath = testPath("fake/clang"); std::vector<std::string> Cmd = {"clang++", "foo.cc"}; - Mangler.adjust(Cmd); + Mangler.adjust(Cmd, "foo.cc"); EXPECT_EQ(testPath("fake/clang++"), Cmd.front()); Cmd = {"unknown-binary", "foo.cc"}; - Mangler.adjust(Cmd); + Mangler.adjust(Cmd, "foo.cc"); EXPECT_EQ(testPath("fake/unknown-binary"), Cmd.front()); Cmd = {testPath("path/clang++"), "foo.cc"}; - Mangler.adjust(Cmd); + Mangler.adjust(Cmd, "foo.cc"); EXPECT_EQ(testPath("path/clang++"), Cmd.front()); Cmd = {"foo/unknown-binary", "foo.cc"}; - Mangler.adjust(Cmd); + Mangler.adjust(Cmd, "foo.cc"); EXPECT_EQ("foo/unknown-binary", Cmd.front()); } @@ -129,7 +129,7 @@ TEST(CommandMangler, ClangPathResolve) { auto Mangler = CommandMangler::forTests(); Mangler.ClangPath = testPath("fake/clang"); std::vector<std::string> Cmd = {(TempDir + "/bin/foo").str(), "foo.cc"}; - Mangler.adjust(Cmd); + Mangler.adjust(Cmd, "foo.cc"); // Directory based on resolved symlink, basename preserved. EXPECT_EQ((TempDir + "/lib/foo").str(), Cmd.front()); @@ -146,13 +146,13 @@ TEST(CommandMangler, ClangPathResolve) { Mangler.ClangPath = testPath("fake/clang"); // Driver found on PATH. Cmd = {"foo", "foo.cc"}; - Mangler.adjust(Cmd); + Mangler.adjust(Cmd, "foo.cc"); // Found the symlink and resolved the path as above. EXPECT_EQ((TempDir + "/lib/foo").str(), Cmd.front()); // Symlink not resolved with -no-canonical-prefixes. Cmd = {"foo", "-no-canonical-prefixes", "foo.cc"}; - Mangler.adjust(Cmd); + Mangler.adjust(Cmd, "foo.cc"); EXPECT_EQ((TempDir + "/bin/foo").str(), Cmd.front()); } #endif @@ -171,7 +171,7 @@ TEST(CommandMangler, ConfigEdits) { Argv = tooling::getInsertArgumentAdjuster("--hello")(Argv, ""); }); WithContextValue WithConfig(Config::Key, std::move(Cfg)); - Mangler.adjust(Cmd); + Mangler.adjust(Cmd, "foo.cc"); } // Edits are applied in given order and before other mangling and they always // go before filename. @@ -350,7 +350,7 @@ TEST(CommandMangler, InputsAfterDashDash) { const auto Mangler = CommandMangler::forTests(); { std::vector<std::string> Args = {"clang", "/Users/foo.cc"}; - Mangler.adjust(Args); + Mangler.adjust(Args, "/Users/foo.cc"); EXPECT_THAT(llvm::makeArrayRef(Args).take_back(2), ElementsAre("--", "/Users/foo.cc")); EXPECT_THAT(llvm::makeArrayRef(Args).drop_back(2), @@ -361,11 +361,21 @@ TEST(CommandMangler, InputsAfterDashDash) { { std::vector<std::string> Args = {"clang", "--driver-mode=cl", "bar.cc", "/Users/foo.cc"}; - Mangler.adjust(Args); + Mangler.adjust(Args, "bar.cc"); EXPECT_THAT(llvm::makeArrayRef(Args).take_back(2), ElementsAre("--", "bar.cc")); EXPECT_THAT(llvm::makeArrayRef(Args).drop_back(2), Not(Contains("bar.cc"))); } + // All inputs but the main file is dropped. + { + std::vector<std::string> Args = {"clang", "foo.cc", "bar.cc"}; + Mangler.adjust(Args, "baz.cc"); + EXPECT_THAT(llvm::makeArrayRef(Args).take_back(2), + ElementsAre("--", "baz.cc")); + EXPECT_THAT( + llvm::makeArrayRef(Args).drop_back(2), + testing::AllOf(Not(Contains("foo.cc")), Not(Contains("bar.cc")))); + } } } // namespace } // namespace clangd diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp index 052603ce6851a..a848a50730caa 100644 --- a/clang-tools-extra/clangd/unittests/TestTU.cpp +++ b/clang-tools-extra/clangd/unittests/TestTU.cpp @@ -59,7 +59,7 @@ ParseInputs TestTU::inputs(MockFS &FS) const { Argv.push_back(FullFilename); auto Mangler = CommandMangler::forTests(); - Mangler.adjust(Inputs.CompileCommand.CommandLine); + Mangler.adjust(Inputs.CompileCommand.CommandLine, FullFilename); Inputs.CompileCommand.Filename = FullFilename; Inputs.CompileCommand.Directory = testRoot(); Inputs.Contents = Code; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits