Author: Dmitry Polukhin Date: 2023-03-13T07:00:56-07:00 New Revision: 2a84c53ccdc015a7f53a144aa4f7c0dddf839604
URL: https://github.com/llvm/llvm-project/commit/2a84c53ccdc015a7f53a144aa4f7c0dddf839604 DIFF: https://github.com/llvm/llvm-project/commit/2a84c53ccdc015a7f53a144aa4f7c0dddf839604.diff LOG: Revert "[clangd] Move standard options adaptor to CommandMangler" This reverts commit 34de7da6246cdfa6ff6f3d3c514583cddc0a10ec. 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/include/clang/Tooling/Tooling.h clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp clang/lib/Tooling/Tooling.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp index 39b180e002a68..bcd39b2d38ba5 100644 --- a/clang-tools-extra/clangd/CompileCommands.cpp +++ b/clang-tools-extra/clangd/CompileCommands.cpp @@ -14,7 +14,6 @@ #include "clang/Driver/Options.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Tooling/CompilationDatabase.h" -#include "clang/Tooling/Tooling.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" @@ -28,7 +27,6 @@ #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Path.h" #include "llvm/Support/Program.h" -#include "llvm/TargetParser/Host.h" #include <iterator> #include <optional> #include <string> @@ -187,12 +185,6 @@ static std::string resolveDriver(llvm::StringRef Driver, bool FollowSymlink, } // namespace -CommandMangler::CommandMangler() { - Tokenizer = llvm::Triple(llvm::sys::getProcessTriple()).isOSWindows() - ? llvm::cl::TokenizeWindowsCommandLine - : llvm::cl::TokenizeGNUCommandLine; -} - CommandMangler CommandMangler::detect() { CommandMangler Result; Result.ClangPath = detectClangPath(); @@ -209,18 +201,9 @@ void CommandMangler::operator()(tooling::CompileCommand &Command, trace::Span S("AdjustCompileFlags"); // Most of the modifications below assumes the Cmd starts with a driver name. // We might consider injecting a generic driver name like "cc" or "c++", but - // a Cmd missing the driver is probably rare enough in practice and erroneous. + // a Cmd missing the driver is probably rare enough in practice and errnous. if (Cmd.empty()) return; - - // FS used for expanding response files. - // FIXME: ExpandResponseFiles appears not to provide the usual - // thread-safety guarantees, as the access to FS is not locked! - // For now, use the real FS, which is known to be threadsafe (if we don't - // use/change working directory, which ExpandResponseFiles doesn't). - auto FS = llvm::vfs::getRealFileSystem(); - tooling::addExpandedResponseFiles(Cmd, Command.Directory, Tokenizer, *FS); - auto &OptTable = clang::driver::getDriverOptTable(); // OriginalArgs needs to outlive ArgList. llvm::SmallVector<const char *, 16> OriginalArgs; @@ -229,7 +212,7 @@ void CommandMangler::operator()(tooling::CompileCommand &Command, OriginalArgs.push_back(S.c_str()); bool IsCLMode = driver::IsClangCL(driver::getDriverMode( OriginalArgs[0], llvm::ArrayRef(OriginalArgs).slice(1))); - // ParseArgs propagates missing arg/opt counts on error, but preserves + // 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 @@ -324,16 +307,12 @@ void CommandMangler::operator()(tooling::CompileCommand &Command, // necessary for the system include extractor to identify the file type // - AFTER applying CompileFlags.Edits, because the name of the compiler // that needs to be invoked may come from the CompileFlags->Compiler key - // - BEFORE addTargetAndModeForProgramName(), because gcc doesn't support - // the target flag that might be added. // - BEFORE resolveDriver() because that can mess up the driver path, // e.g. changing gcc to /path/to/clang/bin/gcc if (SystemIncludeExtractor) { SystemIncludeExtractor(Command, File); } - tooling::addTargetAndModeForProgramName(Cmd, Cmd.front()); - // Check whether the flag exists, either as -flag or -flag=* auto Has = [&](llvm::StringRef Flag) { for (llvm::StringRef Arg : Cmd) { diff --git a/clang-tools-extra/clangd/CompileCommands.h b/clang-tools-extra/clangd/CompileCommands.h index 1b37f44f0b9db..eb318d18baf63 100644 --- a/clang-tools-extra/clangd/CompileCommands.h +++ b/clang-tools-extra/clangd/CompileCommands.h @@ -12,7 +12,6 @@ #include "support/Threading.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" -#include "llvm/Support/CommandLine.h" #include <deque> #include <optional> #include <string> @@ -51,11 +50,10 @@ struct CommandMangler { llvm::StringRef TargetFile) const; private: - CommandMangler(); + CommandMangler() = default; Memoize<llvm::StringMap<std::string>> ResolvedDrivers; Memoize<llvm::StringMap<std::string>> ResolvedDriversNoFollow; - llvm::cl::TokenizerCallback Tokenizer; }; // Removes args from a command-line in a semantically-aware way. diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp index d1833759917a3..def43683e0ab7 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp @@ -244,7 +244,15 @@ static std::unique_ptr<tooling::CompilationDatabase> parseJSON(PathRef Path, llvm::StringRef Data, std::string &Error) { if (auto CDB = tooling::JSONCompilationDatabase::loadFromBuffer( Data, Error, tooling::JSONCommandLineSyntax::AutoDetect)) { - return tooling::inferMissingCompileCommands(std::move(CDB)); + // FS used for expanding response files. + // FIXME: ExpandResponseFilesDatabase appears not to provide the usual + // thread-safety guarantees, as the access to FS is not locked! + // For now, use the real FS, which is known to be threadsafe (if we don't + // use/change working directory, which ExpandResponseFilesDatabase doesn't). + auto FS = llvm::vfs::getRealFileSystem(); + return tooling::inferTargetAndDriverMode( + tooling::inferMissingCompileCommands( + expandResponseFiles(std::move(CDB), std::move(FS)))); } return nullptr; } 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 edda1e1f16787..08c7b4bcb57ad 100644 --- a/clang-tools-extra/clangd/test/did-change-configuration-params.test +++ b/clang-tools-extra/clangd/test/did-change-configuration-params.test @@ -45,17 +45,13 @@ # CHECK-NEXT: "uri": "file://{{.*}}/foo.c", # CHECK-NEXT: "version": 0 # CHECK-NEXT: } ---- -{"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabaseChanges":{"/clangd-test/foo.c": {"workingDirectory":"/clangd-test2", "compilationCommand": ["x86_64-linux-unknown-gcc", "-c", "foo.c", "-Wall", "-Werror"]}}}}} -# CHECK: "method": "textDocument/publishDiagnostics", # # ERR: ASTWorker building file {{.*}}foo.c version 0 with command # ERR: [{{.*}}clangd-test2] # ERR: clang -c -Wall -Werror {{.*}} -- {{.*}}foo.c -# ERR: ASTWorker building file {{.*}}foo.c version 0 with command -# ERR: [{{.*}}clangd-test2] -# ERR: x86_64-linux-unknown-gcc --target=x86_64-linux-unknown -c -Wall -Werror {{.*}} -- {{.*}}foo.c --- {"jsonrpc":"2.0","id":5,"method":"shutdown"} --- {"jsonrpc":"2.0","method":"exit"} + + diff --git a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp index acb1044287a59..be44ce59f6657 100644 --- a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp +++ b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp @@ -20,7 +20,6 @@ #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" #include "llvm/Support/Process.h" -#include "llvm/Support/TargetSelect.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -42,17 +41,15 @@ using ::testing::Not; // Make use of all features and assert the exact command we get out. // Other tests just verify presence/absence of certain args. TEST(CommandMangler, Everything) { - llvm::InitializeAllTargetInfos(); // As in ClangdMain auto Mangler = CommandMangler::forTests(); Mangler.ClangPath = testPath("fake/clang"); Mangler.ResourceDir = testPath("fake/resources"); Mangler.Sysroot = testPath("fake/sysroot"); tooling::CompileCommand Cmd; - Cmd.CommandLine = {"x86_64-linux-unknown-clang++", "--", "foo.cc", "bar.cc"}; + Cmd.CommandLine = {"clang++", "--", "foo.cc", "bar.cc"}; Mangler(Cmd, "foo.cc"); EXPECT_THAT(Cmd.CommandLine, - ElementsAre(testPath("fake/x86_64-linux-unknown-clang++"), - "--target=x86_64-linux-unknown", "--driver-mode=g++", + ElementsAre(testPath("fake/clang++"), "-resource-dir=" + testPath("fake/resources"), "-isysroot", testPath("fake/sysroot"), "--", "foo.cc")); @@ -200,26 +197,8 @@ TEST(CommandMangler, ConfigEdits) { Mangler(Cmd, "foo.cc"); } // Edits are applied in given order and before other mangling and they always - // go before filename. `--driver-mode=g++` here is in lower case because - // options inserted by addTargetAndModeForProgramName are not editable, - // see discussion in https://reviews.llvm.org/D138546 - EXPECT_THAT(Cmd.CommandLine, - ElementsAre(_, "--driver-mode=g++", "--hello", "--", "FOO.CC")); -} - -TEST(CommandMangler, ExpandedResponseFiles) { - SmallString<1024> Path; - int FD; - ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("args", "", FD, Path)); - llvm::raw_fd_ostream OutStream(FD, true); - OutStream << "-Wall"; - OutStream.close(); - - auto Mangler = CommandMangler::forTests(); - tooling::CompileCommand Cmd; - Cmd.CommandLine = {"clang", ("@" + Path).str(), "foo.cc"}; - Mangler(Cmd, "foo.cc"); - EXPECT_THAT(Cmd.CommandLine, ElementsAre(_, "-Wall", "--", "foo.cc")); + // go before filename. + EXPECT_THAT(Cmd.CommandLine, ElementsAre(_, "--hello", "--", "FOO.CC")); } static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) { diff --git a/clang/include/clang/Tooling/Tooling.h b/clang/include/clang/Tooling/Tooling.h index 7a1c62e3a3d57..52a81cd9e7787 100644 --- a/clang/include/clang/Tooling/Tooling.h +++ b/clang/include/clang/Tooling/Tooling.h @@ -506,12 +506,6 @@ llvm::Expected<std::string> getAbsolutePath(llvm::vfs::FileSystem &FS, void addTargetAndModeForProgramName(std::vector<std::string> &CommandLine, StringRef InvokedAs); -/// Helper function that expands response files in command line. -void addExpandedResponseFiles(std::vector<std::string> &CommandLine, - llvm::StringRef WorkingDir, - llvm::cl::TokenizerCallback Tokenizer, - llvm::vfs::FileSystem &FS); - /// Creates a \c CompilerInvocation. CompilerInvocation *newInvocation(DiagnosticsEngine *Diagnostics, ArrayRef<const char *> CC1Args, diff --git a/clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp b/clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp index ebf8aa2a7628c..dd9b9c992d846 100644 --- a/clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp +++ b/clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp @@ -7,7 +7,6 @@ //===----------------------------------------------------------------------===// #include "clang/Tooling/CompilationDatabase.h" -#include "clang/Tooling/Tooling.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/ConvertUTF.h" @@ -49,9 +48,28 @@ class ExpandResponseFilesDatabase : public CompilationDatabase { private: std::vector<CompileCommand> expand(std::vector<CompileCommand> Cmds) const { - for (auto &Cmd : Cmds) - tooling::addExpandedResponseFiles(Cmd.CommandLine, Cmd.Directory, - Tokenizer, *FS); + for (auto &Cmd : Cmds) { + bool SeenRSPFile = false; + llvm::SmallVector<const char *, 20> Argv; + Argv.reserve(Cmd.CommandLine.size()); + for (auto &Arg : Cmd.CommandLine) { + Argv.push_back(Arg.c_str()); + if (!Arg.empty()) + SeenRSPFile |= Arg.front() == '@'; + } + if (!SeenRSPFile) + continue; + llvm::BumpPtrAllocator Alloc; + llvm::cl::ExpansionContext ECtx(Alloc, Tokenizer); + llvm::Error Err = ECtx.setVFS(FS.get()) + .setCurrentDir(Cmd.Directory) + .expandResponseFiles(Argv); + if (Err) + llvm::errs() << Err; + // Don't assign directly, Argv aliases CommandLine. + std::vector<std::string> ExpandedArgv(Argv.begin(), Argv.end()); + Cmd.CommandLine = std::move(ExpandedArgv); + } return Cmds; } diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp index dd22cfedd0ffe..3048f69eef251 100644 --- a/clang/lib/Tooling/Tooling.cpp +++ b/clang/lib/Tooling/Tooling.cpp @@ -43,7 +43,6 @@ #include "llvm/Option/OptTable.h" #include "llvm/Option/Option.h" #include "llvm/Support/Casting.h" -#include "llvm/Support/CommandLine.h" #include "llvm/Support/Debug.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FileSystem.h" @@ -300,31 +299,6 @@ void addTargetAndModeForProgramName(std::vector<std::string> &CommandLine, } } -void addExpandedResponseFiles(std::vector<std::string> &CommandLine, - llvm::StringRef WorkingDir, - llvm::cl::TokenizerCallback Tokenizer, - llvm::vfs::FileSystem &FS) { - bool SeenRSPFile = false; - llvm::SmallVector<const char *, 20> Argv; - Argv.reserve(CommandLine.size()); - for (auto &Arg : CommandLine) { - Argv.push_back(Arg.c_str()); - if (!Arg.empty()) - SeenRSPFile |= Arg.front() == '@'; - } - if (!SeenRSPFile) - return; - llvm::BumpPtrAllocator Alloc; - llvm::cl::ExpansionContext ECtx(Alloc, Tokenizer); - llvm::Error Err = - ECtx.setVFS(&FS).setCurrentDir(WorkingDir).expandResponseFiles(Argv); - if (Err) - llvm::errs() << Err; - // Don't assign directly, Argv aliases CommandLine. - std::vector<std::string> ExpandedArgv(Argv.begin(), Argv.end()); - CommandLine = std::move(ExpandedArgv); -} - } // namespace tooling } // namespace clang @@ -710,7 +684,7 @@ std::unique_ptr<ASTUnit> buildASTFromCodeWithArgs( if (!Invocation.run()) return nullptr; - + assert(ASTs.size() == 1); return std::move(ASTs[0]); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits