DmitryPolukhin created this revision. DmitryPolukhin added reviewers: sammccall, ilya-biryukov. DmitryPolukhin added projects: clang, clang-tools-extra. Herald added subscribers: s.egerton, kadircet, arphaman, simoncook. Herald added a project: All. DmitryPolukhin requested review of this revision. Herald added subscribers: pcwang-thead, MaskRay.
There is a discrepancy between how clangd processes CDB loaded from JSON file on disk and pushed via LSP. This the same CDB pushed via LSP protocol may not work as expected. I think there is no fundamental difference between these CDB loaded from JSON and pushed via LSP. This diff extracts adaptors application into a separate function and calls it in both cases. Test Plan: check-clang-tools Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D143436 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp clang-tools-extra/clangd/GlobalCompilationDatabase.cpp clang-tools-extra/clangd/GlobalCompilationDatabase.h clang-tools-extra/clangd/test/did-change-configuration-params.test clang/include/clang/Tooling/CompilationDatabase.h clang/lib/Tooling/CompilationDatabase.cpp
Index: clang/lib/Tooling/CompilationDatabase.cpp =================================================================== --- clang/lib/Tooling/CompilationDatabase.cpp +++ clang/lib/Tooling/CompilationDatabase.cpp @@ -378,6 +378,10 @@ StringRef()); } +FixedCompilationDatabase::FixedCompilationDatabase(CompileCommand&& CommandLine) { + CompileCommands.emplace_back(CommandLine); +} + std::vector<CompileCommand> FixedCompilationDatabase::getCompileCommands(StringRef FilePath) const { std::vector<CompileCommand> Result(CompileCommands); Index: clang/include/clang/Tooling/CompilationDatabase.h =================================================================== --- clang/include/clang/Tooling/CompilationDatabase.h +++ clang/include/clang/Tooling/CompilationDatabase.h @@ -199,6 +199,9 @@ FixedCompilationDatabase(const Twine &Directory, ArrayRef<std::string> CommandLine); + /// Constructs a compilation data base from passed CompileCommand. + FixedCompilationDatabase(CompileCommand&& CommandLine); + /// Returns the given compile command. /// /// Will always return a vector with one entry that contains the directory Index: clang-tools-extra/clangd/test/did-change-configuration-params.test =================================================================== --- clang-tools-extra/clangd/test/did-change-configuration-params.test +++ clang-tools-extra/clangd/test/did-change-configuration-params.test @@ -45,13 +45,17 @@ # 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": ["riscv64-unknown-elf-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: riscv64-unknown-elf-gcc --target=riscv64-unknown-elf -c -Wall -Werror {{.*}} -- {{.*}}foo.c --- {"jsonrpc":"2.0","id":5,"method":"shutdown"} --- {"jsonrpc":"2.0","method":"exit"} - - Index: clang-tools-extra/clangd/GlobalCompilationDatabase.h =================================================================== --- clang-tools-extra/clangd/GlobalCompilationDatabase.h +++ clang-tools-extra/clangd/GlobalCompilationDatabase.h @@ -169,6 +169,11 @@ SystemIncludeExtractorFn getSystemIncludeExtractor(llvm::ArrayRef<std::string> QueryDriverGlobs); +/// Creates standard set of CDB adaptors arround passed CDB. +std::unique_ptr<tooling::CompilationDatabase> +wrapCDBInExpandingAndInferingAdaptors( + std::unique_ptr<tooling::CompilationDatabase> &&CDB); + /// Wraps another compilation database, and supports overriding the commands /// using an in-memory mapping. class OverlayCDB : public DelegatingCDB { Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp =================================================================== --- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp @@ -239,20 +239,25 @@ return {LoadResult::FoundNewData, std::move(*Buf)}; } +std::unique_ptr<tooling::CompilationDatabase> +wrapCDBInExpandingAndInferingAdaptors( + std::unique_ptr<tooling::CompilationDatabase> &&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)))); +} + // Adapt CDB-loading functions to a common interface for DirectoryCache::load(). 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)) { - // 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 wrapCDBInExpandingAndInferingAdaptors(std::move(CDB)); } return nullptr; } Index: clang-tools-extra/clangd/ClangdLSPServer.cpp =================================================================== --- clang-tools-extra/clangd/ClangdLSPServer.cpp +++ clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -1342,6 +1342,11 @@ tooling::CompileCommand(std::move(Entry.second.workingDirectory), File, std::move(Entry.second.compilationCommand), /*Output=*/""); + // Apply standard adaptors to the compile commands passed via LSP protocol. + New = std::move( + wrapCDBInExpandingAndInferingAdaptors( + std::make_unique<tooling::FixedCompilationDatabase>(std::move(New))) + ->getCompileCommands(File)[0]); if (Old != New) { CDB->setCompileCommand(File, std::move(New)); ModifiedFiles.insert(File);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits