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