From 32010ae7e0a47cd4a70a9401980b32ed1d3e10f6 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Fri, 15 Sep 2023 11:33:53 +0800 Subject: [PATCH 1/6] [clangd] [C++20] [Modules] Introduce initial support for C++20 Modules Alternatives to https://reviews.llvm.org/D153114. Try to address https://github.com/clangd/clangd/issues/1293. See the links for design ideas. We want to have some initial support in clang18. This is the initial support for C++20 Modules in clangd. As suggested by sammccall in https://reviews.llvm.org/D153114, we should minimize the scope of the initial patch to make it easier to review and understand so that every one are in the same page: > Don't attempt any cross-file or cross-version coordination: i.e. don't > try to reuse BMIs between different files, don't try to reuse BMIs > between (preamble) reparses of the same file, don't try to persist the > module graph. Instead, when building a preamble, synchronously scan > for the module graph, build the required PCMs on the single preamble > thread with filenames private to that preamble, and then proceed to > build the preamble. And this patch reflects the above opinions. --- clang-tools-extra/clangd/CMakeLists.txt | 4 + clang-tools-extra/clangd/ClangdServer.cpp | 2 + clang-tools-extra/clangd/ClangdServer.h | 3 + .../clangd/GlobalCompilationDatabase.cpp | 23 ++ .../clangd/GlobalCompilationDatabase.h | 14 + .../clangd/ModuleDependencyScanner.cpp | 81 +++++ .../clangd/ModuleDependencyScanner.h | 106 ++++++ clang-tools-extra/clangd/ModulesBuilder.cpp | 339 ++++++++++++++++++ clang-tools-extra/clangd/ModulesBuilder.h | 71 ++++ clang-tools-extra/clangd/ParsedAST.cpp | 7 + clang-tools-extra/clangd/Preamble.cpp | 28 +- clang-tools-extra/clangd/Preamble.h | 10 + .../clangd/PrerequisiteModules.h | 87 +++++ clang-tools-extra/clangd/ProjectModules.cpp | 62 ++++ clang-tools-extra/clangd/ProjectModules.h | 55 +++ clang-tools-extra/clangd/TUScheduler.cpp | 50 ++- clang-tools-extra/clangd/TUScheduler.h | 7 + clang-tools-extra/clangd/test/CMakeLists.txt | 1 + clang-tools-extra/clangd/test/modules.test | 83 +++++ clang-tools-extra/clangd/tool/Check.cpp | 13 +- clang-tools-extra/clangd/tool/ClangdMain.cpp | 8 + .../clangd/unittests/CMakeLists.txt | 2 + .../clangd/unittests/CodeCompleteTests.cpp | 22 +- .../clangd/unittests/FileIndexTests.cpp | 4 +- .../unittests/ModuleDependencyScannerTest.cpp | 176 +++++++++ .../clangd/unittests/ModulesTestSetup.h | 103 ++++++ .../clangd/unittests/ParsedASTTests.cpp | 8 +- .../clangd/unittests/PreambleTests.cpp | 6 +- .../unittests/PrerequisiteModulesTest.cpp | 224 ++++++++++++ clang-tools-extra/clangd/unittests/TestTU.cpp | 14 +- clang-tools-extra/docs/ReleaseNotes.rst | 3 + 31 files changed, 1576 insertions(+), 40 deletions(-) create mode 100644 clang-tools-extra/clangd/ModuleDependencyScanner.cpp create mode 100644 clang-tools-extra/clangd/ModuleDependencyScanner.h create mode 100644 clang-tools-extra/clangd/ModulesBuilder.cpp create mode 100644 clang-tools-extra/clangd/ModulesBuilder.h create mode 100644 clang-tools-extra/clangd/PrerequisiteModules.h create mode 100644 clang-tools-extra/clangd/ProjectModules.cpp create mode 100644 clang-tools-extra/clangd/ProjectModules.h create mode 100644 clang-tools-extra/clangd/test/modules.test create mode 100644 clang-tools-extra/clangd/unittests/ModuleDependencyScannerTest.cpp create mode 100644 clang-tools-extra/clangd/unittests/ModulesTestSetup.h create mode 100644 clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt index 3911fb6c6c746a..242a8ad2e350be 100644 --- a/clang-tools-extra/clangd/CMakeLists.txt +++ b/clang-tools-extra/clangd/CMakeLists.txt @@ -97,7 +97,10 @@ add_clang_library(clangDaemon IncludeFixer.cpp InlayHints.cpp JSONTransport.cpp + ModuleDependencyScanner.cpp + ModulesBuilder.cpp PathMapping.cpp + ProjectModules.cpp Protocol.cpp Quality.cpp ParsedAST.cpp @@ -161,6 +164,7 @@ clang_target_link_libraries(clangDaemon clangAST clangASTMatchers clangBasic + clangDependencyScanning clangDriver clangFormat clangFrontend diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 13d788162817fb..8ba4b38c420ab6 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -202,6 +202,7 @@ ClangdServer::Options::operator TUScheduler::Options() const { Opts.UpdateDebounce = UpdateDebounce; Opts.ContextProvider = ContextProvider; Opts.PreambleThrottler = PreambleThrottler; + Opts.ExperimentalModulesSupport = ExperimentalModulesSupport; return Opts; } @@ -222,6 +223,7 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB, DirtyFS(std::make_unique(TFS, DraftMgr)) { if (Opts.AsyncThreadsCount != 0) IndexTasks.emplace(); + // Pass a callback into `WorkScheduler` to extract symbols from a newly // parsed file and rebuild the file index synchronously each time an AST // is parsed. diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index a416602251428b..dc546b118cb8f5 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -112,6 +112,9 @@ class ClangdServer { /// This throttler controls which preambles may be built at a given time. clangd::PreambleThrottler *PreambleThrottler = nullptr; + /// Enable experimental support for modules. + bool ExperimentalModulesSupport = false; + /// If true, ClangdServer builds a dynamic in-memory index for symbols in /// opened files and uses the index to augment code completion results. bool BuildDynamicSymbolIndex = false; diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp index d1833759917a30..3a1931041eb0c5 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp @@ -9,6 +9,7 @@ #include "GlobalCompilationDatabase.h" #include "Config.h" #include "FS.h" +#include "ProjectModules.h" #include "SourceCode.h" #include "support/Logger.h" #include "support/Path.h" @@ -729,6 +730,21 @@ DirectoryBasedGlobalCompilationDatabase::getProjectInfo(PathRef File) const { return Res->PI; } +std::shared_ptr +DirectoryBasedGlobalCompilationDatabase::getProjectModules(PathRef File) const { + CDBLookupRequest Req; + Req.FileName = File; + Req.ShouldBroadcast = false; + Req.FreshTime = Req.FreshTimeMissing = + std::chrono::steady_clock::time_point::min(); + auto Res = lookupCDB(Req); + if (!Res) + return {}; + return ProjectModules::create( + ProjectModules::ProjectModulesKind::ScanningAllFiles, + Res->CDB->getAllFiles(), *this, Opts.TFS); +} + OverlayCDB::OverlayCDB(const GlobalCompilationDatabase *Base, std::vector FallbackFlags, CommandMangler Mangler) @@ -805,6 +821,13 @@ std::optional DelegatingCDB::getProjectInfo(PathRef File) const { return Base->getProjectInfo(File); } +std::shared_ptr +DelegatingCDB::getProjectModules(PathRef File) const { + if (!Base) + return nullptr; + return Base->getProjectModules(File); +} + tooling::CompileCommand DelegatingCDB::getFallbackCommand(PathRef File) const { if (!Base) return GlobalCompilationDatabase::getFallbackCommand(File); diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.h b/clang-tools-extra/clangd/GlobalCompilationDatabase.h index 2bf8c973c534c6..1bd6324c4cd8e4 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.h +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.h @@ -25,6 +25,8 @@ namespace clang { namespace clangd { +class ProjectModules; + struct ProjectInfo { // The directory in which the compilation database was discovered. // Empty if directory-based compilation database discovery was not used. @@ -45,6 +47,12 @@ class GlobalCompilationDatabase { return std::nullopt; } + /// Get the modules in the closest project to \p File + virtual std::shared_ptr + getProjectModules(PathRef File) const { + return nullptr; + } + /// Makes a guess at how to build a file. /// The default implementation just runs clang on the file. /// Clangd should treat the results as unreliable. @@ -76,6 +84,9 @@ class DelegatingCDB : public GlobalCompilationDatabase { std::optional getProjectInfo(PathRef File) const override; + std::shared_ptr + getProjectModules(PathRef File) const override; + tooling::CompileCommand getFallbackCommand(PathRef File) const override; bool blockUntilIdle(Deadline D) const override; @@ -122,6 +133,9 @@ class DirectoryBasedGlobalCompilationDatabase /// \p File's parents. std::optional getProjectInfo(PathRef File) const override; + std::shared_ptr + getProjectModules(PathRef File) const override; + bool blockUntilIdle(Deadline Timeout) const override; private: diff --git a/clang-tools-extra/clangd/ModuleDependencyScanner.cpp b/clang-tools-extra/clangd/ModuleDependencyScanner.cpp new file mode 100644 index 00000000000000..3413d0bb0eaa59 --- /dev/null +++ b/clang-tools-extra/clangd/ModuleDependencyScanner.cpp @@ -0,0 +1,81 @@ +//===---------------- ModuleDependencyScanner.cpp ----------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "ModuleDependencyScanner.h" +#include "support/Logger.h" + +namespace clang { +namespace clangd { + +std::optional +ModuleDependencyScanner::scan(PathRef FilePath) { + std::optional Cmd = CDB.getCompileCommand(FilePath); + + if (!Cmd) + return std::nullopt; + + using namespace clang::tooling::dependencies; + + llvm::SmallString<128> FilePathDir(FilePath); + llvm::sys::path::remove_filename(FilePathDir); + DependencyScanningTool ScanningTool(Service, TFS.view(FilePathDir)); + + llvm::Expected ScanningResult = + ScanningTool.getP1689ModuleDependencyFile(*Cmd, Cmd->Directory); + + if (auto E = ScanningResult.takeError()) { + log("Scanning modules dependencies for {0} failed: {1}", FilePath, + llvm::toString(std::move(E))); + return std::nullopt; + } + + ModuleDependencyInfo Result; + + if (ScanningResult->Provides) { + ModuleNameToSource[ScanningResult->Provides->ModuleName] = FilePath; + Result.ModuleName = ScanningResult->Provides->ModuleName; + } + + for (auto &Required : ScanningResult->Requires) + Result.RequiredModules.push_back(Required.ModuleName); + + return Result; +} + +void ModuleDependencyScanner::globalScan( + const std::vector &AllFiles) { + for (auto &File : AllFiles) + scan(File); + + GlobalScanned = true; +} + +PathRef +ModuleDependencyScanner::getSourceForModuleName(StringRef ModuleName) const { + assert( + GlobalScanned && + "We should only call getSourceForModuleName after calling globalScan()"); + + if (auto It = ModuleNameToSource.find(ModuleName); + It != ModuleNameToSource.end()) + return It->second; + + return {}; +} + +std::vector +ModuleDependencyScanner::getRequiredModules(PathRef File) { + auto ScanningResult = scan(File); + if (!ScanningResult) + return {}; + + return ScanningResult->RequiredModules; +} + +} // namespace clangd +} // namespace clang diff --git a/clang-tools-extra/clangd/ModuleDependencyScanner.h b/clang-tools-extra/clangd/ModuleDependencyScanner.h new file mode 100644 index 00000000000000..5251bdeadfb851 --- /dev/null +++ b/clang-tools-extra/clangd/ModuleDependencyScanner.h @@ -0,0 +1,106 @@ +//===-------------- ModuleDependencyScanner.h --------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H + +#include "GlobalCompilationDatabase.h" +#include "support/Path.h" +#include "support/ThreadsafeFS.h" + +#include "clang/Tooling/DependencyScanning/DependencyScanningService.h" +#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h" + +#include "llvm/ADT/SmallString.h" +#include "llvm/ADT/StringMap.h" + +namespace clang { +namespace clangd { + +/// A scanner to query the dependency information for C++20 Modules. +/// +/// The scanner can scan a single file with `scan(PathRef)` member function +/// or scan the whole project with `globalScan(PathRef)` member function. See +/// the comments of `globalScan` to see the details. +/// +/// ModuleDependencyScanner should only be used via ScanningAllProjectModules. +/// +/// The ModuleDependencyScanner can get the directly required module name for a +/// specific source file. Also the ModuleDependencyScanner can get the source +/// file declaring a specific module name. +/// +/// IMPORTANT NOTE: we assume that every module unit is only declared once in a +/// source file in the project. But the assumption is not strictly true even +/// besides the invalid projects. The language specification requires that every +/// module unit should be unique in a valid program. But a project can contain +/// multiple programs. Then it is valid that we can have multiple source files +/// declaring the same module in a project as long as these source files don't +/// interere with each other.` +class ModuleDependencyScanner { +public: + ModuleDependencyScanner(const GlobalCompilationDatabase &CDB, + const ThreadsafeFS &TFS) + : CDB(CDB), TFS(TFS), + Service(tooling::dependencies::ScanningMode::CanonicalPreprocessing, + tooling::dependencies::ScanningOutputFormat::P1689) {} + + // The scanned modules dependency information for a specific source file. + struct ModuleDependencyInfo { + // The name of the module if the file is a module unit. + std::optional ModuleName; + // A list of names for the modules that the file directly depends. + std::vector RequiredModules; + }; + + /// Scanning the single file specified by \param FilePath. + /// NOTE: This is only used by unittests for external uses. + std::optional scan(PathRef FilePath); + + /// Scanning every source file in the current project to get the + /// to map. + /// It looks unefficiency to scan the whole project especially for + /// every version of every file! + /// TODO: We should find an efficient method to get the + /// to map. We can make it either by providing + /// a global module dependency scanner to monitor every file. Or we + /// can simply require the build systems (or even if the end users) + /// to provide the map. + void globalScan(const std::vector &AllFiles); + bool isGlobalScanned() const { return GlobalScanned; } + + /// Get the source file from the module name. Note that the language + /// guarantees all the module names are unique in a valid program. + /// This function should only be called after globalScan. + /// + /// FIXME: We should handle the case that there are multiple source files + /// declaring the same module. + PathRef getSourceForModuleName(StringRef ModuleName) const; + + /// Return the direct required modules. Indirect required modules are not + /// included. + std::vector getRequiredModules(PathRef File); + +private: + const GlobalCompilationDatabase &CDB; + const ThreadsafeFS &TFS; + + // Whether the scanner has scanned the project globally. + bool GlobalScanned = false; + + clang::tooling::dependencies::DependencyScanningService Service; + + // TODO: Add a scanning cache. + + // Map module name to source file path. + llvm::StringMap ModuleNameToSource; +}; + +} // namespace clangd +} // namespace clang + +#endif diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp new file mode 100644 index 00000000000000..5c56f2fc31d399 --- /dev/null +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -0,0 +1,339 @@ +//===----------------- ModulesBuilder.cpp ------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "ModulesBuilder.h" +#include "PrerequisiteModules.h" +#include "support/Logger.h" + +#include "clang/Frontend/FrontendAction.h" +#include "clang/Frontend/FrontendActions.h" + +namespace clang { +namespace clangd { + +namespace { + +/// Get or create a path to store module files. Generally it should be: +/// +/// project_root/.cache/clangd/module_files/{RequiredPrefixDir}/. +/// +/// \param MainFile is used to get the root of the project from global +/// compilation database. \param RequiredPrefixDir is used to get the user +/// defined prefix for module files. This is useful when we want to seperate +/// module files. e.g., we want to build module files for the same module unit +/// `a.cppm` with 2 different users `b.cpp` and `c.cpp` and we don't want the +/// module file for `b.cpp` be conflict with the module files for `c.cpp`. Then +/// we can put the 2 module files into different dirs like: +/// +/// project_root/.cache/clangd/module_files/b.cpp/a.pcm +/// project_root/.cache/clangd/module_files/c.cpp/a.pcm +llvm::SmallString<256> getModuleFilesPath(PathRef MainFile, + const GlobalCompilationDatabase &CDB, + StringRef RequiredPrefixDir) { + std::optional PI = CDB.getProjectInfo(MainFile); + if (!PI) + return {}; + + // FIXME: PI->SourceRoot may be empty, depending on the CDB strategy. + llvm::SmallString<256> Result(PI->SourceRoot); + + llvm::sys::path::append(Result, ".cache"); + llvm::sys::path::append(Result, "clangd"); + llvm::sys::path::append(Result, "module_files"); + + llvm::sys::path::append(Result, RequiredPrefixDir); + + llvm::sys::fs::create_directories(Result, /*IgnoreExisting=*/true); + + return Result; +} + +/// Get the absolute path for the filename from the compile command. +llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand &Cmd) { + llvm::SmallString<128> AbsolutePath; + if (llvm::sys::path::is_absolute(Cmd.Filename)) { + AbsolutePath = Cmd.Filename; + } else { + AbsolutePath = Cmd.Directory; + llvm::sys::path::append(AbsolutePath, Cmd.Filename); + llvm::sys::path::remove_dots(AbsolutePath, true); + } + return AbsolutePath; +} + +/// Get a unique module file path under \param ModuleFilesPrefix. +std::string getUniqueModuleFilePath(StringRef ModuleName, + PathRef ModuleFilesPrefix) { + llvm::SmallString<256> ModuleFilePattern(ModuleFilesPrefix); + auto [PrimaryModuleName, PartitionName] = ModuleName.split(':'); + llvm::sys::path::append(ModuleFilePattern, PrimaryModuleName); + if (!PartitionName.empty()) { + ModuleFilePattern.append("-"); + ModuleFilePattern.append(PartitionName); + } + + ModuleFilePattern.append("-%%-%%-%%-%%-%%-%%"); + ModuleFilePattern.append(".pcm"); + + llvm::SmallString<256> ModuleFilePath; + llvm::sys::fs::createUniquePath(ModuleFilePattern, ModuleFilePath, + /*MakeAbsolute=*/false); + + return (std::string)ModuleFilePath; +} +} // namespace + +bool ModulesBuilder::buildModuleFile(StringRef ModuleName, + const ThreadsafeFS *TFS, + std::shared_ptr MDB, + PathRef ModuleFilesPrefix, + PrerequisiteModules &BuiltModuleFiles) { + if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) + return true; + + PathRef ModuleUnitFileName = MDB->getSourceForModuleName(ModuleName); + /// It is possible that we're meeting third party modules (modules whose + /// source are not in the project. e.g, the std module may be a third-party + /// module for most project) or something wrong with the implementation of + /// ProjectModules. + /// FIXME: How should we treat third party modules here? If we want to ignore + /// third party modules, we should return true instead of false here. + /// Currently we simply bail out. + if (ModuleUnitFileName.empty()) + return false; + + for (auto &RequiredModuleName : MDB->getRequiredModules(ModuleUnitFileName)) { + // Return early if there are errors building the module file. + if (!buildModuleFile(RequiredModuleName, TFS, MDB, ModuleFilesPrefix, + BuiltModuleFiles)) { + log("Failed to build module {0}", RequiredModuleName); + return false; + } + } + + auto Cmd = CDB.getCompileCommand(ModuleUnitFileName); + if (!Cmd) + return false; + + std::string ModuleFileName = + getUniqueModuleFilePath(ModuleName, ModuleFilesPrefix); + Cmd->Output = ModuleFileName; + + ParseInputs Inputs; + Inputs.TFS = TFS; + Inputs.CompileCommand = std::move(*Cmd); + + IgnoreDiagnostics IgnoreDiags; + auto CI = buildCompilerInvocation(Inputs, IgnoreDiags); + if (!CI) + return false; + + auto FS = Inputs.TFS->view(Inputs.CompileCommand.Directory); + auto AbsolutePath = getAbsolutePath(Inputs.CompileCommand); + auto Buf = FS->getBufferForFile(AbsolutePath); + if (!Buf) + return false; + + // Hash the contents of input files and store the hash value to the BMI files. + // So that we can check if the files are still valid when we want to reuse the + // BMI files. + CI->getHeaderSearchOpts().ValidateASTInputFilesContent = true; + + BuiltModuleFiles.adjustHeaderSearchOptions(CI->getHeaderSearchOpts()); + + CI->getFrontendOpts().OutputFile = Inputs.CompileCommand.Output; + auto Clang = + prepareCompilerInstance(std::move(CI), /*Preamble=*/nullptr, + std::move(*Buf), std::move(FS), IgnoreDiags); + if (!Clang) + return false; + + GenerateModuleInterfaceAction Action; + Clang->ExecuteAction(Action); + + if (Clang->getDiagnostics().hasErrorOccurred()) + return false; + + BuiltModuleFiles.addModuleFile(ModuleName, ModuleFileName); + return true; +} + +/// FailedPrerequisiteModules - stands for the PrerequisiteModules which has +/// errors happened during the building process. +class FailedPrerequisiteModules : public PrerequisiteModules { +public: + ~FailedPrerequisiteModules() override = default; + + /// We shouldn't adjust the compilation commands based on + /// FailedPrerequisiteModules. + void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override { + } + + /// FailedPrerequisiteModules can never be reused. + bool + canReuse(const CompilerInvocation &CI, + llvm::IntrusiveRefCntPtr) const override { + return false; + } + + /// No module unit got built in FailedPrerequisiteModules. + bool isModuleUnitBuilt(StringRef ModuleName) const override { return false; } + + /// We shouldn't add any module files to the FailedPrerequisiteModules. + void addModuleFile(StringRef ModuleName, StringRef ModuleFilePath) override { + assert(false && "We shouldn't operator based on failed module files"); + } +}; + +/// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all +/// the required modules are built successfully. All the module files +/// are owned by the StandalonePrerequisiteModules class. +/// +/// All the built module files won't be shared with other instances of the +/// class. So that we can avoid worrying thread safety. +/// +/// We don't need to worry about duplicated module names here since the standard +/// guarantees the module names should be unique to a program. +class StandalonePrerequisiteModules : public PrerequisiteModules { +public: + StandalonePrerequisiteModules() = default; + + StandalonePrerequisiteModules(const StandalonePrerequisiteModules &) = delete; + StandalonePrerequisiteModules + operator=(const StandalonePrerequisiteModules &) = delete; + StandalonePrerequisiteModules(StandalonePrerequisiteModules &&) = delete; + StandalonePrerequisiteModules + operator=(StandalonePrerequisiteModules &&) = delete; + + ~StandalonePrerequisiteModules() override = default; + + void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override { + // Appending all built module files. + for (auto &RequiredModule : RequiredModules) + Options.PrebuiltModuleFiles.insert_or_assign( + RequiredModule.ModuleName, RequiredModule.ModuleFilePath); + } + + bool canReuse(const CompilerInvocation &CI, + llvm::IntrusiveRefCntPtr) const override; + + bool isModuleUnitBuilt(StringRef ModuleName) const override { + constexpr unsigned SmallSizeThreshold = 8; + if (RequiredModules.size() < SmallSizeThreshold) + return llvm::any_of(RequiredModules, [&](auto &MF) { + return MF.ModuleName == ModuleName; + }); + + return BuiltModuleNames.contains(ModuleName); + } + + void addModuleFile(StringRef ModuleName, StringRef ModuleFilePath) override { + RequiredModules.emplace_back(ModuleName, ModuleFilePath); + BuiltModuleNames.insert(ModuleName); + } + +private: + struct ModuleFile { + ModuleFile(StringRef ModuleName, PathRef ModuleFilePath) + : ModuleName(ModuleName.str()), ModuleFilePath(ModuleFilePath.str()) {} + + ModuleFile() = delete; + + ModuleFile(const ModuleFile &) = delete; + ModuleFile operator=(const ModuleFile &) = delete; + + // The move constructor is needed for llvm::SmallVector. + ModuleFile(ModuleFile &&Other) + : ModuleName(std::move(Other.ModuleName)), + ModuleFilePath(std::move(Other.ModuleFilePath)) {} + + ModuleFile &operator=(ModuleFile &&Other) = delete; + + ~ModuleFile() { + if (!ModuleFilePath.empty()) + llvm::sys::fs::remove(ModuleFilePath); + } + + std::string ModuleName; + std::string ModuleFilePath; + }; + + llvm::SmallVector RequiredModules; + /// A helper class to speedup the query if a module is built. + llvm::StringSet<> BuiltModuleNames; +}; + +std::unique_ptr +ModulesBuilder::buildPrerequisiteModulesFor(PathRef File, + const ThreadsafeFS *TFS) { + std::shared_ptr MDB = CDB.getProjectModules(File); + if (!MDB) + return {}; + + std::vector RequiredModuleNames = MDB->getRequiredModules(File); + if (RequiredModuleNames.empty()) + return {}; + + llvm::SmallString<256> ModuleFilesPrefix = + getModuleFilesPath(File, CDB, llvm::sys::path::filename(File)); + + auto RequiredModules = std::make_unique(); + + for (const std::string &RequiredModuleName : RequiredModuleNames) + // Return early if there is any error. + if (!buildModuleFile(RequiredModuleName, TFS, MDB, ModuleFilesPrefix, + *RequiredModules.get())) { + log("Failed to build module {0}", RequiredModuleName); + return std::make_unique(); + } + + return std::move(RequiredModules); +} + +bool StandalonePrerequisiteModules::canReuse( + const CompilerInvocation &CI, + llvm::IntrusiveRefCntPtr VFS) const { + CompilerInstance Clang; + + Clang.setInvocation(std::make_shared(CI)); + IntrusiveRefCntPtr Diags = + CompilerInstance::createDiagnostics(new DiagnosticOptions()); + Clang.setDiagnostics(Diags.get()); + + FileManager *FM = Clang.createFileManager(VFS); + Clang.createSourceManager(*FM); + + if (!Clang.createTarget()) + return false; + + assert(Clang.getHeaderSearchOptsPtr()); + adjustHeaderSearchOptions(Clang.getHeaderSearchOpts()); + // Since we don't need to compile the source code actually, the TU kind here + // doesn't matter. + Clang.createPreprocessor(TU_Complete); + Clang.getHeaderSearchOpts().ForceCheckCXX20ModulesInputFiles = true; + Clang.getHeaderSearchOpts().ValidateASTInputFilesContent = true; + + Clang.createASTReader(); + for (auto &RequiredModule : RequiredModules) { + StringRef BMIPath = RequiredModule.ModuleFilePath; + auto ReadResult = + Clang.getASTReader()->ReadAST(BMIPath, serialization::MK_MainFile, + SourceLocation(), ASTReader::ARR_None); + + if (ReadResult != ASTReader::Success) { + log("Failed to reuse {0}", BMIPath); + return false; + } + } + + return true; +} + +} // namespace clangd +} // namespace clang diff --git a/clang-tools-extra/clangd/ModulesBuilder.h b/clang-tools-extra/clangd/ModulesBuilder.h new file mode 100644 index 00000000000000..ae7fc8778be676 --- /dev/null +++ b/clang-tools-extra/clangd/ModulesBuilder.h @@ -0,0 +1,71 @@ +//===----------------- ModulesBuilder.h --------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// Experimental support for C++20 Modules. +// +// Currently we simplify the implementations by preventing reusing module files +// across different versions and different source files. But this is clearly a +// waste of time and space in the end of the day. +// +// FIXME: Supporting reusing module files across different versions and +// different source files. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULES_BUILDER_H + +#include "GlobalCompilationDatabase.h" +#include "ProjectModules.h" + +#include "support/Path.h" +#include "support/ThreadsafeFS.h" + +#include "llvm/ADT/SmallString.h" + +#include + +namespace clang { +namespace clangd { + +class PrerequisiteModules; + +/// This class handles building module files for a given source file. +/// +/// In the future, we want the class to manage the module files acorss +/// different versions and different source files. +class ModulesBuilder { +public: + ModulesBuilder() = delete; + + ModulesBuilder(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} + + ModulesBuilder(const ModulesBuilder &) = delete; + ModulesBuilder(ModulesBuilder &&) = delete; + + ModulesBuilder &operator=(const ModulesBuilder &) = delete; + ModulesBuilder &operator=(ModulesBuilder &&) = delete; + + ~ModulesBuilder() = default; + + std::unique_ptr + buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS *TFS); + +private: + bool buildModuleFile(StringRef ModuleName, const ThreadsafeFS *TFS, + std::shared_ptr MDB, + PathRef ModuleFilesPrefix, + PrerequisiteModules &RequiredModules); + + const GlobalCompilationDatabase &CDB; +}; + +} // namespace clangd +} // namespace clang + +#endif diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index edd0f77b1031ef..565f24df188fc4 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -444,6 +444,12 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, L->sawDiagnostic(D, Diag); }); + // Adjust header search options to load the built module files recorded + // in RequiredModules. + if (Preamble && Preamble->RequiredModules) + Preamble->RequiredModules->adjustHeaderSearchOptions( + CI->getHeaderSearchOpts()); + std::optional Patch; // We might use an ignoring diagnostic consumer if they are going to be // dropped later on to not pay for extra latency by processing them. @@ -457,6 +463,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs, std::move(CI), PreamblePCH, llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, Filename), VFS, *DiagConsumer); + if (!Clang) { // The last diagnostic contains information about the reason of this // failure. diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp index f181c7befec156..989f839fe36377 100644 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -587,11 +587,10 @@ class DiagPatcher { }; } // namespace -std::shared_ptr -buildPreamble(PathRef FileName, CompilerInvocation CI, - const ParseInputs &Inputs, bool StoreInMemory, - PreambleParsedCallback PreambleCallback, - PreambleBuildStats *Stats) { +std::shared_ptr buildPreamble( + PathRef FileName, CompilerInvocation CI, const ParseInputs &Inputs, + bool StoreInMemory, ModulesBuilder *RequiredModuleBuilder, + PreambleParsedCallback PreambleCallback, PreambleBuildStats *Stats) { // Note that we don't need to copy the input contents, preamble can live // without those. auto ContentsBuffer = @@ -660,10 +659,12 @@ buildPreamble(PathRef FileName, CompilerInvocation CI, WallTimer PreambleTimer; PreambleTimer.startTimer(); + auto BuiltPreamble = PrecompiledPreamble::Build( CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, Stats ? TimedFS : StatCacheFS, std::make_shared(), StoreInMemory, /*StoragePath=*/"", CapturedInfo); + PreambleTimer.stopTimer(); // We have to setup DiagnosticConsumer that will be alife @@ -696,6 +697,19 @@ buildPreamble(PathRef FileName, CompilerInvocation CI, Result->Includes = CapturedInfo.takeIncludes(); Result->Pragmas = std::make_shared( CapturedInfo.takePragmaIncludes()); + + if (RequiredModuleBuilder) { + WallTimer PrerequisiteModuleTimer; + PrerequisiteModuleTimer.startTimer(); + Result->RequiredModules = + RequiredModuleBuilder->buildPrerequisiteModulesFor(FileName, + Inputs.TFS); + PrerequisiteModuleTimer.stopTimer(); + + log("Built prerequisite modules for file {0} in {1} seconds", FileName, + PrerequisiteModuleTimer.getTime()); + } + Result->Macros = CapturedInfo.takeMacros(); Result->Marks = CapturedInfo.takeMarks(); Result->StatCache = StatCache; @@ -736,7 +750,9 @@ bool isPreambleCompatible(const PreambleData &Preamble, auto VFS = Inputs.TFS->view(Inputs.CompileCommand.Directory); return compileCommandsAreEqual(Inputs.CompileCommand, Preamble.CompileCommand) && - Preamble.Preamble.CanReuse(CI, *ContentsBuffer, Bounds, *VFS); + Preamble.Preamble.CanReuse(CI, *ContentsBuffer, Bounds, *VFS) && + (!Preamble.RequiredModules || + Preamble.RequiredModules->canReuse(CI, VFS)); } void escapeBackslashAndQuotes(llvm::StringRef Text, llvm::raw_ostream &OS) { diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h index 37da3833748a9c..076677ab3bed5f 100644 --- a/clang-tools-extra/clangd/Preamble.h +++ b/clang-tools-extra/clangd/Preamble.h @@ -27,6 +27,10 @@ #include "Diagnostics.h" #include "FS.h" #include "Headers.h" + +#include "ModulesBuilder.h" +#include "PrerequisiteModules.h" + #include "clang-include-cleaner/Record.h" #include "support/Path.h" #include "clang/Basic/SourceManager.h" @@ -104,6 +108,8 @@ struct PreambleData { IncludeStructure Includes; // Captures #include-mapping information in #included headers. std::shared_ptr Pragmas; + // Information about required module files for this preamble. + std::unique_ptr RequiredModules; // Macros defined in the preamble section of the main file. // Users care about headers vs main-file, not preamble vs non-preamble. // These should be treated as main-file entities e.g. for code completion. @@ -144,9 +150,13 @@ struct PreambleBuildStats { /// If \p PreambleCallback is set, it will be run on top of the AST while /// building the preamble. /// If Stats is not non-null, build statistics will be exported there. +/// If \p RequiredModuleBuilder is not null, it will scan the source file +/// to see if it is related to modules, and if yes, modules related things +/// will be built. std::shared_ptr buildPreamble(PathRef FileName, CompilerInvocation CI, const ParseInputs &Inputs, bool StoreInMemory, + ModulesBuilder *RequiredModuleBuilder, PreambleParsedCallback PreambleCallback, PreambleBuildStats *Stats = nullptr); diff --git a/clang-tools-extra/clangd/PrerequisiteModules.h b/clang-tools-extra/clangd/PrerequisiteModules.h new file mode 100644 index 00000000000000..a73beb76480248 --- /dev/null +++ b/clang-tools-extra/clangd/PrerequisiteModules.h @@ -0,0 +1,87 @@ +//===----------------- PrerequisiteModules.h ---------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PREREQUISITEMODULES_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PREREQUISITEMODULES_H + +#include "Compiler.h" +#include "support/Path.h" + +#include "clang/Lex/HeaderSearchOptions.h" + +#include "llvm/ADT/StringSet.h" + +namespace clang { +namespace clangd { + +class ModulesBuilder; + +/// Store all the needed module files information to parse a single +/// source file. e.g., +/// +/// ``` +/// // a.cppm +/// export module a; +/// +/// // b.cppm +/// export module b; +/// import a; +/// +/// // c.cppm +/// export module c; +/// import a; +/// ``` +/// +/// For the source file `c.cppm`, an instance of the class will store +/// the module files for `a.cppm` and `b.cppm`. But the module file for `c.cppm` +/// won't be stored. Since it is not needed to parse `c.cppm`. +/// +/// Users should only get PrerequisiteModules from +/// `ModulesBuilder::buildPrerequisiteModulesFor(...)`. +/// +/// Users can detect whether the PrerequisiteModules is still up to date by +/// calling the `canReuse()` member function. +/// +/// The users should call `adjustHeaderSearchOptions(...)` to update the +/// compilation commands to select the built module files first. Before calling +/// `adjustHeaderSearchOptions()`, users should call `canReuse()` first to check +/// if all the stored module files are valid. In case they are not valid, +/// users should call `ModulesBuilder::buildPrerequisiteModulesFor(...)` again +/// to get the new PrerequisiteModules. +class PrerequisiteModules { +public: + /// Change commands to load the module files recorded in this + /// PrerequisiteModules first. + virtual void + adjustHeaderSearchOptions(HeaderSearchOptions &Options) const = 0; + + /// Whether or not the built module files are up to date. + /// Note that this can only be used after building the module files. + virtual bool + canReuse(const CompilerInvocation &CI, + llvm::IntrusiveRefCntPtr) const = 0; + + /// Return true if the modile file specified by ModuleName is built. + /// Note that this interface will only check the existence of the module + /// file instead of checking the validness of the module file. + virtual bool isModuleUnitBuilt(StringRef ModuleName) const = 0; + + virtual ~PrerequisiteModules() = default; + +private: + friend class ModulesBuilder; + + /// Add a module file to the PrerequisiteModules. + virtual void addModuleFile(StringRef ModuleName, + StringRef ModuleFilePath) = 0; +}; + +} // namespace clangd +} // namespace clang + +#endif diff --git a/clang-tools-extra/clangd/ProjectModules.cpp b/clang-tools-extra/clangd/ProjectModules.cpp new file mode 100644 index 00000000000000..6a9d9731c309ae --- /dev/null +++ b/clang-tools-extra/clangd/ProjectModules.cpp @@ -0,0 +1,62 @@ +//===------------------ ProjectModules.h -------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "ProjectModules.h" + +namespace clang { +namespace clangd { + +/// TODO: The existing `ScanningAllProjectModules` is not efficient. See the +/// comments in ModuleDependencyScanner for detail. +/// +/// In the future, we wish the build system can provide a well design +/// compilation database for modules then we can query that new compilation +/// database directly. Or we need to have a global long-live scanner to detect +/// the state of each file. +class ScanningAllProjectModules : public ProjectModules { +public: + ScanningAllProjectModules(std::vector &&AllFiles, + const GlobalCompilationDatabase &CDB, + const ThreadsafeFS &TFS) + : AllFiles(std::move(AllFiles)), Scanner(CDB, TFS) {} + + ~ScanningAllProjectModules() override = default; + + std::vector getRequiredModules(PathRef File) override { + return Scanner.getRequiredModules(File); + } + + /// RequiredSourceFile is not used intentionally. See the comments of + /// ModuleDependencyScanner for detail. + PathRef + getSourceForModuleName(StringRef ModuleName, + PathRef RequiredSourceFile = PathRef()) override { + if (!Scanner.isGlobalScanned()) + Scanner.globalScan(AllFiles); + + return Scanner.getSourceForModuleName(ModuleName); + } + +private: + std::vector AllFiles; + + ModuleDependencyScanner Scanner; +}; + +std::shared_ptr ProjectModules::create( + ProjectModulesKind Kind, std::vector &&AllFiles, + const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS) { + if (Kind == ProjectModulesKind::ScanningAllFiles) + return std::make_shared(std::move(AllFiles), CDB, + TFS); + + llvm_unreachable("Unknown ProjectModulesKind."); +} + +} // namespace clangd +} // namespace clang \ No newline at end of file diff --git a/clang-tools-extra/clangd/ProjectModules.h b/clang-tools-extra/clangd/ProjectModules.h new file mode 100644 index 00000000000000..98720ee06d4724 --- /dev/null +++ b/clang-tools-extra/clangd/ProjectModules.h @@ -0,0 +1,55 @@ +//===------------------ ProjectModules.h -------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROJECTMODULES_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROJECTMODULES_H + +#include "ModuleDependencyScanner.h" + +#include + +namespace clang { +namespace clangd { + +/// An interface to query the modules information in the project. +/// Users should get instances of `ProjectModules` from +/// `GlobalCompilationDatabase::getProjectModules(PathRef)`. +/// +/// Currently, the modules information includes: +/// - Given a source file, what are the required modules. +/// - Given a module name and a required source file, what is +/// the corresponding source file. +/// +/// Note that there can be multiple source files declaring the same module +/// in a valid project. Although the language specification requires that +/// every module unit's name must be unique in valid program, there can be +/// multiple program in a project. And it is technically valid if these program +/// doesn't interfere with each other. +/// +/// A module name should be in the format: +/// `[:partition-name]`. So module names covers partitions. +class ProjectModules { +public: + enum class ProjectModulesKind { ScanningAllFiles }; + + static std::shared_ptr + create(ProjectModulesKind Kind, std::vector &&AllFiles, + const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS); + + virtual std::vector getRequiredModules(PathRef File) = 0; + virtual PathRef + getSourceForModuleName(StringRef ModuleName, + PathRef RequiredSrcFile = PathRef()) = 0; + + virtual ~ProjectModules() = default; +}; + +} // namespace clangd +} // namespace clang + +#endif diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp index 324ba1fc8cb895..cb90d29c2313ae 100644 --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -52,6 +52,7 @@ #include "Config.h" #include "Diagnostics.h" #include "GlobalCompilationDatabase.h" +#include "ModulesBuilder.h" #include "ParsedAST.h" #include "Preamble.h" #include "clang-include-cleaner/Record.h" @@ -605,8 +606,8 @@ class ASTWorker { ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB, TUScheduler::ASTCache &LRUCache, TUScheduler::HeaderIncluderCache &HeaderIncluders, - Semaphore &Barrier, bool RunSync, const TUScheduler::Options &Opts, - ParsingCallbacks &Callbacks); + Semaphore &Barrier, ModulesBuilder *ModulesManager, bool RunSync, + const TUScheduler::Options &Opts, ParsingCallbacks &Callbacks); public: /// Create a new ASTWorker and return a handle to it. @@ -619,7 +620,8 @@ class ASTWorker { TUScheduler::ASTCache &IdleASTs, TUScheduler::HeaderIncluderCache &HeaderIncluders, AsyncTaskRunner *Tasks, Semaphore &Barrier, - const TUScheduler::Options &Opts, ParsingCallbacks &Callbacks); + ModulesBuilder *ModulesManager, const TUScheduler::Options &Opts, + ParsingCallbacks &Callbacks); ~ASTWorker(); void update(ParseInputs Inputs, WantDiagnostics, bool ContentChanged); @@ -652,6 +654,10 @@ class ASTWorker { TUScheduler::FileStats stats() const; bool isASTCached() const; + const GlobalCompilationDatabase &getCompilationDatabase() { return CDB; } + + ModulesBuilder *getModulesManager() const { return ModulesManager; } + private: // Details of an update request that are relevant to scheduling. struct UpdateType { @@ -710,6 +716,7 @@ class ASTWorker { TUScheduler::ASTCache &IdleASTs; TUScheduler::HeaderIncluderCache &HeaderIncluders; const bool RunSync; + /// Time to wait after an update to see whether another update obsoletes it. const DebouncePolicy UpdateDebounce; /// File that ASTWorker is responsible for. @@ -763,6 +770,8 @@ class ASTWorker { SynchronizedTUStatus Status; PreambleThread PreamblePeer; + + ModulesBuilder *ModulesManager = nullptr; }; /// A smart-pointer-like class that points to an active ASTWorker. @@ -807,16 +816,15 @@ class ASTWorkerHandle { std::shared_ptr Worker; }; -ASTWorkerHandle -ASTWorker::create(PathRef FileName, const GlobalCompilationDatabase &CDB, - TUScheduler::ASTCache &IdleASTs, - TUScheduler::HeaderIncluderCache &HeaderIncluders, - AsyncTaskRunner *Tasks, Semaphore &Barrier, - const TUScheduler::Options &Opts, - ParsingCallbacks &Callbacks) { - std::shared_ptr Worker( - new ASTWorker(FileName, CDB, IdleASTs, HeaderIncluders, Barrier, - /*RunSync=*/!Tasks, Opts, Callbacks)); +ASTWorkerHandle ASTWorker::create( + PathRef FileName, const GlobalCompilationDatabase &CDB, + TUScheduler::ASTCache &IdleASTs, + TUScheduler::HeaderIncluderCache &HeaderIncluders, AsyncTaskRunner *Tasks, + Semaphore &Barrier, ModulesBuilder *ModulesManager, + const TUScheduler::Options &Opts, ParsingCallbacks &Callbacks) { + std::shared_ptr Worker(new ASTWorker( + FileName, CDB, IdleASTs, HeaderIncluders, Barrier, ModulesManager, + /*RunSync=*/!Tasks, Opts, Callbacks)); if (Tasks) { Tasks->runAsync("ASTWorker:" + llvm::sys::path::filename(FileName), [Worker]() { Worker->run(); }); @@ -830,15 +838,16 @@ ASTWorker::create(PathRef FileName, const GlobalCompilationDatabase &CDB, ASTWorker::ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB, TUScheduler::ASTCache &LRUCache, TUScheduler::HeaderIncluderCache &HeaderIncluders, - Semaphore &Barrier, bool RunSync, - const TUScheduler::Options &Opts, + Semaphore &Barrier, ModulesBuilder *ModulesManager, + bool RunSync, const TUScheduler::Options &Opts, ParsingCallbacks &Callbacks) : IdleASTs(LRUCache), HeaderIncluders(HeaderIncluders), RunSync(RunSync), UpdateDebounce(Opts.UpdateDebounce), FileName(FileName), ContextProvider(Opts.ContextProvider), CDB(CDB), Callbacks(Callbacks), Barrier(Barrier), Done(false), Status(FileName, Callbacks), PreamblePeer(FileName, Callbacks, Opts.StorePreamblesInMemory, RunSync, - Opts.PreambleThrottler, Status, HeaderIncluders, *this) { + Opts.PreambleThrottler, Status, HeaderIncluders, *this), + ModulesManager(ModulesManager) { // Set a fallback command because compile command can be accessed before // `Inputs` is initialized. Other fields are only used after initialization // from client inputs. @@ -1082,8 +1091,9 @@ void PreambleThread::build(Request Req) { PreambleBuildStats Stats; bool IsFirstPreamble = !LatestBuild; + LatestBuild = clang::clangd::buildPreamble( - FileName, *Req.CI, Inputs, StoreInMemory, + FileName, *Req.CI, Inputs, StoreInMemory, ASTPeer.getModulesManager(), [&](CapturedASTCtx ASTCtx, std::shared_ptr PI) { Callbacks.onPreambleAST(FileName, Inputs.Version, std::move(ASTCtx), @@ -1644,6 +1654,9 @@ TUScheduler::TUScheduler(const GlobalCompilationDatabase &CDB, PreambleTasks.emplace(); WorkerThreads.emplace(); } + + if (Opts.ExperimentalModulesSupport) + ModulesManager.emplace(CDB); } TUScheduler::~TUScheduler() { @@ -1676,7 +1689,8 @@ bool TUScheduler::update(PathRef File, ParseInputs Inputs, // Create a new worker to process the AST-related tasks. ASTWorkerHandle Worker = ASTWorker::create( File, CDB, *IdleASTs, *HeaderIncluders, - WorkerThreads ? &*WorkerThreads : nullptr, Barrier, Opts, *Callbacks); + WorkerThreads ? &*WorkerThreads : nullptr, Barrier, + ModulesManager ? &*ModulesManager : nullptr, Opts, *Callbacks); FD = std::unique_ptr( new FileData{Inputs.Contents, std::move(Worker)}); ContentChanged = true; diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h index fb936d46bbcf7e..66e17cbaa7f032 100644 --- a/clang-tools-extra/clangd/TUScheduler.h +++ b/clang-tools-extra/clangd/TUScheduler.h @@ -204,6 +204,8 @@ class ParsingCallbacks { virtual void onPreamblePublished(PathRef File) {} }; +class ModulesBuilder; + /// Handles running tasks for ClangdServer and managing the resources (e.g., /// preambles and ASTs) for opened files. /// TUScheduler is not thread-safe, only one thread should be providing updates @@ -222,6 +224,9 @@ class TUScheduler { /// Cache (large) preamble data in RAM rather than temporary files on disk. bool StorePreamblesInMemory = false; + /// Enable experimental support for modules. + bool ExperimentalModulesSupport = false; + /// Time to wait after an update to see if another one comes along. /// This tries to ensure we rebuild once the user stops typing. DebouncePolicy UpdateDebounce; @@ -372,6 +377,8 @@ class TUScheduler { // running tasks asynchronously. std::optional PreambleTasks; std::optional WorkerThreads; + // Manages to build module files. + std::optional ModulesManager; // Used to create contexts for operations that are not bound to a particular // file (e.g. index queries). std::string LastActiveFile; diff --git a/clang-tools-extra/clangd/test/CMakeLists.txt b/clang-tools-extra/clangd/test/CMakeLists.txt index d073267066e0b4..b51f461a498665 100644 --- a/clang-tools-extra/clangd/test/CMakeLists.txt +++ b/clang-tools-extra/clangd/test/CMakeLists.txt @@ -2,6 +2,7 @@ set(CLANGD_TEST_DEPS clangd ClangdTests clangd-indexer + split-file # No tests for it, but we should still make sure they build. dexp ) diff --git a/clang-tools-extra/clangd/test/modules.test b/clang-tools-extra/clangd/test/modules.test new file mode 100644 index 00000000000000..74280811a6cff8 --- /dev/null +++ b/clang-tools-extra/clangd/test/modules.test @@ -0,0 +1,83 @@ +# A smoke test to check the modules can work basically. +# +# Windows have different escaping modes. +# FIXME: We should add one for windows. +# UNSUPPORTED: system-windows +# +# RUN: rm -fr %t +# RUN: mkdir -p %t +# RUN: split-file %s %t +# +# RUN: sed -e "s|DIR|%/t|g" %t/compile_commands.json.tmpl > %t/compile_commands.json.tmp +# RUN: sed -e "s|CLANG_CC|%clang|g" %t/compile_commands.json.tmp > %t/compile_commands.json +# RUN: sed -e "s|DIR|%/t|g" %t/definition.jsonrpc.tmpl > %t/definition.jsonrpc +# +# RUN: clangd -experimental-modules-support -lit-test < %t/definition.jsonrpc \ +# RUN: | FileCheck -strict-whitespace %t/definition.jsonrpc + +#--- A.cppm +export module A; +export void printA() {} + +#--- Use.cpp +import A; +void foo() { + print +} + +#--- compile_commands.json.tmpl +[ + { + "directory": "DIR", + "command": "CLANG_CC -fprebuilt-module-path=DIR -std=c++20 -o DIR/main.cpp.o -c DIR/Use.cpp", + "file": "DIR/Use.cpp" + }, + { + "directory": "DIR", + "command": "CLANG_CC -std=c++20 DIR/A.cppm --precompile -o DIR/A.pcm", + "file": "DIR/A.cppm" + } +] + +#--- definition.jsonrpc.tmpl +{ + "jsonrpc": "2.0", + "id": 0, + "method": "initialize", + "params": { + "processId": 123, + "rootPath": "clangd", + "capabilities": { + "textDocument": { + "completion": { + "completionItem": { + "snippetSupport": true + } + } + } + }, + "trace": "off" + } +} +--- +{ + "jsonrpc": "2.0", + "method": "textDocument/didOpen", + "params": { + "textDocument": { + "uri": "file://DIR/Use.cpp", + "languageId": "cpp", + "version": 1, + "text": "import A;\nvoid foo() {\n print\n}\n" + } + } +} + +# CHECK: "message"{{.*}}printA{{.*}}(fix available) + +--- +{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file://DIR/Use.cpp"},"context":{"triggerKind":1},"position":{"line":2,"character":6}}} +--- +{"jsonrpc":"2.0","id":2,"method":"shutdown"} +--- +{"jsonrpc":"2.0","method":"exit"} diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp index b5c4d145619df3..1a245b4d007b8c 100644 --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -146,6 +146,8 @@ class Checker { ClangdLSPServer::Options Opts; // from buildCommand tooling::CompileCommand Cmd; + std::unique_ptr BaseCDB; + std::unique_ptr CDB; // from buildInvocation ParseInputs Inputs; std::unique_ptr Invocation; @@ -168,14 +170,14 @@ class Checker { DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS); CDBOpts.CompileCommandsDir = Config::current().CompileFlags.CDBSearch.FixedCDBPath; - std::unique_ptr BaseCDB = + BaseCDB = std::make_unique(CDBOpts); auto Mangler = CommandMangler::detect(); Mangler.SystemIncludeExtractor = getSystemIncludeExtractor(llvm::ArrayRef(Opts.QueryDriverGlobs)); if (Opts.ResourceDir) Mangler.ResourceDir = *Opts.ResourceDir; - auto CDB = std::make_unique( + CDB = std::make_unique( BaseCDB.get(), std::vector{}, std::move(Mangler)); if (auto TrueCmd = CDB->getCompileCommand(File)) { @@ -234,8 +236,15 @@ class Checker { // Build preamble and AST, and index them. bool buildAST() { log("Building preamble..."); + + auto RequiredModuleBuilder = + Opts.ExperimentalModulesSupport + ? std::make_unique(*CDB.get()) + : nullptr; + Preamble = buildPreamble( File, *Invocation, Inputs, /*StoreInMemory=*/true, + RequiredModuleBuilder.get(), [&](CapturedASTCtx Ctx, std::shared_ptr PI) { if (!Opts.BuildDynamicSymbolIndex) diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp index f656a8c587c653..e38b7c567fb7a9 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -550,6 +550,13 @@ opt ProjectRoot{ }; #endif +opt ExperimentalModulesSupport{ + "experimental-modules-support", + cat(Features), + desc("Experimental support for standard c++ modules"), + init(false), +}; + /// Supports a test URI scheme with relaxed constraints for lit tests. /// The path in a test URI will be combined with a platform-specific fake /// directory to form an absolute path. For example, test:///a.cpp is resolved @@ -861,6 +868,7 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var ClangdLSPServer::Options Opts; Opts.UseDirBasedCDB = (CompileArgsFrom == FilesystemCompileArgs); + Opts.ExperimentalModulesSupport = ExperimentalModulesSupport; switch (PCHStorage) { case PCHStorageFlag::Memory: diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt index 8d02b91fdd7166..529b8d2a11733f 100644 --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -72,6 +72,8 @@ add_unittest(ClangdUnitTests ClangdTests LoggerTests.cpp LSPBinderTests.cpp LSPClient.cpp + ModuleDependencyScannerTest.cpp + PrerequisiteModulesTest.cpp ModulesTests.cpp ParsedASTTests.cpp PathMappingTests.cpp diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 766998eb4f3c71..0cb14c429696b7 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -132,8 +132,12 @@ CodeCompleteResult completions(const TestTU &TU, Position Point, ADD_FAILURE() << "Couldn't build CompilerInvocation"; return {}; } + + MockCompilationDatabase CDB; auto Preamble = buildPreamble(testPath(TU.Filename), *CI, Inputs, - /*InMemory=*/true, /*Callback=*/nullptr); + /*InMemory=*/true, + /*RequiredModuleBuilder=*/nullptr, + /*Callback=*/nullptr); return codeComplete(testPath(TU.Filename), Point, Preamble.get(), Inputs, Opts); } @@ -1360,8 +1364,12 @@ signatures(llvm::StringRef Text, Position Point, ADD_FAILURE() << "Couldn't build CompilerInvocation"; return {}; } - auto Preamble = buildPreamble(testPath(TU.Filename), *CI, Inputs, - /*InMemory=*/true, /*Callback=*/nullptr); + + MockCompilationDatabase CDB; + auto Preamble = + buildPreamble(testPath(TU.Filename), *CI, Inputs, + /*InMemory=*/true, /*RequiredModuleBuilder=*/nullptr, + /*Callback=*/nullptr); if (!Preamble) { ADD_FAILURE() << "Couldn't build Preamble"; return {}; @@ -1642,8 +1650,12 @@ TEST(SignatureHelpTest, StalePreamble) { auto Inputs = TU.inputs(FS); auto CI = buildCompilerInvocation(Inputs, Diags); ASSERT_TRUE(CI); - auto EmptyPreamble = buildPreamble(testPath(TU.Filename), *CI, Inputs, - /*InMemory=*/true, /*Callback=*/nullptr); + + MockCompilationDatabase CDB; + auto EmptyPreamble = + buildPreamble(testPath(TU.Filename), *CI, Inputs, + /*InMemory=*/true, /*RequiredModuleBuilder=*/nullptr, + /*Callback=*/nullptr); ASSERT_TRUE(EmptyPreamble); TU.AdditionalFiles["a.h"] = "int foo(int x);"; diff --git a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp index cf30b388d234df..b3370d5f4da9b1 100644 --- a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp @@ -336,9 +336,11 @@ TEST(FileIndexTest, RebuildWithPreamble) { FileIndex Index; bool IndexUpdated = false; + + MockCompilationDatabase CDB; buildPreamble( FooCpp, *CI, PI, - /*StoreInMemory=*/true, + /*StoreInMemory=*/true, /*RequiredModuleBuilder=*/nullptr, [&](CapturedASTCtx ASTCtx, std::shared_ptr PI) { auto &Ctx = ASTCtx.getASTContext(); diff --git a/clang-tools-extra/clangd/unittests/ModuleDependencyScannerTest.cpp b/clang-tools-extra/clangd/unittests/ModuleDependencyScannerTest.cpp new file mode 100644 index 00000000000000..6d42f8730538f2 --- /dev/null +++ b/clang-tools-extra/clangd/unittests/ModuleDependencyScannerTest.cpp @@ -0,0 +1,176 @@ +//===------------ ModuleDependencyScannerTest.cpp ---------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +/// FIXME: Skip testing on windows temporarily due to the different escaping +/// code mode. +#ifndef _WIN32 + +#include "ModuleDependencyScanner.h" +#include "ModulesTestSetup.h" +#include "TestFS.h" + +using namespace clang; +using namespace clang::clangd; +using namespace clang::tooling::dependencies; + +namespace { +class ModuleDependencyScannerTests : public ModuleTestSetup {}; + +TEST_F(ModuleDependencyScannerTests, SingleFile) { + addFile("foo.h", R"cpp( +import foo; + )cpp"); + + addFile("A.cppm", R"cpp( +module; +#include "foo.h" +export module A; +export import :partA; +import :partB; +import C; + +module :private; +import D; + )cpp"); + + MockCompilationDatabase CDB(TestDir); + CDB.ExtraClangFlags.push_back("-std=c++20"); + + ModuleDependencyScanner Scanner(CDB, TFS); + std::optional ScanningResult = + Scanner.scan(getFullPath("A.cppm")); + EXPECT_TRUE(ScanningResult); + + EXPECT_TRUE(ScanningResult->ModuleName); + EXPECT_EQ(*ScanningResult->ModuleName, "A"); + + EXPECT_EQ(ScanningResult->RequiredModules.size(), 5u); + EXPECT_EQ(ScanningResult->RequiredModules[0], "foo"); + EXPECT_EQ(ScanningResult->RequiredModules[1], "A:partA"); + EXPECT_EQ(ScanningResult->RequiredModules[2], "A:partB"); + EXPECT_EQ(ScanningResult->RequiredModules[3], "C"); + EXPECT_EQ(ScanningResult->RequiredModules[4], "D"); +} + +TEST_F(ModuleDependencyScannerTests, GlobalScanning) { + addFile("build/compile_commands.json", R"cpp( +[ +{ + "directory": "__DIR__", + "command": "clang++ -std=c++20 __DIR__/foo.cppm -fmodule-output=__DIR__/foo.pcm -c -o __DIR__/foo.o", + "file": "__DIR__/foo.cppm", + "output": "__DIR__/foo.o" +}, +{ + "directory": "__DIR__", + "command": "clang++ -std=c++20 __DIR__/C.cppm -fmodule-output=__DIR__/C.pcm -c -o __DIR__/C.o", + "file": "__DIR__/C.cppm", + "output": "__DIR__/C.o" +}, +{ + "directory": "__DIR__", + "command": "clang++ -std=c++20 __DIR__/D.cppm -fmodule-output=__DIR__/D.pcm -c -o __DIR__/D.o", + "file": "__DIR__/D.cppm", + "output": "__DIR__/D.o" +}, +{ + "directory": "__DIR__", + "command": "clang++ -std=c++20 __DIR__/A-partA.cppm -fmodule-file=foo=__DIR__/foo.pcm -fmodule-output=__DIR__/A-partA.pcm -c -o __DIR__/A-partA.o", + "file": "__DIR__/A-partA.cppm", + "output": "__DIR__/A-partA.o" +}, +{ + "directory": "__DIR__", + "command": "clang++ -std=c++20 __DIR__/A-partB.cppm -fmodule-file=C=__DIR__/C.pcm -fmodule-output=__DIR__/A-partB.pcm -c -o __DIR__/A-partB.o", + "file": "__DIR__/A-partB.cppm", + "output": "__DIR__/A-partB.o" +}, +{ + "directory": "__DIR__", + "command": "clang++ -std=c++20 __DIR__/A.cppm -fmodule-file=A:partB=__DIR__/A-partB.pcm -fmodule-file=A:partA=__DIR__/A-partA.pcm -fmodule-file=foo=__DIR__/foo.pcm -fmodule-file=C=__DIR__/C.pcm -fmodule-file=D=__DIR__/C.pcm -fmodule-output=__DIR__/A.pcm -c -o __DIR__/A.o", + "file": "__DIR__/A.cppm", + "output": "__DIR__/A.o" +}, +] + )cpp"); + + addFile("foo.cppm", R"cpp( +export module foo; + )cpp"); + + addFile("foo.h", R"cpp( +import foo; + )cpp"); + + addFile("A-partA.cppm", R"cpp( +export module A:partA; +import foo; + )cpp"); + + addFile("A-partB.cppm", R"cpp( +module A:partB; +import C; + )cpp"); + + addFile("C.cppm", R"cpp( +export module C; + )cpp"); + + addFile("D.cppm", R"cpp( +export module D; + )cpp"); + + addFile("A.cppm", R"cpp( +module; +#include "foo.h" +export module A; +export import :partA; +import :partB; +import C; + +module :private; +import D; + )cpp"); + + std::unique_ptr CDB = + getGlobalCompilationDatabase(); + ModuleDependencyScanner Scanner(*CDB.get(), TFS); + Scanner.globalScan({getFullPath("A.cppm"), getFullPath("foo.cppm"), + getFullPath("A-partA.cppm"), getFullPath("A-partB.cppm"), + getFullPath("C.cppm"), getFullPath("D.cppm")}); + + EXPECT_TRUE(Scanner.getSourceForModuleName("foo").endswith("foo.cppm")); + EXPECT_TRUE(Scanner.getSourceForModuleName("A").endswith("A.cppm")); + EXPECT_TRUE( + Scanner.getSourceForModuleName("A:partA").endswith("A-partA.cppm")); + EXPECT_TRUE( + Scanner.getSourceForModuleName("A:partB").endswith("A-partB.cppm")); + EXPECT_TRUE(Scanner.getSourceForModuleName("C").endswith("C.cppm")); + EXPECT_TRUE(Scanner.getSourceForModuleName("D").endswith("D.cppm")); + + EXPECT_TRUE(Scanner.getRequiredModules(getFullPath("foo.cppm")).empty()); + EXPECT_TRUE(Scanner.getRequiredModules(getFullPath("C.cppm")).empty()); + EXPECT_TRUE(Scanner.getRequiredModules(getFullPath("D.cppm")).empty()); + + EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A-partA.cppm")).size(), 1u); + EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A-partA.cppm"))[0], "foo"); + + EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A-partB.cppm")).size(), 1u); + EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A-partB.cppm"))[0], "C"); + + EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A.cppm")).size(), 5u); + EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A.cppm"))[0], "foo"); + EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A.cppm"))[1], "A:partA"); + EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A.cppm"))[2], "A:partB"); + EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A.cppm"))[3], "C"); + EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A.cppm"))[4], "D"); +} + +} // namespace + +#endif diff --git a/clang-tools-extra/clangd/unittests/ModulesTestSetup.h b/clang-tools-extra/clangd/unittests/ModulesTestSetup.h new file mode 100644 index 00000000000000..3f66c5ff80dc69 --- /dev/null +++ b/clang-tools-extra/clangd/unittests/ModulesTestSetup.h @@ -0,0 +1,103 @@ +//===-- ModulesTestSetup.h - Setup the module test environment --*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "Compiler.h" +#include "support/ThreadsafeFS.h" + +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/raw_ostream.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { +class ModuleTestSetup : public ::testing::Test { +protected: + void SetUp() override { + ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("modules-test", TestDir)); + } + + void TearDown() override { llvm::sys::fs::remove_directories(TestDir); } + +public: + // Add files to the working testing directory and repalce all the + // `__DIR__` to TestDir. + void addFile(StringRef Path, StringRef Contents) { + ASSERT_FALSE(llvm::sys::path::is_absolute(Path)); + + SmallString<256> AbsPath(TestDir); + llvm::sys::path::append(AbsPath, Path); + + ASSERT_FALSE(llvm::sys::fs::create_directories( + llvm::sys::path::parent_path(AbsPath))); + + std::error_code EC; + llvm::raw_fd_ostream OS(AbsPath, EC); + ASSERT_FALSE(EC); + + std::size_t Pos = Contents.find("__DIR__"); + while (Pos != llvm::StringRef::npos) { + OS << Contents.take_front(Pos); + OS << TestDir; + Contents = Contents.drop_front(Pos + sizeof("__DIR__") - 1); + Pos = Contents.find("__DIR__"); + } + + OS << Contents; + } + + // Get the absolute path for file specified by Path under testing working + // directory. + std::string getFullPath(StringRef Path) { + SmallString<128> Result(TestDir); + llvm::sys::path::append(Result, Path); + EXPECT_TRUE(llvm::sys::fs::exists(Result.str())); + return Result.str().str(); + } + + std::unique_ptr getGlobalCompilationDatabase() { + // The compilation flags with modules are much complex so it looks better + // to use DirectoryBasedGlobalCompilationDatabase than a mocked compilation + // database. + DirectoryBasedGlobalCompilationDatabase::Options Opts(TFS); + return std::make_unique(Opts); + } + + ParseInputs getInputs(StringRef FileName, + const GlobalCompilationDatabase &CDB) { + std::string FullPathName = getFullPath(FileName); + + ParseInputs Inputs; + std::optional Cmd = + CDB.getCompileCommand(FullPathName); + EXPECT_TRUE(Cmd); + Inputs.CompileCommand = std::move(*Cmd); + Inputs.TFS = &TFS; + + if (auto Contents = TFS.view(TestDir)->getBufferForFile(FullPathName)) + Inputs.Contents = Contents->get()->getBuffer().str(); + + return Inputs; + } + + std::unique_ptr + getCompilerInvocation(const ParseInputs &Inputs) { + std::vector CC1Args; + return buildCompilerInvocation(Inputs, DiagConsumer, &CC1Args); + } + + SmallString<256> TestDir; + // Noticed MockFS but its member variable 'OverlayRealFileSystemForModules' + // implies that it will better to use RealThreadsafeFS directly. + RealThreadsafeFS TFS; + + DiagnosticConsumer DiagConsumer; +}; +} // namespace clangd +} // namespace clang diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp index 500b72b9b327a0..800d8c5450cb05 100644 --- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp @@ -380,8 +380,10 @@ TEST(ParsedASTTest, PatchesAdditionalIncludes) { MockFS FS; auto Inputs = TU.inputs(FS); auto CI = buildCompilerInvocation(Inputs, Diags); + MockCompilationDatabase CDB; auto EmptyPreamble = - buildPreamble(testPath("foo.cpp"), *CI, Inputs, true, nullptr); + buildPreamble(testPath("foo.cpp"), *CI, Inputs, true, + /*RequiredModuleBuilder=*/nullptr, nullptr); ASSERT_TRUE(EmptyPreamble); EXPECT_THAT(EmptyPreamble->Includes.MainFileIncludes, IsEmpty()); @@ -422,8 +424,10 @@ TEST(ParsedASTTest, PatchesDeletedIncludes) { MockFS FS; auto Inputs = TU.inputs(FS); auto CI = buildCompilerInvocation(Inputs, Diags); + MockCompilationDatabase CDB; auto BaselinePreamble = - buildPreamble(testPath("foo.cpp"), *CI, Inputs, true, nullptr); + buildPreamble(testPath("foo.cpp"), *CI, Inputs, true, + /*RequiredModuleBuilder=*/nullptr, nullptr); ASSERT_TRUE(BaselinePreamble); EXPECT_THAT(BaselinePreamble->Includes.MainFileIncludes, ElementsAre(testing::Field(&Inclusion::Written, ""))); diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp index 6420516e785576..149ca7819947b3 100644 --- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp +++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp @@ -192,8 +192,10 @@ TEST(PreamblePatchTest, PatchesPreambleIncludes) { TU.AdditionalFiles["b.h"] = ""; TU.AdditionalFiles["c.h"] = ""; auto PI = TU.inputs(FS); - auto BaselinePreamble = buildPreamble( - TU.Filename, *buildCompilerInvocation(PI, Diags), PI, true, nullptr); + MockCompilationDatabase CDB; + auto BaselinePreamble = + buildPreamble(TU.Filename, *buildCompilerInvocation(PI, Diags), PI, true, + /*RequiredModuleBuilder=*/nullptr, nullptr); // We drop c.h from modified and add a new header. Since the latter is patched // we should only get a.h in preamble includes. d.h shouldn't be part of the // preamble, as it's coming from a disabled region. diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp new file mode 100644 index 00000000000000..9107d0e2b85a5e --- /dev/null +++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp @@ -0,0 +1,224 @@ +//===--------------- PrerequisiteModulesTests.cpp -------------------*- C++ +//-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +/// FIXME: Skip testing on windows temporarily due to the different escaping +/// code mode. +#ifndef _WIN32 + +#include "PrerequisiteModules.h" +#include "ModulesBuilder.h" +#include "ModulesTestSetup.h" + +using namespace clang; +using namespace clang::clangd; + +namespace { +class PrerequisiteModulesTests : public ModuleTestSetup {}; + +TEST_F(PrerequisiteModulesTests, PrerequisiteModulesTest) { + addFile("build/compile_commands.json", R"cpp( +[ +{ + "directory": "__DIR__", + "command": "clang++ -std=c++20 __DIR__/M.cppm -fmodule-output=__DIR__/M.pcm -c -o __DIR__/M.o", + "file": "__DIR__/M.cppm", + "output": "__DIR__/M.o" +}, +{ + "directory": "__DIR__", + "command": "clang++ -std=c++20 __DIR__/N.cppm -fmodule-file=M=__DIR__/M.pcm -fmodule-file=N:Part=__DIR__/N-partition.pcm -fprebuilt-module-path=__DIR__ -fmodule-output=__DIR__/N.pcm -c -o __DIR__/N.o", + "file": "__DIR__/N.cppm", + "output": "__DIR__/N.o" +}, +{ + "directory": "__DIR__", + "command": "clang++ -std=c++20 __DIR__/N-part.cppm -fmodule-output=__DIR__/N-partition.pcm -c -o __DIR__/N-part.o", + "file": "__DIR__/N-part.cppm", + "output": "__DIR__/N-part.o" +}, +{ + "directory": "__DIR__", + "command": "clang++ -std=c++20 __DIR__/L.cppm -fmodule-output=__DIR__/L.pcm -c -o __DIR__/L.o", + "file": "__DIR__/L.cppm", + "output": "__DIR__/L.o" +}, +{ + "directory": "__DIR__", + "command": "clang++ -std=c++20 __DIR__/NonModular.cpp -c -o __DIR__/NonModular.o", + "file": "__DIR__/NonModular.cpp", + "output": "__DIR__/NonModular.o" +} +] + )cpp"); + + addFile("foo.h", R"cpp( +inline void foo() {} + )cpp"); + + addFile("M.cppm", R"cpp( +module; +#include "foo.h" +export module M; + )cpp"); + + addFile("N.cppm", R"cpp( +export module N; +import :Part; +import M; + )cpp"); + + addFile("N-part.cppm", R"cpp( +// Different name with filename intentionally. +export module N:Part; + )cpp"); + + addFile("bar.h", R"cpp( +inline void bar() {} + )cpp"); + + addFile("L.cppm", R"cpp( +module; +#include "bar.h" +export module L; + )cpp"); + + addFile("NonModular.cpp", R"cpp( +#include "bar.h" +#include "foo.h" +void use() { + foo(); + bar(); +} + )cpp"); + + std::unique_ptr CDB = + getGlobalCompilationDatabase(); + EXPECT_TRUE(CDB); + ModulesBuilder Builder(*CDB.get()); + + // NonModular.cpp is not related to modules. So nothing should be built. + auto NonModularInfo = + Builder.buildPrerequisiteModulesFor(getFullPath("NonModular.cpp"), &TFS); + EXPECT_FALSE(NonModularInfo); + + auto MInfo = Builder.buildPrerequisiteModulesFor(getFullPath("M.cppm"), &TFS); + // buildPrerequisiteModulesFor won't built the module itself. + EXPECT_FALSE(MInfo); + + // Module N shouldn't be able to be built. + auto NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), &TFS); + EXPECT_TRUE(NInfo); + EXPECT_TRUE(NInfo->isModuleUnitBuilt("M")); + EXPECT_TRUE(NInfo->isModuleUnitBuilt("N:Part")); + + ParseInputs NInput = getInputs("N.cppm", *CDB); + std::vector CC1Args; + std::unique_ptr Invocation = + getCompilerInvocation(NInput); + // Test that `PrerequisiteModules::canReuse` works basically. + EXPECT_TRUE(NInfo->canReuse(*Invocation, TFS.view(TestDir))); + + // Test that we can still reuse the NInfo after we touch a unrelated file. + { + addFile("L.cppm", R"cpp( +module; +#include "bar.h" +export module L; +export int ll = 43; + )cpp"); + EXPECT_TRUE(NInfo->canReuse(*Invocation, TFS.view(TestDir))); + + addFile("bar.h", R"cpp( +inline void bar() {} +inline void bar(int) {} + )cpp"); + EXPECT_TRUE(NInfo->canReuse(*Invocation, TFS.view(TestDir))); + } + + // Test that we can't reuse the NInfo after we touch a related file. + { + addFile("M.cppm", R"cpp( +module; +#include "foo.h" +export module M; +export int mm = 44; + )cpp"); + EXPECT_FALSE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); + + NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), &TFS); + EXPECT_TRUE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); + + addFile("foo.h", R"cpp( +inline void foo() {} +inline void foo(int) {} + )cpp"); + EXPECT_FALSE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); + + NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), &TFS); + EXPECT_TRUE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); + } + + addFile("N-part.cppm", R"cpp( +export module N:Part; +// Intentioned to make it uncompilable. +export int NPart = 4LIdjwldijaw + )cpp"); + EXPECT_FALSE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); + NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), &TFS); + EXPECT_TRUE(NInfo); + // So NInfo should be unreusable even after rebuild. + EXPECT_FALSE(NInfo->canReuse(*Invocation, TFS.view(TestDir))); + + addFile("N-part.cppm", R"cpp( +export module N:Part; +export int NPart = 43; + )cpp"); + EXPECT_TRUE(NInfo); + EXPECT_FALSE(NInfo->canReuse(*Invocation, TFS.view(TestDir))); + NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), &TFS); + // So NInfo should be unreusable even after rebuild. + EXPECT_TRUE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); + + // Test that if we changed the modification time of the file, the module files + // info is still reusable if its content doesn't change. + addFile("N-part.cppm", R"cpp( +export module N:Part; +export int NPart = 43; + )cpp"); + EXPECT_TRUE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); + + addFile("N.cppm", R"cpp( +export module N; +import :Part; +import M; + +export int nn = 43; + )cpp"); + // NInfo should be reusable after we change its content. + EXPECT_TRUE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); + + { + // Check that + // `PrerequisiteModules::adjustHeaderSearchOptions(HeaderSearchOptions&)` + // can replace HeaderSearchOptions correctly. + ParseInputs NInput = getInputs("N.cppm", *CDB); + std::vector CC1Args; + std::unique_ptr NInvocation = + getCompilerInvocation(NInput); + HeaderSearchOptions &HSOpts = NInvocation->getHeaderSearchOpts(); + NInfo->adjustHeaderSearchOptions(HSOpts); + + EXPECT_TRUE(HSOpts.PrebuiltModuleFiles.count("M")); + EXPECT_TRUE(HSOpts.PrebuiltModuleFiles.count("N:Part")); + } +} + +} // namespace + +#endif diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp index e65ae825b41677..0d852d916fd9dd 100644 --- a/clang-tools-extra/clangd/unittests/TestTU.cpp +++ b/clang-tools-extra/clangd/unittests/TestTU.cpp @@ -107,8 +107,11 @@ TestTU::preamble(PreambleParsedCallback PreambleCallback) const { initializeModuleCache(*CI); auto ModuleCacheDeleter = llvm::make_scope_exit( std::bind(deleteModuleCache, CI->getHeaderSearchOpts().ModuleCachePath)); + MockCompilationDatabase CDB; return clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs, - /*StoreInMemory=*/true, PreambleCallback); + /*StoreInMemory=*/true, + /*RequiredModuleBuilder=*/nullptr, + PreambleCallback); } ParsedAST TestTU::build() const { @@ -123,9 +126,12 @@ ParsedAST TestTU::build() const { auto ModuleCacheDeleter = llvm::make_scope_exit( std::bind(deleteModuleCache, CI->getHeaderSearchOpts().ModuleCachePath)); - auto Preamble = clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs, - /*StoreInMemory=*/true, - /*PreambleCallback=*/nullptr); + MockCompilationDatabase CDB; + auto Preamble = + clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs, + /*StoreInMemory=*/true, + /*RequiredModuleBuilder=*/nullptr, + /*PreambleCallback=*/nullptr); auto AST = ParsedAST::build(testPath(Filename), Inputs, std::move(CI), Diags.take(), Preamble); if (!AST) { diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ecfb3aa9267f14..93578ffbaf89d5 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -48,6 +48,9 @@ Major New Features Improvements to clangd ---------------------- +- Introduced exmperimental support for C++20 Modules. The experimental support can + be enabled by `-experimental-modules-support` option. + Inlay hints ^^^^^^^^^^^ From a957a79427c07a9d4ec5850e93ba8d5934ba7b67 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Fri, 24 May 2024 15:17:50 +0800 Subject: [PATCH 2/6] Address comments --- clang-tools-extra/clangd/CMakeLists.txt | 1 - clang-tools-extra/clangd/ClangdLSPServer.cpp | 8 +- clang-tools-extra/clangd/ClangdLSPServer.h | 2 + clang-tools-extra/clangd/ClangdServer.cpp | 5 +- clang-tools-extra/clangd/ClangdServer.h | 7 +- clang-tools-extra/clangd/Compiler.h | 4 + .../clangd/GlobalCompilationDatabase.cpp | 7 +- .../clangd/GlobalCompilationDatabase.h | 6 +- .../clangd/ModuleDependencyScanner.cpp | 81 ----- .../clangd/ModuleDependencyScanner.h | 106 ------ clang-tools-extra/clangd/ModulesBuilder.cpp | 329 ++++++++++-------- clang-tools-extra/clangd/ModulesBuilder.h | 68 +++- clang-tools-extra/clangd/Preamble.h | 1 - .../clangd/PrerequisiteModules.h | 87 ----- clang-tools-extra/clangd/ProjectModules.cpp | 158 ++++++++- clang-tools-extra/clangd/ProjectModules.h | 10 +- clang-tools-extra/clangd/TUScheduler.cpp | 46 +-- clang-tools-extra/clangd/TUScheduler.h | 4 - .../clangd/unittests/CMakeLists.txt | 1 - .../unittests/ModuleDependencyScannerTest.cpp | 176 ---------- .../clangd/unittests/ModulesTestSetup.h | 6 +- .../unittests/PrerequisiteModulesTest.cpp | 1 - clang-tools-extra/docs/ReleaseNotes.rst | 3 +- 23 files changed, 447 insertions(+), 670 deletions(-) delete mode 100644 clang-tools-extra/clangd/ModuleDependencyScanner.cpp delete mode 100644 clang-tools-extra/clangd/ModuleDependencyScanner.h delete mode 100644 clang-tools-extra/clangd/PrerequisiteModules.h delete mode 100644 clang-tools-extra/clangd/unittests/ModuleDependencyScannerTest.cpp diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt index b5a15d6464fdc2..393aa795bda05d 100644 --- a/clang-tools-extra/clangd/CMakeLists.txt +++ b/clang-tools-extra/clangd/CMakeLists.txt @@ -97,7 +97,6 @@ add_clang_library(clangDaemon IncludeFixer.cpp InlayHints.cpp JSONTransport.cpp - ModuleDependencyScanner.cpp ModulesBuilder.cpp PathMapping.cpp ProjectModules.cpp diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 7fd599d4e1a0b0..c99f172dac4e42 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -14,6 +14,7 @@ #include "Feature.h" #include "GlobalCompilationDatabase.h" #include "LSPBinder.h" +#include "ModulesBuilder.h" #include "Protocol.h" #include "SemanticHighlighting.h" #include "SourceCode.h" @@ -563,6 +564,10 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params, Mangler.ResourceDir = *Opts.ResourceDir; CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags, std::move(Mangler)); + + if (Opts.ExperimentalModulesSupport) + ModulesManager.emplace(*CDB); + { // Switch caller's context with LSPServer's background context. Since we // rather want to propagate information from LSPServer's context into the @@ -572,7 +577,8 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params, if (Opts.Encoding) WithOffsetEncoding.emplace(kCurrentOffsetEncoding, *Opts.Encoding); Server.emplace(*CDB, TFS, Opts, - static_cast(this)); + static_cast(this), + ModulesManager ? &*ModulesManager : nullptr); } llvm::json::Object ServerCaps{ diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h index 8bcb29522509b7..0d2fea31cf5101 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -323,6 +323,8 @@ class ClangdLSPServer : private ClangdServer::Callbacks, std::optional CDB; // The ClangdServer is created by the "initialize" LSP method. std::optional Server; + // Manages to build module files. + std::optional ModulesManager; }; } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index af3fedd0a2db72..d1017828e987cc 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -214,10 +214,10 @@ ClangdServer::Options::operator TUScheduler::Options() const { ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS, const Options &Opts, - Callbacks *Callbacks) + Callbacks *Callbacks, ModulesBuilder *ModulesManager) : FeatureModules(Opts.FeatureModules), CDB(CDB), TFS(TFS), DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr), - ClangTidyProvider(Opts.ClangTidyProvider), + ModulesManager(ModulesManager), ClangTidyProvider(Opts.ClangTidyProvider), UseDirtyHeaders(Opts.UseDirtyHeaders), LineFoldingOnly(Opts.LineFoldingOnly), PreambleParseForwardingFunctions(Opts.PreambleParseForwardingFunctions), @@ -310,6 +310,7 @@ void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents, Inputs.Index = Index; Inputs.ClangTidyProvider = ClangTidyProvider; Inputs.FeatureModules = FeatureModules; + Inputs.ModulesManager = ModulesManager; bool NewFile = WorkScheduler->update(File, Inputs, WantDiags); // If we loaded Foo.h, we want to make sure Foo.cpp is indexed. if (NewFile && BackgroundIdx) diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index 84023b2510c02d..67b8184e1bcded 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -42,6 +42,8 @@ namespace clang { namespace clangd { + +class ModulesBuilder; /// Manages a collection of source files and derived data (ASTs, indexes), /// and provides language-aware features such as code completion. /// @@ -202,7 +204,8 @@ class ClangdServer { /// those arguments for subsequent reparses. However, ClangdServer will check /// if compilation arguments changed on calls to forceReparse(). ClangdServer(const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS, - const Options &Opts, Callbacks *Callbacks = nullptr); + const Options &Opts, Callbacks *Callbacks = nullptr, + ModulesBuilder *ModulesManager = nullptr); ~ClangdServer(); /// Gets the installed feature module of a given type, if any. @@ -480,6 +483,8 @@ class ClangdServer { std::unique_ptr BackgroundIdx; // Storage for merged views of the various indexes. std::vector> MergedIdx; + // Manage module files. + ModulesBuilder *ModulesManager = nullptr; // When set, provides clang-tidy options for a specific file. TidyProviderRef ClangTidyProvider; diff --git a/clang-tools-extra/clangd/Compiler.h b/clang-tools-extra/clangd/Compiler.h index 56c2567ebe97b0..f4b377bd537a1d 100644 --- a/clang-tools-extra/clangd/Compiler.h +++ b/clang-tools-extra/clangd/Compiler.h @@ -44,6 +44,8 @@ struct ParseOptions { bool ImportInsertions = false; }; +class ModulesBuilder; + /// Information required to run clang, e.g. to parse AST or do code completion. struct ParseInputs { tooling::CompileCommand CompileCommand; @@ -60,6 +62,8 @@ struct ParseInputs { TidyProviderRef ClangTidyProvider = {}; // Used to acquire ASTListeners when parsing files. FeatureModuleSet *FeatureModules = nullptr; + // Used to build and manage (C++) modules. + ModulesBuilder *ModulesManager = nullptr; }; /// Clears \p CI from options that are not supported by clangd, like codegen or diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp index 4e1d4f11e2fdf4..3da770709ec93e 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp @@ -741,7 +741,7 @@ DirectoryBasedGlobalCompilationDatabase::getProjectInfo(PathRef File) const { return Res->PI; } -std::shared_ptr +std::unique_ptr DirectoryBasedGlobalCompilationDatabase::getProjectModules(PathRef File) const { CDBLookupRequest Req; Req.FileName = File; @@ -751,6 +751,9 @@ DirectoryBasedGlobalCompilationDatabase::getProjectModules(PathRef File) const { auto Res = lookupCDB(Req); if (!Res) return {}; + // FIXME: Passing *this here means we won't use outer OverlayCDB, which + // (among other things) has the mechanism for detecting and injecting + // resource-dir. return ProjectModules::create( ProjectModules::ProjectModulesKind::ScanningAllFiles, Res->CDB->getAllFiles(), *this, Opts.TFS); @@ -848,7 +851,7 @@ std::optional DelegatingCDB::getProjectInfo(PathRef File) const { return Base->getProjectInfo(File); } -std::shared_ptr +std::unique_ptr DelegatingCDB::getProjectModules(PathRef File) const { if (!Base) return nullptr; diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.h b/clang-tools-extra/clangd/GlobalCompilationDatabase.h index 1bd6324c4cd8e4..1316f49a473c04 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.h +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.h @@ -48,7 +48,7 @@ class GlobalCompilationDatabase { } /// Get the modules in the closest project to \p File - virtual std::shared_ptr + virtual std::unique_ptr getProjectModules(PathRef File) const { return nullptr; } @@ -84,7 +84,7 @@ class DelegatingCDB : public GlobalCompilationDatabase { std::optional getProjectInfo(PathRef File) const override; - std::shared_ptr + std::unique_ptr getProjectModules(PathRef File) const override; tooling::CompileCommand getFallbackCommand(PathRef File) const override; @@ -133,7 +133,7 @@ class DirectoryBasedGlobalCompilationDatabase /// \p File's parents. std::optional getProjectInfo(PathRef File) const override; - std::shared_ptr + std::unique_ptr getProjectModules(PathRef File) const override; bool blockUntilIdle(Deadline Timeout) const override; diff --git a/clang-tools-extra/clangd/ModuleDependencyScanner.cpp b/clang-tools-extra/clangd/ModuleDependencyScanner.cpp deleted file mode 100644 index 3413d0bb0eaa59..00000000000000 --- a/clang-tools-extra/clangd/ModuleDependencyScanner.cpp +++ /dev/null @@ -1,81 +0,0 @@ -//===---------------- ModuleDependencyScanner.cpp ----------------*- C++-*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "ModuleDependencyScanner.h" -#include "support/Logger.h" - -namespace clang { -namespace clangd { - -std::optional -ModuleDependencyScanner::scan(PathRef FilePath) { - std::optional Cmd = CDB.getCompileCommand(FilePath); - - if (!Cmd) - return std::nullopt; - - using namespace clang::tooling::dependencies; - - llvm::SmallString<128> FilePathDir(FilePath); - llvm::sys::path::remove_filename(FilePathDir); - DependencyScanningTool ScanningTool(Service, TFS.view(FilePathDir)); - - llvm::Expected ScanningResult = - ScanningTool.getP1689ModuleDependencyFile(*Cmd, Cmd->Directory); - - if (auto E = ScanningResult.takeError()) { - log("Scanning modules dependencies for {0} failed: {1}", FilePath, - llvm::toString(std::move(E))); - return std::nullopt; - } - - ModuleDependencyInfo Result; - - if (ScanningResult->Provides) { - ModuleNameToSource[ScanningResult->Provides->ModuleName] = FilePath; - Result.ModuleName = ScanningResult->Provides->ModuleName; - } - - for (auto &Required : ScanningResult->Requires) - Result.RequiredModules.push_back(Required.ModuleName); - - return Result; -} - -void ModuleDependencyScanner::globalScan( - const std::vector &AllFiles) { - for (auto &File : AllFiles) - scan(File); - - GlobalScanned = true; -} - -PathRef -ModuleDependencyScanner::getSourceForModuleName(StringRef ModuleName) const { - assert( - GlobalScanned && - "We should only call getSourceForModuleName after calling globalScan()"); - - if (auto It = ModuleNameToSource.find(ModuleName); - It != ModuleNameToSource.end()) - return It->second; - - return {}; -} - -std::vector -ModuleDependencyScanner::getRequiredModules(PathRef File) { - auto ScanningResult = scan(File); - if (!ScanningResult) - return {}; - - return ScanningResult->RequiredModules; -} - -} // namespace clangd -} // namespace clang diff --git a/clang-tools-extra/clangd/ModuleDependencyScanner.h b/clang-tools-extra/clangd/ModuleDependencyScanner.h deleted file mode 100644 index 5251bdeadfb851..00000000000000 --- a/clang-tools-extra/clangd/ModuleDependencyScanner.h +++ /dev/null @@ -1,106 +0,0 @@ -//===-------------- ModuleDependencyScanner.h --------------------*- C++-*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULEDEPENDENCYSCANNER_H - -#include "GlobalCompilationDatabase.h" -#include "support/Path.h" -#include "support/ThreadsafeFS.h" - -#include "clang/Tooling/DependencyScanning/DependencyScanningService.h" -#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h" - -#include "llvm/ADT/SmallString.h" -#include "llvm/ADT/StringMap.h" - -namespace clang { -namespace clangd { - -/// A scanner to query the dependency information for C++20 Modules. -/// -/// The scanner can scan a single file with `scan(PathRef)` member function -/// or scan the whole project with `globalScan(PathRef)` member function. See -/// the comments of `globalScan` to see the details. -/// -/// ModuleDependencyScanner should only be used via ScanningAllProjectModules. -/// -/// The ModuleDependencyScanner can get the directly required module name for a -/// specific source file. Also the ModuleDependencyScanner can get the source -/// file declaring a specific module name. -/// -/// IMPORTANT NOTE: we assume that every module unit is only declared once in a -/// source file in the project. But the assumption is not strictly true even -/// besides the invalid projects. The language specification requires that every -/// module unit should be unique in a valid program. But a project can contain -/// multiple programs. Then it is valid that we can have multiple source files -/// declaring the same module in a project as long as these source files don't -/// interere with each other.` -class ModuleDependencyScanner { -public: - ModuleDependencyScanner(const GlobalCompilationDatabase &CDB, - const ThreadsafeFS &TFS) - : CDB(CDB), TFS(TFS), - Service(tooling::dependencies::ScanningMode::CanonicalPreprocessing, - tooling::dependencies::ScanningOutputFormat::P1689) {} - - // The scanned modules dependency information for a specific source file. - struct ModuleDependencyInfo { - // The name of the module if the file is a module unit. - std::optional ModuleName; - // A list of names for the modules that the file directly depends. - std::vector RequiredModules; - }; - - /// Scanning the single file specified by \param FilePath. - /// NOTE: This is only used by unittests for external uses. - std::optional scan(PathRef FilePath); - - /// Scanning every source file in the current project to get the - /// to map. - /// It looks unefficiency to scan the whole project especially for - /// every version of every file! - /// TODO: We should find an efficient method to get the - /// to map. We can make it either by providing - /// a global module dependency scanner to monitor every file. Or we - /// can simply require the build systems (or even if the end users) - /// to provide the map. - void globalScan(const std::vector &AllFiles); - bool isGlobalScanned() const { return GlobalScanned; } - - /// Get the source file from the module name. Note that the language - /// guarantees all the module names are unique in a valid program. - /// This function should only be called after globalScan. - /// - /// FIXME: We should handle the case that there are multiple source files - /// declaring the same module. - PathRef getSourceForModuleName(StringRef ModuleName) const; - - /// Return the direct required modules. Indirect required modules are not - /// included. - std::vector getRequiredModules(PathRef File); - -private: - const GlobalCompilationDatabase &CDB; - const ThreadsafeFS &TFS; - - // Whether the scanner has scanned the project globally. - bool GlobalScanned = false; - - clang::tooling::dependencies::DependencyScanningService Service; - - // TODO: Add a scanning cache. - - // Map module name to source file path. - llvm::StringMap ModuleNameToSource; -}; - -} // namespace clangd -} // namespace clang - -#endif diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 5c56f2fc31d399..83c3b3856cb3ef 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -7,53 +7,65 @@ //===----------------------------------------------------------------------===// #include "ModulesBuilder.h" -#include "PrerequisiteModules.h" + +#include "Compiler.h" #include "support/Logger.h" #include "clang/Frontend/FrontendAction.h" #include "clang/Frontend/FrontendActions.h" +#include "clang/Serialization/ASTReader.h" + namespace clang { namespace clangd { namespace { -/// Get or create a path to store module files. Generally it should be: -/// -/// project_root/.cache/clangd/module_files/{RequiredPrefixDir}/. -/// -/// \param MainFile is used to get the root of the project from global -/// compilation database. \param RequiredPrefixDir is used to get the user -/// defined prefix for module files. This is useful when we want to seperate -/// module files. e.g., we want to build module files for the same module unit -/// `a.cppm` with 2 different users `b.cpp` and `c.cpp` and we don't want the -/// module file for `b.cpp` be conflict with the module files for `c.cpp`. Then -/// we can put the 2 module files into different dirs like: -/// -/// project_root/.cache/clangd/module_files/b.cpp/a.pcm -/// project_root/.cache/clangd/module_files/c.cpp/a.pcm -llvm::SmallString<256> getModuleFilesPath(PathRef MainFile, - const GlobalCompilationDatabase &CDB, - StringRef RequiredPrefixDir) { - std::optional PI = CDB.getProjectInfo(MainFile); - if (!PI) - return {}; +// Create a path to store module files. Generally it should be: +// +// {TEMP_DIRS}/clangd/module_files/{PrefixDir}-%%-%%-%%-%%-%%-%%/. +// +// {TEMP_DIRS} is the temporary directory for the system, e.g., "/var/tmp" +// or "C:/TEMP". +// +// '%%' means random value to make the generated path unique. +// +// \param MainFile is used to get the root of the project from global +// compilation database. \param PrefixDir is used to get the user +// defined prefix for module files. This is useful when we want to seperate +// module files. e.g., we want to build module files for the same module unit +// `a.cppm` with 2 different users `b.cpp` and `c.cpp` and we don't want the +// module file for `b.cpp` be conflict with the module files for `c.cpp`. Then +// we can put the 2 module files into different dirs like: +// +// ${TEMP_DIRS}/clangd/module_files/b.cpp/a.pcm +// ${TEMP_DIRS}/clangd/module_files/c.cpp/a.pcm +// +// TODO: Move these module fils out of the temporary directory if the module +// files are persistent. +llvm::SmallString<256> getUniqueModuleFilesPath(PathRef MainFile, + llvm::StringRef PrefixDir) { + llvm::SmallString<256> ResultPattern; - // FIXME: PI->SourceRoot may be empty, depending on the CDB strategy. - llvm::SmallString<256> Result(PI->SourceRoot); + llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true, + ResultPattern); - llvm::sys::path::append(Result, ".cache"); - llvm::sys::path::append(Result, "clangd"); - llvm::sys::path::append(Result, "module_files"); + llvm::sys::path::append(ResultPattern, "clangd"); + llvm::sys::path::append(ResultPattern, "module_files"); - llvm::sys::path::append(Result, RequiredPrefixDir); + llvm::sys::path::append(ResultPattern, PrefixDir); - llvm::sys::fs::create_directories(Result, /*IgnoreExisting=*/true); + ResultPattern.append("-%%-%%-%%-%%-%%-%%"); + llvm::SmallString<256> Result; + llvm::sys::fs::createUniquePath(ResultPattern, Result, + /*MakeAbsolute=*/false); + + llvm::sys::fs::create_directories(Result); return Result; } -/// Get the absolute path for the filename from the compile command. +// Get the absolute path for the filename from the compile command. llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand &Cmd) { llvm::SmallString<128> AbsolutePath; if (llvm::sys::path::is_absolute(Cmd.Filename)) { @@ -66,9 +78,9 @@ llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand &Cmd) { return AbsolutePath; } -/// Get a unique module file path under \param ModuleFilesPrefix. -std::string getUniqueModuleFilePath(StringRef ModuleName, - PathRef ModuleFilesPrefix) { +// Get a unique module file path under \param ModuleFilesPrefix. +std::string getModuleFilePath(llvm::StringRef ModuleName, + PathRef ModuleFilesPrefix) { llvm::SmallString<256> ModuleFilePattern(ModuleFilesPrefix); auto [PrimaryModuleName, PartitionName] = ModuleName.split(':'); llvm::sys::path::append(ModuleFilePattern, PrimaryModuleName); @@ -77,128 +89,49 @@ std::string getUniqueModuleFilePath(StringRef ModuleName, ModuleFilePattern.append(PartitionName); } - ModuleFilePattern.append("-%%-%%-%%-%%-%%-%%"); ModuleFilePattern.append(".pcm"); llvm::SmallString<256> ModuleFilePath; llvm::sys::fs::createUniquePath(ModuleFilePattern, ModuleFilePath, /*MakeAbsolute=*/false); - return (std::string)ModuleFilePath; + return std::string(ModuleFilePath); } } // namespace -bool ModulesBuilder::buildModuleFile(StringRef ModuleName, - const ThreadsafeFS *TFS, - std::shared_ptr MDB, - PathRef ModuleFilesPrefix, - PrerequisiteModules &BuiltModuleFiles) { - if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) - return true; - - PathRef ModuleUnitFileName = MDB->getSourceForModuleName(ModuleName); - /// It is possible that we're meeting third party modules (modules whose - /// source are not in the project. e.g, the std module may be a third-party - /// module for most project) or something wrong with the implementation of - /// ProjectModules. - /// FIXME: How should we treat third party modules here? If we want to ignore - /// third party modules, we should return true instead of false here. - /// Currently we simply bail out. - if (ModuleUnitFileName.empty()) - return false; - - for (auto &RequiredModuleName : MDB->getRequiredModules(ModuleUnitFileName)) { - // Return early if there are errors building the module file. - if (!buildModuleFile(RequiredModuleName, TFS, MDB, ModuleFilesPrefix, - BuiltModuleFiles)) { - log("Failed to build module {0}", RequiredModuleName); - return false; - } - } - - auto Cmd = CDB.getCompileCommand(ModuleUnitFileName); - if (!Cmd) - return false; - - std::string ModuleFileName = - getUniqueModuleFilePath(ModuleName, ModuleFilesPrefix); - Cmd->Output = ModuleFileName; - - ParseInputs Inputs; - Inputs.TFS = TFS; - Inputs.CompileCommand = std::move(*Cmd); - - IgnoreDiagnostics IgnoreDiags; - auto CI = buildCompilerInvocation(Inputs, IgnoreDiags); - if (!CI) - return false; - - auto FS = Inputs.TFS->view(Inputs.CompileCommand.Directory); - auto AbsolutePath = getAbsolutePath(Inputs.CompileCommand); - auto Buf = FS->getBufferForFile(AbsolutePath); - if (!Buf) - return false; - - // Hash the contents of input files and store the hash value to the BMI files. - // So that we can check if the files are still valid when we want to reuse the - // BMI files. - CI->getHeaderSearchOpts().ValidateASTInputFilesContent = true; - - BuiltModuleFiles.adjustHeaderSearchOptions(CI->getHeaderSearchOpts()); - - CI->getFrontendOpts().OutputFile = Inputs.CompileCommand.Output; - auto Clang = - prepareCompilerInstance(std::move(CI), /*Preamble=*/nullptr, - std::move(*Buf), std::move(FS), IgnoreDiags); - if (!Clang) - return false; - - GenerateModuleInterfaceAction Action; - Clang->ExecuteAction(Action); - - if (Clang->getDiagnostics().hasErrorOccurred()) - return false; - - BuiltModuleFiles.addModuleFile(ModuleName, ModuleFileName); - return true; -} - -/// FailedPrerequisiteModules - stands for the PrerequisiteModules which has -/// errors happened during the building process. +// FailedPrerequisiteModules - stands for the PrerequisiteModules which has +// errors happened during the building process. class FailedPrerequisiteModules : public PrerequisiteModules { public: ~FailedPrerequisiteModules() override = default; - /// We shouldn't adjust the compilation commands based on - /// FailedPrerequisiteModules. + // We shouldn't adjust the compilation commands based on + // FailedPrerequisiteModules. void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override { } - /// FailedPrerequisiteModules can never be reused. + // FailedPrerequisiteModules can never be reused. bool canReuse(const CompilerInvocation &CI, llvm::IntrusiveRefCntPtr) const override { return false; } - /// No module unit got built in FailedPrerequisiteModules. - bool isModuleUnitBuilt(StringRef ModuleName) const override { return false; } - - /// We shouldn't add any module files to the FailedPrerequisiteModules. - void addModuleFile(StringRef ModuleName, StringRef ModuleFilePath) override { - assert(false && "We shouldn't operator based on failed module files"); + // No module unit got built in FailedPrerequisiteModules. + bool isModuleUnitBuilt(llvm::StringRef ModuleName) const override { + return false; } }; -/// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all -/// the required modules are built successfully. All the module files -/// are owned by the StandalonePrerequisiteModules class. -/// -/// All the built module files won't be shared with other instances of the -/// class. So that we can avoid worrying thread safety. -/// -/// We don't need to worry about duplicated module names here since the standard -/// guarantees the module names should be unique to a program. +// StandalonePrerequisiteModules - stands for PrerequisiteModules for which all +// the required modules are built successfully. All the module files +// are owned by the StandalonePrerequisiteModules class. +// +// Any of the built module files won't be shared with other instances of the +// class. So that we can avoid worrying thread safety. +// +// We don't need to worry about duplicated module names here since the standard +// guarantees the module names should be unique to a program. class StandalonePrerequisiteModules : public PrerequisiteModules { public: StandalonePrerequisiteModules() = default; @@ -222,24 +155,19 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { bool canReuse(const CompilerInvocation &CI, llvm::IntrusiveRefCntPtr) const override; - bool isModuleUnitBuilt(StringRef ModuleName) const override { - constexpr unsigned SmallSizeThreshold = 8; - if (RequiredModules.size() < SmallSizeThreshold) - return llvm::any_of(RequiredModules, [&](auto &MF) { - return MF.ModuleName == ModuleName; - }); - + bool isModuleUnitBuilt(llvm::StringRef ModuleName) const override { return BuiltModuleNames.contains(ModuleName); } - void addModuleFile(StringRef ModuleName, StringRef ModuleFilePath) override { + void addModuleFile(llvm::StringRef ModuleName, + llvm::StringRef ModuleFilePath) { RequiredModules.emplace_back(ModuleName, ModuleFilePath); BuiltModuleNames.insert(ModuleName); } private: struct ModuleFile { - ModuleFile(StringRef ModuleName, PathRef ModuleFilePath) + ModuleFile(llvm::StringRef ModuleName, PathRef ModuleFilePath) : ModuleName(ModuleName.str()), ModuleFilePath(ModuleFilePath.str()) {} ModuleFile() = delete; @@ -264,14 +192,108 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { }; llvm::SmallVector RequiredModules; - /// A helper class to speedup the query if a module is built. + // A helper class to speedup the query if a module is built. llvm::StringSet<> BuiltModuleNames; }; +namespace { +// Build a module file for module with `ModuleName`. Return false +// when there are problem happens. Return true when the +// module file exists or built successfully. The information of built +// module file are stored in \param BuiltModuleFiles. +bool buildModuleFile(llvm::StringRef ModuleName, + const GlobalCompilationDatabase &CDB, + const ThreadsafeFS *TFS, ProjectModules *MDB, + PathRef ModuleFilesPrefix, + StandalonePrerequisiteModules &BuiltModuleFiles) { + if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) + return true; + + PathRef ModuleUnitFileName = MDB->getSourceForModuleName(ModuleName); + // It is possible that we're meeting third party modules (modules whose + // source are not in the project. e.g, the std module may be a third-party + // module for most projects) or something wrong with the implementation of + // ProjectModules. + // FIXME: How should we treat third party modules here? If we want to ignore + // third party modules, we should return true instead of false here. + // Currently we simply bail out. + if (ModuleUnitFileName.empty()) { + elog("Failed to get the source for module name {0}. Maybe it is from third " + "party libraries or something goes wrong", + ModuleName); + return false; + } + + for (auto &RequiredModuleName : MDB->getRequiredModules(ModuleUnitFileName)) { + // Return early if there are errors building the module file. + if (!buildModuleFile(RequiredModuleName, CDB, TFS, MDB, ModuleFilesPrefix, + BuiltModuleFiles)) { + elog("Failed to build {0} since failed to build {1}", ModuleName, + RequiredModuleName); + return false; + } + } + + auto Cmd = CDB.getCompileCommand(ModuleUnitFileName); + if (!Cmd) { + elog("Failed to get compile command for {0}", ModuleName); + return false; + } + + Cmd->Output = getModuleFilePath(ModuleName, ModuleFilesPrefix); + + ParseInputs Inputs; + Inputs.TFS = TFS; + Inputs.CompileCommand = std::move(*Cmd); + + IgnoreDiagnostics IgnoreDiags; + auto CI = buildCompilerInvocation(Inputs, IgnoreDiags); + if (!CI) { + elog("Failed to build compiler invocation for {0}", ModuleName); + return false; + } + + auto FS = Inputs.TFS->view(Inputs.CompileCommand.Directory); + auto AbsolutePath = getAbsolutePath(Inputs.CompileCommand); + auto Buf = FS->getBufferForFile(AbsolutePath); + if (!Buf) { + elog("Failed to create buffer for {0}", AbsolutePath); + return false; + } + + // Hash the contents of input files and store the hash value to the BMI files. + // So that we can check if the files are still valid when we want to reuse the + // BMI files. + CI->getHeaderSearchOpts().ValidateASTInputFilesContent = true; + + BuiltModuleFiles.adjustHeaderSearchOptions(CI->getHeaderSearchOpts()); + + CI->getFrontendOpts().OutputFile = Inputs.CompileCommand.Output; + auto Clang = + prepareCompilerInstance(std::move(CI), /*Preamble=*/nullptr, + std::move(*Buf), std::move(FS), IgnoreDiags); + if (!Clang) { + elog("Failed to prepare compiler instance for {0}", AbsolutePath); + return false; + } + + GenerateModuleInterfaceAction Action; + Clang->ExecuteAction(Action); + + if (Clang->getDiagnostics().hasErrorOccurred()) { + elog("Compilation for {0} failed", AbsolutePath); + return false; + } + + BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); + return true; +} +} // namespace + std::unique_ptr ModulesBuilder::buildPrerequisiteModulesFor(PathRef File, - const ThreadsafeFS *TFS) { - std::shared_ptr MDB = CDB.getProjectModules(File); + const ThreadsafeFS *TFS) const { + std::unique_ptr MDB = CDB.getProjectModules(File); if (!MDB) return {}; @@ -279,18 +301,25 @@ ModulesBuilder::buildPrerequisiteModulesFor(PathRef File, if (RequiredModuleNames.empty()) return {}; + llvm::SmallString<128> Prefix = llvm::sys::path::filename(File); + // There might be multiple files with the same name in a project. So appending + // the hash value of the full path to make sure they won't conflict. + Prefix += std::to_string(llvm::hash_value(File)); llvm::SmallString<256> ModuleFilesPrefix = - getModuleFilesPath(File, CDB, llvm::sys::path::filename(File)); + getUniqueModuleFilesPath(File, Prefix); + + log("Trying to build required modules for {0} in {1}", File, + ModuleFilesPrefix); auto RequiredModules = std::make_unique(); - for (const std::string &RequiredModuleName : RequiredModuleNames) + for (llvm::StringRef RequiredModuleName : RequiredModuleNames) // Return early if there is any error. - if (!buildModuleFile(RequiredModuleName, TFS, MDB, ModuleFilesPrefix, - *RequiredModules.get())) { - log("Failed to build module {0}", RequiredModuleName); + if (!buildModuleFile(RequiredModuleName, CDB, TFS, MDB.get(), + ModuleFilesPrefix, *RequiredModules.get())) return std::make_unique(); - } + + log("Built required modules for {0} in {1}", File, ModuleFilesPrefix); return std::move(RequiredModules); } @@ -321,13 +350,15 @@ bool StandalonePrerequisiteModules::canReuse( Clang.createASTReader(); for (auto &RequiredModule : RequiredModules) { - StringRef BMIPath = RequiredModule.ModuleFilePath; + llvm::StringRef BMIPath = RequiredModule.ModuleFilePath; + // TODO: Loading BMI fully is too heavy considering something cheaply to + // check if we can reuse the BMI. auto ReadResult = Clang.getASTReader()->ReadAST(BMIPath, serialization::MK_MainFile, SourceLocation(), ASTReader::ARR_None); if (ReadResult != ASTReader::Success) { - log("Failed to reuse {0}", BMIPath); + elog("Failed to reuse {0} due to {1}", BMIPath, ReadResult); return false; } } diff --git a/clang-tools-extra/clangd/ModulesBuilder.h b/clang-tools-extra/clangd/ModulesBuilder.h index ae7fc8778be676..1a67d3e1db028e 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.h +++ b/clang-tools-extra/clangd/ModulesBuilder.h @@ -12,7 +12,7 @@ // across different versions and different source files. But this is clearly a // waste of time and space in the end of the day. // -// FIXME: Supporting reusing module files across different versions and +// TODO: Supporting reusing module files across different versions and // different source files. // //===----------------------------------------------------------------------===// @@ -26,6 +26,8 @@ #include "support/Path.h" #include "support/ThreadsafeFS.h" +#include "clang/Frontend/CompilerInvocation.h" + #include "llvm/ADT/SmallString.h" #include @@ -33,7 +35,58 @@ namespace clang { namespace clangd { -class PrerequisiteModules; +/// Store all the needed module files information to parse a single +/// source file. e.g., +/// +/// ``` +/// // a.cppm +/// export module a; +/// +/// // b.cppm +/// export module b; +/// import a; +/// +/// // c.cppm +/// export module c; +/// import a; +/// ``` +/// +/// For the source file `c.cppm`, an instance of the class will store +/// the module files for `a.cppm` and `b.cppm`. But the module file for `c.cppm` +/// won't be stored. Since it is not needed to parse `c.cppm`. +/// +/// Users should only get PrerequisiteModules from +/// `ModulesBuilder::buildPrerequisiteModulesFor(...)`. +/// +/// Users can detect whether the PrerequisiteModules is still up to date by +/// calling the `canReuse()` member function. +/// +/// The users should call `adjustHeaderSearchOptions(...)` to update the +/// compilation commands to select the built module files first. Before calling +/// `adjustHeaderSearchOptions()`, users should call `canReuse()` first to check +/// if all the stored module files are valid. In case they are not valid, +/// users should call `ModulesBuilder::buildPrerequisiteModulesFor(...)` again +/// to get the new PrerequisiteModules. +class PrerequisiteModules { +public: + /// Change commands to load the module files recorded in this + /// PrerequisiteModules first. + virtual void + adjustHeaderSearchOptions(HeaderSearchOptions &Options) const = 0; + + /// Whether or not the built module files are up to date. + /// Note that this can only be used after building the module files. + virtual bool + canReuse(const CompilerInvocation &CI, + llvm::IntrusiveRefCntPtr) const = 0; + + /// Return true if the modile file specified by ModuleName is built. + /// Note that this interface will only check the existence of the module + /// file instead of checking the validness of the module file. + virtual bool isModuleUnitBuilt(llvm::StringRef ModuleName) const = 0; + + virtual ~PrerequisiteModules() = default; +}; /// This class handles building module files for a given source file. /// @@ -41,8 +94,6 @@ class PrerequisiteModules; /// different versions and different source files. class ModulesBuilder { public: - ModulesBuilder() = delete; - ModulesBuilder(const GlobalCompilationDatabase &CDB) : CDB(CDB) {} ModulesBuilder(const ModulesBuilder &) = delete; @@ -51,17 +102,10 @@ class ModulesBuilder { ModulesBuilder &operator=(const ModulesBuilder &) = delete; ModulesBuilder &operator=(ModulesBuilder &&) = delete; - ~ModulesBuilder() = default; - std::unique_ptr - buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS *TFS); + buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS *TFS) const; private: - bool buildModuleFile(StringRef ModuleName, const ThreadsafeFS *TFS, - std::shared_ptr MDB, - PathRef ModuleFilesPrefix, - PrerequisiteModules &RequiredModules); - const GlobalCompilationDatabase &CDB; }; diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h index 3b05856c6c98f2..250a3990b5b678 100644 --- a/clang-tools-extra/clangd/Preamble.h +++ b/clang-tools-extra/clangd/Preamble.h @@ -29,7 +29,6 @@ #include "Headers.h" #include "ModulesBuilder.h" -#include "PrerequisiteModules.h" #include "clang-include-cleaner/Record.h" #include "support/Path.h" diff --git a/clang-tools-extra/clangd/PrerequisiteModules.h b/clang-tools-extra/clangd/PrerequisiteModules.h deleted file mode 100644 index a73beb76480248..00000000000000 --- a/clang-tools-extra/clangd/PrerequisiteModules.h +++ /dev/null @@ -1,87 +0,0 @@ -//===----------------- PrerequisiteModules.h ---------------------*- C++-*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PREREQUISITEMODULES_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PREREQUISITEMODULES_H - -#include "Compiler.h" -#include "support/Path.h" - -#include "clang/Lex/HeaderSearchOptions.h" - -#include "llvm/ADT/StringSet.h" - -namespace clang { -namespace clangd { - -class ModulesBuilder; - -/// Store all the needed module files information to parse a single -/// source file. e.g., -/// -/// ``` -/// // a.cppm -/// export module a; -/// -/// // b.cppm -/// export module b; -/// import a; -/// -/// // c.cppm -/// export module c; -/// import a; -/// ``` -/// -/// For the source file `c.cppm`, an instance of the class will store -/// the module files for `a.cppm` and `b.cppm`. But the module file for `c.cppm` -/// won't be stored. Since it is not needed to parse `c.cppm`. -/// -/// Users should only get PrerequisiteModules from -/// `ModulesBuilder::buildPrerequisiteModulesFor(...)`. -/// -/// Users can detect whether the PrerequisiteModules is still up to date by -/// calling the `canReuse()` member function. -/// -/// The users should call `adjustHeaderSearchOptions(...)` to update the -/// compilation commands to select the built module files first. Before calling -/// `adjustHeaderSearchOptions()`, users should call `canReuse()` first to check -/// if all the stored module files are valid. In case they are not valid, -/// users should call `ModulesBuilder::buildPrerequisiteModulesFor(...)` again -/// to get the new PrerequisiteModules. -class PrerequisiteModules { -public: - /// Change commands to load the module files recorded in this - /// PrerequisiteModules first. - virtual void - adjustHeaderSearchOptions(HeaderSearchOptions &Options) const = 0; - - /// Whether or not the built module files are up to date. - /// Note that this can only be used after building the module files. - virtual bool - canReuse(const CompilerInvocation &CI, - llvm::IntrusiveRefCntPtr) const = 0; - - /// Return true if the modile file specified by ModuleName is built. - /// Note that this interface will only check the existence of the module - /// file instead of checking the validness of the module file. - virtual bool isModuleUnitBuilt(StringRef ModuleName) const = 0; - - virtual ~PrerequisiteModules() = default; - -private: - friend class ModulesBuilder; - - /// Add a module file to the PrerequisiteModules. - virtual void addModuleFile(StringRef ModuleName, - StringRef ModuleFilePath) = 0; -}; - -} // namespace clangd -} // namespace clang - -#endif diff --git a/clang-tools-extra/clangd/ProjectModules.cpp b/clang-tools-extra/clangd/ProjectModules.cpp index 6a9d9731c309ae..eb2458679b00ee 100644 --- a/clang-tools-extra/clangd/ProjectModules.cpp +++ b/clang-tools-extra/clangd/ProjectModules.cpp @@ -8,6 +8,152 @@ #include "ProjectModules.h" +#include "support/Logger.h" + +#include "clang/Tooling/DependencyScanning/DependencyScanningService.h" +#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h" + +using namespace clang; +using namespace clang::clangd; + +namespace { +/// A scanner to query the dependency information for C++20 Modules. +/// +/// The scanner can scan a single file with `scan(PathRef)` member function +/// or scan the whole project with `globalScan(vector)` member +/// function. See the comments of `globalScan` to see the details. +/// +/// The ModuleDependencyScanner can get the directly required module names for a +/// specific source file. Also the ModuleDependencyScanner can get the source +/// file declaring the primary module interface for a specific module name. +/// +/// IMPORTANT NOTE: we assume that every module unit is only declared once in a +/// source file in the project. But the assumption is not strictly true even +/// besides the invalid projects. The language specification requires that every +/// module unit should be unique in a valid program. But a project can contain +/// multiple programs. Then it is valid that we can have multiple source files +/// declaring the same module in a project as long as these source files don't +/// interfere with each other. +class ModuleDependencyScanner { +public: + ModuleDependencyScanner(const GlobalCompilationDatabase &CDB, + const ThreadsafeFS &TFS) + : CDB(CDB), TFS(TFS), + Service(tooling::dependencies::ScanningMode::CanonicalPreprocessing, + tooling::dependencies::ScanningOutputFormat::P1689) {} + + /// The scanned modules dependency information for a specific source file. + struct ModuleDependencyInfo { + /// The name of the module if the file is a module unit. + std::optional ModuleName; + /// A list of names for the modules that the file directly depends. + std::vector RequiredModules; + }; + + /// Scanning the single file specified by \param FilePath. + std::optional scan(PathRef FilePath); + + /// Scanning every source file in the current project to get the + /// to map. + /// TODO: We should find an efficient method to get the + /// to map. We can make it either by providing + /// a global module dependency scanner to monitor every file. Or we + /// can simply require the build systems (or even the end users) + /// to provide the map. + void globalScan(llvm::ArrayRef AllFiles); + + /// Get the source file from the module name. Note that the language + /// guarantees all the module names are unique in a valid program. + /// This function should only be called after globalScan. + /// + /// TODO: We should handle the case that there are multiple source files + /// declaring the same module. + PathRef getSourceForModuleName(llvm::StringRef ModuleName) const; + + /// Return the direct required modules. Indirect required modules are not + /// included. + std::vector getRequiredModules(PathRef File); + +private: + const GlobalCompilationDatabase &CDB; + const ThreadsafeFS &TFS; + + // Whether the scanner has scanned the project globally. + bool GlobalScanned = false; + + clang::tooling::dependencies::DependencyScanningService Service; + + // TODO: Add a scanning cache. + + // Map module name to source file path. + llvm::StringMap ModuleNameToSource; +}; + +std::optional +ModuleDependencyScanner::scan(PathRef FilePath) { + std::optional Cmd = CDB.getCompileCommand(FilePath); + + if (!Cmd) + return std::nullopt; + + using namespace clang::tooling::dependencies; + + llvm::SmallString<128> FilePathDir(FilePath); + llvm::sys::path::remove_filename(FilePathDir); + DependencyScanningTool ScanningTool(Service, TFS.view(FilePathDir)); + + llvm::Expected ScanningResult = + ScanningTool.getP1689ModuleDependencyFile(*Cmd, Cmd->Directory); + + if (auto E = ScanningResult.takeError()) { + elog("Scanning modules dependencies for {0} failed: {1}", FilePath, + llvm::toString(std::move(E))); + return std::nullopt; + } + + ModuleDependencyInfo Result; + + if (ScanningResult->Provides) { + ModuleNameToSource[ScanningResult->Provides->ModuleName] = FilePath; + Result.ModuleName = ScanningResult->Provides->ModuleName; + } + + for (auto &Required : ScanningResult->Requires) + Result.RequiredModules.push_back(Required.ModuleName); + + return Result; +} + +void ModuleDependencyScanner::globalScan(llvm::ArrayRef AllFiles) { + for (auto &File : AllFiles) + scan(File); + + GlobalScanned = true; +} + +PathRef ModuleDependencyScanner::getSourceForModuleName( + llvm::StringRef ModuleName) const { + assert( + GlobalScanned && + "We should only call getSourceForModuleName after calling globalScan()"); + + if (auto It = ModuleNameToSource.find(ModuleName); + It != ModuleNameToSource.end()) + return It->second; + + return {}; +} + +std::vector +ModuleDependencyScanner::getRequiredModules(PathRef File) { + auto ScanningResult = scan(File); + if (!ScanningResult) + return {}; + + return ScanningResult->RequiredModules; +} +} // namespace + namespace clang { namespace clangd { @@ -34,11 +180,9 @@ class ScanningAllProjectModules : public ProjectModules { /// RequiredSourceFile is not used intentionally. See the comments of /// ModuleDependencyScanner for detail. PathRef - getSourceForModuleName(StringRef ModuleName, + getSourceForModuleName(llvm::StringRef ModuleName, PathRef RequiredSourceFile = PathRef()) override { - if (!Scanner.isGlobalScanned()) - Scanner.globalScan(AllFiles); - + Scanner.globalScan(AllFiles); return Scanner.getSourceForModuleName(ModuleName); } @@ -48,15 +192,15 @@ class ScanningAllProjectModules : public ProjectModules { ModuleDependencyScanner Scanner; }; -std::shared_ptr ProjectModules::create( +std::unique_ptr ProjectModules::create( ProjectModulesKind Kind, std::vector &&AllFiles, const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS) { if (Kind == ProjectModulesKind::ScanningAllFiles) - return std::make_shared(std::move(AllFiles), CDB, + return std::make_unique(std::move(AllFiles), CDB, TFS); llvm_unreachable("Unknown ProjectModulesKind."); } } // namespace clangd -} // namespace clang \ No newline at end of file +} // namespace clang diff --git a/clang-tools-extra/clangd/ProjectModules.h b/clang-tools-extra/clangd/ProjectModules.h index 98720ee06d4724..60ac586ec3e3db 100644 --- a/clang-tools-extra/clangd/ProjectModules.h +++ b/clang-tools-extra/clangd/ProjectModules.h @@ -9,7 +9,9 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROJECTMODULES_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROJECTMODULES_H -#include "ModuleDependencyScanner.h" +#include "GlobalCompilationDatabase.h" +#include "support/Path.h" +#include "support/ThreadsafeFS.h" #include @@ -37,15 +39,15 @@ class ProjectModules { public: enum class ProjectModulesKind { ScanningAllFiles }; - static std::shared_ptr + static std::unique_ptr create(ProjectModulesKind Kind, std::vector &&AllFiles, const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS); virtual std::vector getRequiredModules(PathRef File) = 0; virtual PathRef - getSourceForModuleName(StringRef ModuleName, + getSourceForModuleName(llvm::StringRef ModuleName, PathRef RequiredSrcFile = PathRef()) = 0; - + virtual ~ProjectModules() = default; }; diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp index cb90d29c2313ae..d3dfa64dc7a63d 100644 --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -52,7 +52,6 @@ #include "Config.h" #include "Diagnostics.h" #include "GlobalCompilationDatabase.h" -#include "ModulesBuilder.h" #include "ParsedAST.h" #include "Preamble.h" #include "clang-include-cleaner/Record.h" @@ -606,8 +605,8 @@ class ASTWorker { ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB, TUScheduler::ASTCache &LRUCache, TUScheduler::HeaderIncluderCache &HeaderIncluders, - Semaphore &Barrier, ModulesBuilder *ModulesManager, bool RunSync, - const TUScheduler::Options &Opts, ParsingCallbacks &Callbacks); + Semaphore &Barrier, bool RunSync, const TUScheduler::Options &Opts, + ParsingCallbacks &Callbacks); public: /// Create a new ASTWorker and return a handle to it. @@ -620,8 +619,7 @@ class ASTWorker { TUScheduler::ASTCache &IdleASTs, TUScheduler::HeaderIncluderCache &HeaderIncluders, AsyncTaskRunner *Tasks, Semaphore &Barrier, - ModulesBuilder *ModulesManager, const TUScheduler::Options &Opts, - ParsingCallbacks &Callbacks); + const TUScheduler::Options &Opts, ParsingCallbacks &Callbacks); ~ASTWorker(); void update(ParseInputs Inputs, WantDiagnostics, bool ContentChanged); @@ -656,7 +654,7 @@ class ASTWorker { const GlobalCompilationDatabase &getCompilationDatabase() { return CDB; } - ModulesBuilder *getModulesManager() const { return ModulesManager; } + ModulesBuilder *getModulesBuilder() { return FileInputs.ModulesManager; } private: // Details of an update request that are relevant to scheduling. @@ -770,8 +768,6 @@ class ASTWorker { SynchronizedTUStatus Status; PreambleThread PreamblePeer; - - ModulesBuilder *ModulesManager = nullptr; }; /// A smart-pointer-like class that points to an active ASTWorker. @@ -816,15 +812,16 @@ class ASTWorkerHandle { std::shared_ptr Worker; }; -ASTWorkerHandle ASTWorker::create( - PathRef FileName, const GlobalCompilationDatabase &CDB, - TUScheduler::ASTCache &IdleASTs, - TUScheduler::HeaderIncluderCache &HeaderIncluders, AsyncTaskRunner *Tasks, - Semaphore &Barrier, ModulesBuilder *ModulesManager, - const TUScheduler::Options &Opts, ParsingCallbacks &Callbacks) { - std::shared_ptr Worker(new ASTWorker( - FileName, CDB, IdleASTs, HeaderIncluders, Barrier, ModulesManager, - /*RunSync=*/!Tasks, Opts, Callbacks)); +ASTWorkerHandle +ASTWorker::create(PathRef FileName, const GlobalCompilationDatabase &CDB, + TUScheduler::ASTCache &IdleASTs, + TUScheduler::HeaderIncluderCache &HeaderIncluders, + AsyncTaskRunner *Tasks, Semaphore &Barrier, + const TUScheduler::Options &Opts, + ParsingCallbacks &Callbacks) { + std::shared_ptr Worker( + new ASTWorker(FileName, CDB, IdleASTs, HeaderIncluders, Barrier, + /*RunSync=*/!Tasks, Opts, Callbacks)); if (Tasks) { Tasks->runAsync("ASTWorker:" + llvm::sys::path::filename(FileName), [Worker]() { Worker->run(); }); @@ -838,16 +835,15 @@ ASTWorkerHandle ASTWorker::create( ASTWorker::ASTWorker(PathRef FileName, const GlobalCompilationDatabase &CDB, TUScheduler::ASTCache &LRUCache, TUScheduler::HeaderIncluderCache &HeaderIncluders, - Semaphore &Barrier, ModulesBuilder *ModulesManager, - bool RunSync, const TUScheduler::Options &Opts, + Semaphore &Barrier, bool RunSync, + const TUScheduler::Options &Opts, ParsingCallbacks &Callbacks) : IdleASTs(LRUCache), HeaderIncluders(HeaderIncluders), RunSync(RunSync), UpdateDebounce(Opts.UpdateDebounce), FileName(FileName), ContextProvider(Opts.ContextProvider), CDB(CDB), Callbacks(Callbacks), Barrier(Barrier), Done(false), Status(FileName, Callbacks), PreamblePeer(FileName, Callbacks, Opts.StorePreamblesInMemory, RunSync, - Opts.PreambleThrottler, Status, HeaderIncluders, *this), - ModulesManager(ModulesManager) { + Opts.PreambleThrottler, Status, HeaderIncluders, *this) { // Set a fallback command because compile command can be accessed before // `Inputs` is initialized. Other fields are only used after initialization // from client inputs. @@ -1093,7 +1089,7 @@ void PreambleThread::build(Request Req) { bool IsFirstPreamble = !LatestBuild; LatestBuild = clang::clangd::buildPreamble( - FileName, *Req.CI, Inputs, StoreInMemory, ASTPeer.getModulesManager(), + FileName, *Req.CI, Inputs, StoreInMemory, ASTPeer.getModulesBuilder(), [&](CapturedASTCtx ASTCtx, std::shared_ptr PI) { Callbacks.onPreambleAST(FileName, Inputs.Version, std::move(ASTCtx), @@ -1654,9 +1650,6 @@ TUScheduler::TUScheduler(const GlobalCompilationDatabase &CDB, PreambleTasks.emplace(); WorkerThreads.emplace(); } - - if (Opts.ExperimentalModulesSupport) - ModulesManager.emplace(CDB); } TUScheduler::~TUScheduler() { @@ -1689,8 +1682,7 @@ bool TUScheduler::update(PathRef File, ParseInputs Inputs, // Create a new worker to process the AST-related tasks. ASTWorkerHandle Worker = ASTWorker::create( File, CDB, *IdleASTs, *HeaderIncluders, - WorkerThreads ? &*WorkerThreads : nullptr, Barrier, - ModulesManager ? &*ModulesManager : nullptr, Opts, *Callbacks); + WorkerThreads ? &*WorkerThreads : nullptr, Barrier, Opts, *Callbacks); FD = std::unique_ptr( new FileData{Inputs.Contents, std::move(Worker)}); ContentChanged = true; diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h index 66e17cbaa7f032..706a13c59dd31d 100644 --- a/clang-tools-extra/clangd/TUScheduler.h +++ b/clang-tools-extra/clangd/TUScheduler.h @@ -204,8 +204,6 @@ class ParsingCallbacks { virtual void onPreamblePublished(PathRef File) {} }; -class ModulesBuilder; - /// Handles running tasks for ClangdServer and managing the resources (e.g., /// preambles and ASTs) for opened files. /// TUScheduler is not thread-safe, only one thread should be providing updates @@ -377,8 +375,6 @@ class TUScheduler { // running tasks asynchronously. std::optional PreambleTasks; std::optional WorkerThreads; - // Manages to build module files. - std::optional ModulesManager; // Used to create contexts for operations that are not bound to a particular // file (e.g. index queries). std::string LastActiveFile; diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt index b7be7d0f709abb..992e43b9ff42fc 100644 --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -73,7 +73,6 @@ add_unittest(ClangdUnitTests ClangdTests LoggerTests.cpp LSPBinderTests.cpp LSPClient.cpp - ModuleDependencyScannerTest.cpp PrerequisiteModulesTest.cpp ModulesTests.cpp ParsedASTTests.cpp diff --git a/clang-tools-extra/clangd/unittests/ModuleDependencyScannerTest.cpp b/clang-tools-extra/clangd/unittests/ModuleDependencyScannerTest.cpp deleted file mode 100644 index 6d42f8730538f2..00000000000000 --- a/clang-tools-extra/clangd/unittests/ModuleDependencyScannerTest.cpp +++ /dev/null @@ -1,176 +0,0 @@ -//===------------ ModuleDependencyScannerTest.cpp ---------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -/// FIXME: Skip testing on windows temporarily due to the different escaping -/// code mode. -#ifndef _WIN32 - -#include "ModuleDependencyScanner.h" -#include "ModulesTestSetup.h" -#include "TestFS.h" - -using namespace clang; -using namespace clang::clangd; -using namespace clang::tooling::dependencies; - -namespace { -class ModuleDependencyScannerTests : public ModuleTestSetup {}; - -TEST_F(ModuleDependencyScannerTests, SingleFile) { - addFile("foo.h", R"cpp( -import foo; - )cpp"); - - addFile("A.cppm", R"cpp( -module; -#include "foo.h" -export module A; -export import :partA; -import :partB; -import C; - -module :private; -import D; - )cpp"); - - MockCompilationDatabase CDB(TestDir); - CDB.ExtraClangFlags.push_back("-std=c++20"); - - ModuleDependencyScanner Scanner(CDB, TFS); - std::optional ScanningResult = - Scanner.scan(getFullPath("A.cppm")); - EXPECT_TRUE(ScanningResult); - - EXPECT_TRUE(ScanningResult->ModuleName); - EXPECT_EQ(*ScanningResult->ModuleName, "A"); - - EXPECT_EQ(ScanningResult->RequiredModules.size(), 5u); - EXPECT_EQ(ScanningResult->RequiredModules[0], "foo"); - EXPECT_EQ(ScanningResult->RequiredModules[1], "A:partA"); - EXPECT_EQ(ScanningResult->RequiredModules[2], "A:partB"); - EXPECT_EQ(ScanningResult->RequiredModules[3], "C"); - EXPECT_EQ(ScanningResult->RequiredModules[4], "D"); -} - -TEST_F(ModuleDependencyScannerTests, GlobalScanning) { - addFile("build/compile_commands.json", R"cpp( -[ -{ - "directory": "__DIR__", - "command": "clang++ -std=c++20 __DIR__/foo.cppm -fmodule-output=__DIR__/foo.pcm -c -o __DIR__/foo.o", - "file": "__DIR__/foo.cppm", - "output": "__DIR__/foo.o" -}, -{ - "directory": "__DIR__", - "command": "clang++ -std=c++20 __DIR__/C.cppm -fmodule-output=__DIR__/C.pcm -c -o __DIR__/C.o", - "file": "__DIR__/C.cppm", - "output": "__DIR__/C.o" -}, -{ - "directory": "__DIR__", - "command": "clang++ -std=c++20 __DIR__/D.cppm -fmodule-output=__DIR__/D.pcm -c -o __DIR__/D.o", - "file": "__DIR__/D.cppm", - "output": "__DIR__/D.o" -}, -{ - "directory": "__DIR__", - "command": "clang++ -std=c++20 __DIR__/A-partA.cppm -fmodule-file=foo=__DIR__/foo.pcm -fmodule-output=__DIR__/A-partA.pcm -c -o __DIR__/A-partA.o", - "file": "__DIR__/A-partA.cppm", - "output": "__DIR__/A-partA.o" -}, -{ - "directory": "__DIR__", - "command": "clang++ -std=c++20 __DIR__/A-partB.cppm -fmodule-file=C=__DIR__/C.pcm -fmodule-output=__DIR__/A-partB.pcm -c -o __DIR__/A-partB.o", - "file": "__DIR__/A-partB.cppm", - "output": "__DIR__/A-partB.o" -}, -{ - "directory": "__DIR__", - "command": "clang++ -std=c++20 __DIR__/A.cppm -fmodule-file=A:partB=__DIR__/A-partB.pcm -fmodule-file=A:partA=__DIR__/A-partA.pcm -fmodule-file=foo=__DIR__/foo.pcm -fmodule-file=C=__DIR__/C.pcm -fmodule-file=D=__DIR__/C.pcm -fmodule-output=__DIR__/A.pcm -c -o __DIR__/A.o", - "file": "__DIR__/A.cppm", - "output": "__DIR__/A.o" -}, -] - )cpp"); - - addFile("foo.cppm", R"cpp( -export module foo; - )cpp"); - - addFile("foo.h", R"cpp( -import foo; - )cpp"); - - addFile("A-partA.cppm", R"cpp( -export module A:partA; -import foo; - )cpp"); - - addFile("A-partB.cppm", R"cpp( -module A:partB; -import C; - )cpp"); - - addFile("C.cppm", R"cpp( -export module C; - )cpp"); - - addFile("D.cppm", R"cpp( -export module D; - )cpp"); - - addFile("A.cppm", R"cpp( -module; -#include "foo.h" -export module A; -export import :partA; -import :partB; -import C; - -module :private; -import D; - )cpp"); - - std::unique_ptr CDB = - getGlobalCompilationDatabase(); - ModuleDependencyScanner Scanner(*CDB.get(), TFS); - Scanner.globalScan({getFullPath("A.cppm"), getFullPath("foo.cppm"), - getFullPath("A-partA.cppm"), getFullPath("A-partB.cppm"), - getFullPath("C.cppm"), getFullPath("D.cppm")}); - - EXPECT_TRUE(Scanner.getSourceForModuleName("foo").endswith("foo.cppm")); - EXPECT_TRUE(Scanner.getSourceForModuleName("A").endswith("A.cppm")); - EXPECT_TRUE( - Scanner.getSourceForModuleName("A:partA").endswith("A-partA.cppm")); - EXPECT_TRUE( - Scanner.getSourceForModuleName("A:partB").endswith("A-partB.cppm")); - EXPECT_TRUE(Scanner.getSourceForModuleName("C").endswith("C.cppm")); - EXPECT_TRUE(Scanner.getSourceForModuleName("D").endswith("D.cppm")); - - EXPECT_TRUE(Scanner.getRequiredModules(getFullPath("foo.cppm")).empty()); - EXPECT_TRUE(Scanner.getRequiredModules(getFullPath("C.cppm")).empty()); - EXPECT_TRUE(Scanner.getRequiredModules(getFullPath("D.cppm")).empty()); - - EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A-partA.cppm")).size(), 1u); - EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A-partA.cppm"))[0], "foo"); - - EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A-partB.cppm")).size(), 1u); - EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A-partB.cppm"))[0], "C"); - - EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A.cppm")).size(), 5u); - EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A.cppm"))[0], "foo"); - EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A.cppm"))[1], "A:partA"); - EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A.cppm"))[2], "A:partB"); - EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A.cppm"))[3], "C"); - EXPECT_EQ(Scanner.getRequiredModules(getFullPath("A.cppm"))[4], "D"); -} - -} // namespace - -#endif diff --git a/clang-tools-extra/clangd/unittests/ModulesTestSetup.h b/clang-tools-extra/clangd/unittests/ModulesTestSetup.h index 3f66c5ff80dc69..75e982ea2fb08a 100644 --- a/clang-tools-extra/clangd/unittests/ModulesTestSetup.h +++ b/clang-tools-extra/clangd/unittests/ModulesTestSetup.h @@ -28,7 +28,7 @@ class ModuleTestSetup : public ::testing::Test { public: // Add files to the working testing directory and repalce all the // `__DIR__` to TestDir. - void addFile(StringRef Path, StringRef Contents) { + void addFile(llvm::StringRef Path, llvm::StringRef Contents) { ASSERT_FALSE(llvm::sys::path::is_absolute(Path)); SmallString<256> AbsPath(TestDir); @@ -54,7 +54,7 @@ class ModuleTestSetup : public ::testing::Test { // Get the absolute path for file specified by Path under testing working // directory. - std::string getFullPath(StringRef Path) { + std::string getFullPath(llvm::StringRef Path) { SmallString<128> Result(TestDir); llvm::sys::path::append(Result, Path); EXPECT_TRUE(llvm::sys::fs::exists(Result.str())); @@ -69,7 +69,7 @@ class ModuleTestSetup : public ::testing::Test { return std::make_unique(Opts); } - ParseInputs getInputs(StringRef FileName, + ParseInputs getInputs(llvm::StringRef FileName, const GlobalCompilationDatabase &CDB) { std::string FullPathName = getFullPath(FileName); diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp index 9107d0e2b85a5e..0bb5b8392e82d2 100644 --- a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp +++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp @@ -11,7 +11,6 @@ /// code mode. #ifndef _WIN32 -#include "PrerequisiteModules.h" #include "ModulesBuilder.h" #include "ModulesTestSetup.h" diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 2552b233c58024..ff69fdcaddc5da 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -49,7 +49,8 @@ Improvements to clangd ---------------------- - Introduced exmperimental support for C++20 Modules. The experimental support can - be enabled by `-experimental-modules-support` option. + be enabled by `-experimental-modules-support` option. It is in an early development + stage and may not perform efficiently in real-world scenarios. Inlay hints ^^^^^^^^^^^ From 14dc3ddf22a105b697c40580e7e8f768744d12b6 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Mon, 3 Jun 2024 16:35:58 +0800 Subject: [PATCH 3/6] Remove ExperimentalModulesSupport from TUScheduler Options Address comments Update Update Update address fmt Update fmt update update Update --- clang-tools-extra/clangd/ClangdServer.cpp | 1 - clang-tools-extra/clangd/ClangdServer.h | 2 +- clang-tools-extra/clangd/Compiler.h | 3 +- .../clangd/GlobalCompilationDatabase.cpp | 9 +- .../clangd/GlobalCompilationDatabase.h | 3 +- clang-tools-extra/clangd/ModulesBuilder.cpp | 147 +++++++-------- clang-tools-extra/clangd/ModulesBuilder.h | 9 +- clang-tools-extra/clangd/Preamble.cpp | 10 +- clang-tools-extra/clangd/Preamble.h | 4 - clang-tools-extra/clangd/ProjectModules.cpp | 59 +++---- clang-tools-extra/clangd/ProjectModules.h | 7 - .../clangd/ScanningProjectModules.h | 26 +++ clang-tools-extra/clangd/TUScheduler.cpp | 6 +- clang-tools-extra/clangd/TUScheduler.h | 3 - clang-tools-extra/clangd/tool/Check.cpp | 6 - .../clangd/unittests/CodeCompleteTests.cpp | 5 +- .../clangd/unittests/FileIndexTests.cpp | 2 +- .../clangd/unittests/ModulesTestSetup.h | 103 ----------- .../clangd/unittests/ParsedASTTests.cpp | 6 +- .../clangd/unittests/PreambleTests.cpp | 2 +- .../unittests/PrerequisiteModulesTest.cpp | 167 ++++++++++++++++-- clang-tools-extra/clangd/unittests/TestTU.cpp | 2 - 22 files changed, 288 insertions(+), 294 deletions(-) create mode 100644 clang-tools-extra/clangd/ScanningProjectModules.h delete mode 100644 clang-tools-extra/clangd/unittests/ModulesTestSetup.h diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index d1017828e987cc..0771a0d1a87b20 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -208,7 +208,6 @@ ClangdServer::Options::operator TUScheduler::Options() const { Opts.UpdateDebounce = UpdateDebounce; Opts.ContextProvider = ContextProvider; Opts.PreambleThrottler = PreambleThrottler; - Opts.ExperimentalModulesSupport = ExperimentalModulesSupport; return Opts; } diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index 67b8184e1bcded..d387652fb6a276 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -16,6 +16,7 @@ #include "FeatureModule.h" #include "GlobalCompilationDatabase.h" #include "Hover.h" +#include "ModulesBuilder.h" #include "Protocol.h" #include "SemanticHighlighting.h" #include "TUScheduler.h" @@ -43,7 +44,6 @@ namespace clang { namespace clangd { -class ModulesBuilder; /// Manages a collection of source files and derived data (ASTs, indexes), /// and provides language-aware features such as code completion. /// diff --git a/clang-tools-extra/clangd/Compiler.h b/clang-tools-extra/clangd/Compiler.h index f4b377bd537a1d..4e68da7610ca2c 100644 --- a/clang-tools-extra/clangd/Compiler.h +++ b/clang-tools-extra/clangd/Compiler.h @@ -16,6 +16,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_COMPILER_H #include "FeatureModule.h" +#include "ModulesBuilder.h" #include "TidyProvider.h" #include "index/Index.h" #include "support/ThreadsafeFS.h" @@ -44,8 +45,6 @@ struct ParseOptions { bool ImportInsertions = false; }; -class ModulesBuilder; - /// Information required to run clang, e.g. to parse AST or do code completion. struct ParseInputs { tooling::CompileCommand CompileCommand; diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp index 3da770709ec93e..f11a2f73aa3ff0 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp @@ -10,6 +10,7 @@ #include "Config.h" #include "FS.h" #include "ProjectModules.h" +#include "ScanningProjectModules.h" #include "SourceCode.h" #include "support/Logger.h" #include "support/Path.h" @@ -751,12 +752,8 @@ DirectoryBasedGlobalCompilationDatabase::getProjectModules(PathRef File) const { auto Res = lookupCDB(Req); if (!Res) return {}; - // FIXME: Passing *this here means we won't use outer OverlayCDB, which - // (among other things) has the mechanism for detecting and injecting - // resource-dir. - return ProjectModules::create( - ProjectModules::ProjectModulesKind::ScanningAllFiles, - Res->CDB->getAllFiles(), *this, Opts.TFS); + + return scanningProjectModules(Res->CDB, Opts.TFS); } OverlayCDB::OverlayCDB(const GlobalCompilationDatabase *Base, diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.h b/clang-tools-extra/clangd/GlobalCompilationDatabase.h index 1316f49a473c04..ea999fe8aee017 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.h +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.h @@ -9,6 +9,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_GLOBALCOMPILATIONDATABASE_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_GLOBALCOMPILATIONDATABASE_H +#include "ProjectModules.h" #include "support/Function.h" #include "support/Path.h" #include "support/Threading.h" @@ -25,8 +26,6 @@ namespace clang { namespace clangd { -class ProjectModules; - struct ProjectInfo { // The directory in which the compilation database was discovered. // Empty if directory-based compilation database discovery was not used. diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 83c3b3856cb3ef..4b40961042f93e 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -23,7 +23,7 @@ namespace { // Create a path to store module files. Generally it should be: // -// {TEMP_DIRS}/clangd/module_files/{PrefixDir}-%%-%%-%%-%%-%%-%%/. +// {TEMP_DIRS}/clangd/module_files/{hashed-file-name}-%%-%%-%%-%%-%%-%%/. // // {TEMP_DIRS} is the temporary directory for the system, e.g., "/var/tmp" // or "C:/TEMP". @@ -31,20 +31,16 @@ namespace { // '%%' means random value to make the generated path unique. // // \param MainFile is used to get the root of the project from global -// compilation database. \param PrefixDir is used to get the user -// defined prefix for module files. This is useful when we want to seperate -// module files. e.g., we want to build module files for the same module unit -// `a.cppm` with 2 different users `b.cpp` and `c.cpp` and we don't want the -// module file for `b.cpp` be conflict with the module files for `c.cpp`. Then -// we can put the 2 module files into different dirs like: -// -// ${TEMP_DIRS}/clangd/module_files/b.cpp/a.pcm -// ${TEMP_DIRS}/clangd/module_files/c.cpp/a.pcm +// compilation database. // // TODO: Move these module fils out of the temporary directory if the module // files are persistent. -llvm::SmallString<256> getUniqueModuleFilesPath(PathRef MainFile, - llvm::StringRef PrefixDir) { +llvm::SmallString<256> getUniqueModuleFilesPath(PathRef MainFile) { + llvm::SmallString<128> HashedPrefix = llvm::sys::path::filename(MainFile); + // There might be multiple files with the same name in a project. So appending + // the hash value of the full path to make sure they won't conflict. + HashedPrefix += std::to_string(llvm::hash_value(MainFile)); + llvm::SmallString<256> ResultPattern; llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true, @@ -53,7 +49,7 @@ llvm::SmallString<256> getUniqueModuleFilesPath(PathRef MainFile, llvm::sys::path::append(ResultPattern, "clangd"); llvm::sys::path::append(ResultPattern, "module_files"); - llvm::sys::path::append(ResultPattern, PrefixDir); + llvm::sys::path::append(ResultPattern, HashedPrefix); ResultPattern.append("-%%-%%-%%-%%-%%-%%"); @@ -81,23 +77,17 @@ llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand &Cmd) { // Get a unique module file path under \param ModuleFilesPrefix. std::string getModuleFilePath(llvm::StringRef ModuleName, PathRef ModuleFilesPrefix) { - llvm::SmallString<256> ModuleFilePattern(ModuleFilesPrefix); + llvm::SmallString<256> ModuleFilePath(ModuleFilesPrefix); auto [PrimaryModuleName, PartitionName] = ModuleName.split(':'); - llvm::sys::path::append(ModuleFilePattern, PrimaryModuleName); + llvm::sys::path::append(ModuleFilePath, PrimaryModuleName); if (!PartitionName.empty()) { - ModuleFilePattern.append("-"); - ModuleFilePattern.append(PartitionName); + ModuleFilePath.append("-"); + ModuleFilePath.append(PartitionName); } - ModuleFilePattern.append(".pcm"); - - llvm::SmallString<256> ModuleFilePath; - llvm::sys::fs::createUniquePath(ModuleFilePattern, ModuleFilePath, - /*MakeAbsolute=*/false); - + ModuleFilePath.append(".pcm"); return std::string(ModuleFilePath); } -} // namespace // FailedPrerequisiteModules - stands for the PrerequisiteModules which has // errors happened during the building process. @@ -116,11 +106,6 @@ class FailedPrerequisiteModules : public PrerequisiteModules { llvm::IntrusiveRefCntPtr) const override { return false; } - - // No module unit got built in FailedPrerequisiteModules. - bool isModuleUnitBuilt(llvm::StringRef ModuleName) const override { - return false; - } }; // StandalonePrerequisiteModules - stands for PrerequisiteModules for which all @@ -155,7 +140,7 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { bool canReuse(const CompilerInvocation &CI, llvm::IntrusiveRefCntPtr) const override; - bool isModuleUnitBuilt(llvm::StringRef ModuleName) const override { + bool isModuleUnitBuilt(llvm::StringRef ModuleName) const { return BuiltModuleNames.contains(ModuleName); } @@ -196,18 +181,17 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { llvm::StringSet<> BuiltModuleNames; }; -namespace { // Build a module file for module with `ModuleName`. Return false // when there are problem happens. Return true when the // module file exists or built successfully. The information of built // module file are stored in \param BuiltModuleFiles. -bool buildModuleFile(llvm::StringRef ModuleName, - const GlobalCompilationDatabase &CDB, - const ThreadsafeFS *TFS, ProjectModules *MDB, - PathRef ModuleFilesPrefix, - StandalonePrerequisiteModules &BuiltModuleFiles) { +llvm::Error buildModuleFile(llvm::StringRef ModuleName, + const GlobalCompilationDatabase &CDB, + const ThreadsafeFS &TFS, ProjectModules *MDB, + PathRef ModuleFilesPrefix, + StandalonePrerequisiteModules &BuiltModuleFiles) { if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) - return true; + return llvm::Error::success(); PathRef ModuleUnitFileName = MDB->getSourceForModuleName(ModuleName); // It is possible that we're meeting third party modules (modules whose @@ -217,49 +201,42 @@ bool buildModuleFile(llvm::StringRef ModuleName, // FIXME: How should we treat third party modules here? If we want to ignore // third party modules, we should return true instead of false here. // Currently we simply bail out. - if (ModuleUnitFileName.empty()) { - elog("Failed to get the source for module name {0}. Maybe it is from third " - "party libraries or something goes wrong", - ModuleName); - return false; - } + if (ModuleUnitFileName.empty()) + return llvm::createStringError(llvm::formatv( + "Failed to build '{0}': Failed to get the primary source", ModuleName)); for (auto &RequiredModuleName : MDB->getRequiredModules(ModuleUnitFileName)) { // Return early if there are errors building the module file. - if (!buildModuleFile(RequiredModuleName, CDB, TFS, MDB, ModuleFilesPrefix, - BuiltModuleFiles)) { - elog("Failed to build {0} since failed to build {1}", ModuleName, - RequiredModuleName); - return false; - } + if (llvm::Error Err = buildModuleFile(RequiredModuleName, CDB, TFS, MDB, + ModuleFilesPrefix, BuiltModuleFiles)) + return Err; } auto Cmd = CDB.getCompileCommand(ModuleUnitFileName); - if (!Cmd) { - elog("Failed to get compile command for {0}", ModuleName); - return false; - } + if (!Cmd) + return llvm::createStringError( + llvm::formatv("Failed to build '{0}': No compile command for {1}", + ModuleName, ModuleUnitFileName)); Cmd->Output = getModuleFilePath(ModuleName, ModuleFilesPrefix); ParseInputs Inputs; - Inputs.TFS = TFS; + Inputs.TFS = &TFS; Inputs.CompileCommand = std::move(*Cmd); IgnoreDiagnostics IgnoreDiags; auto CI = buildCompilerInvocation(Inputs, IgnoreDiags); - if (!CI) { - elog("Failed to build compiler invocation for {0}", ModuleName); - return false; - } + if (!CI) + return llvm::createStringError(llvm::formatv( + "Failed to build '{0}': Failed to build compiler invocation for {1}", + ModuleName, ModuleUnitFileName)); auto FS = Inputs.TFS->view(Inputs.CompileCommand.Directory); - auto AbsolutePath = getAbsolutePath(Inputs.CompileCommand); - auto Buf = FS->getBufferForFile(AbsolutePath); - if (!Buf) { - elog("Failed to create buffer for {0}", AbsolutePath); - return false; - } + auto Buf = FS->getBufferForFile(Inputs.CompileCommand.Filename); + if (!Buf) + return llvm::createStringError( + llvm::formatv("Failed to build '{0}': Failed to create buffer for {1}", + ModuleName, Inputs.CompileCommand.Filename)); // Hash the contents of input files and store the hash value to the BMI files. // So that we can check if the files are still valid when we want to reuse the @@ -272,27 +249,27 @@ bool buildModuleFile(llvm::StringRef ModuleName, auto Clang = prepareCompilerInstance(std::move(CI), /*Preamble=*/nullptr, std::move(*Buf), std::move(FS), IgnoreDiags); - if (!Clang) { - elog("Failed to prepare compiler instance for {0}", AbsolutePath); - return false; - } + if (!Clang) + return llvm::createStringError(llvm::formatv( + "Failed to build '{0}': Failed to prepare compiler instance for {0}", + ModuleName, ModuleUnitFileName)); GenerateModuleInterfaceAction Action; Clang->ExecuteAction(Action); - if (Clang->getDiagnostics().hasErrorOccurred()) { - elog("Compilation for {0} failed", AbsolutePath); - return false; - } + if (Clang->getDiagnostics().hasErrorOccurred()) + return llvm::createStringError( + llvm::formatv("Failed to build '{0}': Compilation for {1} failed", + ModuleName, ModuleUnitFileName)); BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); - return true; + return llvm::Error::success(); } } // namespace std::unique_ptr ModulesBuilder::buildPrerequisiteModulesFor(PathRef File, - const ThreadsafeFS *TFS) const { + const ThreadsafeFS &TFS) const { std::unique_ptr MDB = CDB.getProjectModules(File); if (!MDB) return {}; @@ -301,23 +278,23 @@ ModulesBuilder::buildPrerequisiteModulesFor(PathRef File, if (RequiredModuleNames.empty()) return {}; - llvm::SmallString<128> Prefix = llvm::sys::path::filename(File); - // There might be multiple files with the same name in a project. So appending - // the hash value of the full path to make sure they won't conflict. - Prefix += std::to_string(llvm::hash_value(File)); - llvm::SmallString<256> ModuleFilesPrefix = - getUniqueModuleFilesPath(File, Prefix); + llvm::SmallString<256> ModuleFilesPrefix = getUniqueModuleFilesPath(File); log("Trying to build required modules for {0} in {1}", File, ModuleFilesPrefix); auto RequiredModules = std::make_unique(); - for (llvm::StringRef RequiredModuleName : RequiredModuleNames) + for (llvm::StringRef RequiredModuleName : RequiredModuleNames) { // Return early if there is any error. - if (!buildModuleFile(RequiredModuleName, CDB, TFS, MDB.get(), - ModuleFilesPrefix, *RequiredModules.get())) + if (llvm::Error Err = + buildModuleFile(RequiredModuleName, CDB, TFS, MDB.get(), + ModuleFilesPrefix, *RequiredModules.get())) { + elog("Failed to build module {0}; due to {1}", RequiredModuleName, + toString(std::move(Err))); return std::make_unique(); + } + } log("Built required modules for {0} in {1}", File, ModuleFilesPrefix); @@ -351,14 +328,14 @@ bool StandalonePrerequisiteModules::canReuse( Clang.createASTReader(); for (auto &RequiredModule : RequiredModules) { llvm::StringRef BMIPath = RequiredModule.ModuleFilePath; - // TODO: Loading BMI fully is too heavy considering something cheaply to + // FIXME: Loading BMI fully is too heavy considering something cheaply to // check if we can reuse the BMI. auto ReadResult = Clang.getASTReader()->ReadAST(BMIPath, serialization::MK_MainFile, SourceLocation(), ASTReader::ARR_None); if (ReadResult != ASTReader::Success) { - elog("Failed to reuse {0} due to {1}", BMIPath, ReadResult); + elog("Can't reuse {0}: {1}", BMIPath, ReadResult); return false; } } diff --git a/clang-tools-extra/clangd/ModulesBuilder.h b/clang-tools-extra/clangd/ModulesBuilder.h index 1a67d3e1db028e..cece418bcf525c 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.h +++ b/clang-tools-extra/clangd/ModulesBuilder.h @@ -48,7 +48,7 @@ namespace clangd { /// /// // c.cppm /// export module c; -/// import a; +/// import b; /// ``` /// /// For the source file `c.cppm`, an instance of the class will store @@ -80,11 +80,6 @@ class PrerequisiteModules { canReuse(const CompilerInvocation &CI, llvm::IntrusiveRefCntPtr) const = 0; - /// Return true if the modile file specified by ModuleName is built. - /// Note that this interface will only check the existence of the module - /// file instead of checking the validness of the module file. - virtual bool isModuleUnitBuilt(llvm::StringRef ModuleName) const = 0; - virtual ~PrerequisiteModules() = default; }; @@ -103,7 +98,7 @@ class ModulesBuilder { ModulesBuilder &operator=(ModulesBuilder &&) = delete; std::unique_ptr - buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS *TFS) const; + buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS &TFS) const; private: const GlobalCompilationDatabase &CDB; diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp index ebb6e385a3037a..d70c7ad245a42b 100644 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -589,8 +589,8 @@ class DiagPatcher { std::shared_ptr buildPreamble( PathRef FileName, CompilerInvocation CI, const ParseInputs &Inputs, - bool StoreInMemory, ModulesBuilder *RequiredModuleBuilder, - PreambleParsedCallback PreambleCallback, PreambleBuildStats *Stats) { + bool StoreInMemory, PreambleParsedCallback PreambleCallback, + PreambleBuildStats *Stats) { // Note that we don't need to copy the input contents, preamble can live // without those. auto ContentsBuffer = @@ -698,12 +698,12 @@ std::shared_ptr buildPreamble( Result->Pragmas = std::make_shared( CapturedInfo.takePragmaIncludes()); - if (RequiredModuleBuilder) { + if (Inputs.ModulesManager) { WallTimer PrerequisiteModuleTimer; PrerequisiteModuleTimer.startTimer(); Result->RequiredModules = - RequiredModuleBuilder->buildPrerequisiteModulesFor(FileName, - Inputs.TFS); + Inputs.ModulesManager->buildPrerequisiteModulesFor(FileName, + *Inputs.TFS); PrerequisiteModuleTimer.stopTimer(); log("Built prerequisite modules for file {0} in {1} seconds", FileName, diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h index 250a3990b5b678..cfe74764e35000 100644 --- a/clang-tools-extra/clangd/Preamble.h +++ b/clang-tools-extra/clangd/Preamble.h @@ -154,13 +154,9 @@ struct PreambleBuildStats { /// If \p PreambleCallback is set, it will be run on top of the AST while /// building the preamble. /// If Stats is not non-null, build statistics will be exported there. -/// If \p RequiredModuleBuilder is not null, it will scan the source file -/// to see if it is related to modules, and if yes, modules related things -/// will be built. std::shared_ptr buildPreamble(PathRef FileName, CompilerInvocation CI, const ParseInputs &Inputs, bool StoreInMemory, - ModulesBuilder *RequiredModuleBuilder, PreambleParsedCallback PreambleCallback, PreambleBuildStats *Stats = nullptr); diff --git a/clang-tools-extra/clangd/ProjectModules.cpp b/clang-tools-extra/clangd/ProjectModules.cpp index eb2458679b00ee..976582970842b6 100644 --- a/clang-tools-extra/clangd/ProjectModules.cpp +++ b/clang-tools-extra/clangd/ProjectModules.cpp @@ -13,9 +13,7 @@ #include "clang/Tooling/DependencyScanning/DependencyScanningService.h" #include "clang/Tooling/DependencyScanning/DependencyScanningTool.h" -using namespace clang; -using namespace clang::clangd; - +namespace clang::clangd { namespace { /// A scanner to query the dependency information for C++20 Modules. /// @@ -36,8 +34,9 @@ namespace { /// interfere with each other. class ModuleDependencyScanner { public: - ModuleDependencyScanner(const GlobalCompilationDatabase &CDB, - const ThreadsafeFS &TFS) + ModuleDependencyScanner( + std::shared_ptr CDB, + const ThreadsafeFS &TFS) : CDB(CDB), TFS(TFS), Service(tooling::dependencies::ScanningMode::CanonicalPreprocessing, tooling::dependencies::ScanningOutputFormat::P1689) {} @@ -60,7 +59,7 @@ class ModuleDependencyScanner { /// a global module dependency scanner to monitor every file. Or we /// can simply require the build systems (or even the end users) /// to provide the map. - void globalScan(llvm::ArrayRef AllFiles); + void globalScan(); /// Get the source file from the module name. Note that the language /// guarantees all the module names are unique in a valid program. @@ -75,7 +74,7 @@ class ModuleDependencyScanner { std::vector getRequiredModules(PathRef File); private: - const GlobalCompilationDatabase &CDB; + std::shared_ptr CDB; const ThreadsafeFS &TFS; // Whether the scanner has scanned the project globally. @@ -91,11 +90,15 @@ class ModuleDependencyScanner { std::optional ModuleDependencyScanner::scan(PathRef FilePath) { - std::optional Cmd = CDB.getCompileCommand(FilePath); - - if (!Cmd) + auto Candidates = CDB->getCompileCommands(FilePath); + if (Candidates.empty()) return std::nullopt; + // Choose the first candidates as the compile commands as the file. + // Following the same logic with + // DirectoryBasedGlobalCompilationDatabase::getCompileCommand. + tooling::CompileCommand Cmd = std::move(Candidates.front()); + using namespace clang::tooling::dependencies; llvm::SmallString<128> FilePathDir(FilePath); @@ -103,7 +106,7 @@ ModuleDependencyScanner::scan(PathRef FilePath) { DependencyScanningTool ScanningTool(Service, TFS.view(FilePathDir)); llvm::Expected ScanningResult = - ScanningTool.getP1689ModuleDependencyFile(*Cmd, Cmd->Directory); + ScanningTool.getP1689ModuleDependencyFile(Cmd, Cmd.Directory); if (auto E = ScanningResult.takeError()) { elog("Scanning modules dependencies for {0} failed: {1}", FilePath, @@ -124,8 +127,8 @@ ModuleDependencyScanner::scan(PathRef FilePath) { return Result; } -void ModuleDependencyScanner::globalScan(llvm::ArrayRef AllFiles) { - for (auto &File : AllFiles) +void ModuleDependencyScanner::globalScan() { + for (auto &File : CDB->getAllFiles()) scan(File); GlobalScanned = true; @@ -154,9 +157,6 @@ ModuleDependencyScanner::getRequiredModules(PathRef File) { } } // namespace -namespace clang { -namespace clangd { - /// TODO: The existing `ScanningAllProjectModules` is not efficient. See the /// comments in ModuleDependencyScanner for detail. /// @@ -166,10 +166,10 @@ namespace clangd { /// the state of each file. class ScanningAllProjectModules : public ProjectModules { public: - ScanningAllProjectModules(std::vector &&AllFiles, - const GlobalCompilationDatabase &CDB, - const ThreadsafeFS &TFS) - : AllFiles(std::move(AllFiles)), Scanner(CDB, TFS) {} + ScanningAllProjectModules( + std::shared_ptr CDB, + const ThreadsafeFS &TFS) + : Scanner(CDB, TFS) {} ~ScanningAllProjectModules() override = default; @@ -182,25 +182,18 @@ class ScanningAllProjectModules : public ProjectModules { PathRef getSourceForModuleName(llvm::StringRef ModuleName, PathRef RequiredSourceFile = PathRef()) override { - Scanner.globalScan(AllFiles); + Scanner.globalScan(); return Scanner.getSourceForModuleName(ModuleName); } private: - std::vector AllFiles; - ModuleDependencyScanner Scanner; }; -std::unique_ptr ProjectModules::create( - ProjectModulesKind Kind, std::vector &&AllFiles, - const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS) { - if (Kind == ProjectModulesKind::ScanningAllFiles) - return std::make_unique(std::move(AllFiles), CDB, - TFS); - - llvm_unreachable("Unknown ProjectModulesKind."); +std::unique_ptr scanningProjectModules( + std::shared_ptr CDB, + const ThreadsafeFS &TFS) { + return std::make_unique(CDB, TFS); } -} // namespace clangd -} // namespace clang +} // namespace clang::clangd diff --git a/clang-tools-extra/clangd/ProjectModules.h b/clang-tools-extra/clangd/ProjectModules.h index 60ac586ec3e3db..3b9b564a87da01 100644 --- a/clang-tools-extra/clangd/ProjectModules.h +++ b/clang-tools-extra/clangd/ProjectModules.h @@ -9,7 +9,6 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROJECTMODULES_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROJECTMODULES_H -#include "GlobalCompilationDatabase.h" #include "support/Path.h" #include "support/ThreadsafeFS.h" @@ -37,12 +36,6 @@ namespace clangd { /// `[:partition-name]`. So module names covers partitions. class ProjectModules { public: - enum class ProjectModulesKind { ScanningAllFiles }; - - static std::unique_ptr - create(ProjectModulesKind Kind, std::vector &&AllFiles, - const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS); - virtual std::vector getRequiredModules(PathRef File) = 0; virtual PathRef getSourceForModuleName(llvm::StringRef ModuleName, diff --git a/clang-tools-extra/clangd/ScanningProjectModules.h b/clang-tools-extra/clangd/ScanningProjectModules.h new file mode 100644 index 00000000000000..b906f0f8355ee2 --- /dev/null +++ b/clang-tools-extra/clangd/ScanningProjectModules.h @@ -0,0 +1,26 @@ +//===------------ ScanningProjectModules.h -----------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SCANNINGPROJECTMODULES_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SCANNINGPROJECTMODULES_H + +#include "ProjectModules.h" +#include "clang/Tooling/CompilationDatabase.h" + +namespace clang { +namespace clangd { + +// Providing modules information for the project by scanning every file. +std::unique_ptr scanningProjectModules( + std::shared_ptr CDB, + const ThreadsafeFS &TFS); + +} // namespace clangd +} // namespace clang + +#endif diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp index d3dfa64dc7a63d..f080f418da8faa 100644 --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -652,10 +652,6 @@ class ASTWorker { TUScheduler::FileStats stats() const; bool isASTCached() const; - const GlobalCompilationDatabase &getCompilationDatabase() { return CDB; } - - ModulesBuilder *getModulesBuilder() { return FileInputs.ModulesManager; } - private: // Details of an update request that are relevant to scheduling. struct UpdateType { @@ -1089,7 +1085,7 @@ void PreambleThread::build(Request Req) { bool IsFirstPreamble = !LatestBuild; LatestBuild = clang::clangd::buildPreamble( - FileName, *Req.CI, Inputs, StoreInMemory, ASTPeer.getModulesBuilder(), + FileName, *Req.CI, Inputs, StoreInMemory, [&](CapturedASTCtx ASTCtx, std::shared_ptr PI) { Callbacks.onPreambleAST(FileName, Inputs.Version, std::move(ASTCtx), diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h index 706a13c59dd31d..fb936d46bbcf7e 100644 --- a/clang-tools-extra/clangd/TUScheduler.h +++ b/clang-tools-extra/clangd/TUScheduler.h @@ -222,9 +222,6 @@ class TUScheduler { /// Cache (large) preamble data in RAM rather than temporary files on disk. bool StorePreamblesInMemory = false; - /// Enable experimental support for modules. - bool ExperimentalModulesSupport = false; - /// Time to wait after an update to see if another one comes along. /// This tries to ensure we rebuild once the user stops typing. DebouncePolicy UpdateDebounce; diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp index 99d0f12d78f70e..994f582638b236 100644 --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -237,14 +237,8 @@ class Checker { bool buildAST() { log("Building preamble..."); - auto RequiredModuleBuilder = - Opts.ExperimentalModulesSupport - ? std::make_unique(*CDB.get()) - : nullptr; - Preamble = buildPreamble( File, *Invocation, Inputs, /*StoreInMemory=*/true, - RequiredModuleBuilder.get(), [&](CapturedASTCtx Ctx, std::shared_ptr PI) { if (!Opts.BuildDynamicSymbolIndex) diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 80a3b572d1a2a0..86eb40745cac06 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -136,7 +136,6 @@ CodeCompleteResult completions(const TestTU &TU, Position Point, MockCompilationDatabase CDB; auto Preamble = buildPreamble(testPath(TU.Filename), *CI, Inputs, /*InMemory=*/true, - /*RequiredModuleBuilder=*/nullptr, /*Callback=*/nullptr); return codeComplete(testPath(TU.Filename), Point, Preamble.get(), Inputs, Opts); @@ -1371,7 +1370,7 @@ signatures(llvm::StringRef Text, Position Point, MockCompilationDatabase CDB; auto Preamble = buildPreamble(testPath(TU.Filename), *CI, Inputs, - /*InMemory=*/true, /*RequiredModuleBuilder=*/nullptr, + /*InMemory=*/true, /*Callback=*/nullptr); if (!Preamble) { ADD_FAILURE() << "Couldn't build Preamble"; @@ -1674,7 +1673,7 @@ TEST(SignatureHelpTest, StalePreamble) { MockCompilationDatabase CDB; auto EmptyPreamble = buildPreamble(testPath(TU.Filename), *CI, Inputs, - /*InMemory=*/true, /*RequiredModuleBuilder=*/nullptr, + /*InMemory=*/true, /*Callback=*/nullptr); ASSERT_TRUE(EmptyPreamble); diff --git a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp index cb112b1bc229a1..8ffa5d7a3ba0f6 100644 --- a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp @@ -340,7 +340,7 @@ TEST(FileIndexTest, RebuildWithPreamble) { MockCompilationDatabase CDB; buildPreamble( FooCpp, *CI, PI, - /*StoreInMemory=*/true, /*RequiredModuleBuilder=*/nullptr, + /*StoreInMemory=*/true, [&](CapturedASTCtx ASTCtx, std::shared_ptr PI) { auto &Ctx = ASTCtx.getASTContext(); diff --git a/clang-tools-extra/clangd/unittests/ModulesTestSetup.h b/clang-tools-extra/clangd/unittests/ModulesTestSetup.h deleted file mode 100644 index 75e982ea2fb08a..00000000000000 --- a/clang-tools-extra/clangd/unittests/ModulesTestSetup.h +++ /dev/null @@ -1,103 +0,0 @@ -//===-- ModulesTestSetup.h - Setup the module test environment --*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "Compiler.h" -#include "support/ThreadsafeFS.h" - -#include "llvm/Support/FileSystem.h" -#include "llvm/Support/raw_ostream.h" - -#include "gmock/gmock.h" -#include "gtest/gtest.h" - -namespace clang { -namespace clangd { -class ModuleTestSetup : public ::testing::Test { -protected: - void SetUp() override { - ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("modules-test", TestDir)); - } - - void TearDown() override { llvm::sys::fs::remove_directories(TestDir); } - -public: - // Add files to the working testing directory and repalce all the - // `__DIR__` to TestDir. - void addFile(llvm::StringRef Path, llvm::StringRef Contents) { - ASSERT_FALSE(llvm::sys::path::is_absolute(Path)); - - SmallString<256> AbsPath(TestDir); - llvm::sys::path::append(AbsPath, Path); - - ASSERT_FALSE(llvm::sys::fs::create_directories( - llvm::sys::path::parent_path(AbsPath))); - - std::error_code EC; - llvm::raw_fd_ostream OS(AbsPath, EC); - ASSERT_FALSE(EC); - - std::size_t Pos = Contents.find("__DIR__"); - while (Pos != llvm::StringRef::npos) { - OS << Contents.take_front(Pos); - OS << TestDir; - Contents = Contents.drop_front(Pos + sizeof("__DIR__") - 1); - Pos = Contents.find("__DIR__"); - } - - OS << Contents; - } - - // Get the absolute path for file specified by Path under testing working - // directory. - std::string getFullPath(llvm::StringRef Path) { - SmallString<128> Result(TestDir); - llvm::sys::path::append(Result, Path); - EXPECT_TRUE(llvm::sys::fs::exists(Result.str())); - return Result.str().str(); - } - - std::unique_ptr getGlobalCompilationDatabase() { - // The compilation flags with modules are much complex so it looks better - // to use DirectoryBasedGlobalCompilationDatabase than a mocked compilation - // database. - DirectoryBasedGlobalCompilationDatabase::Options Opts(TFS); - return std::make_unique(Opts); - } - - ParseInputs getInputs(llvm::StringRef FileName, - const GlobalCompilationDatabase &CDB) { - std::string FullPathName = getFullPath(FileName); - - ParseInputs Inputs; - std::optional Cmd = - CDB.getCompileCommand(FullPathName); - EXPECT_TRUE(Cmd); - Inputs.CompileCommand = std::move(*Cmd); - Inputs.TFS = &TFS; - - if (auto Contents = TFS.view(TestDir)->getBufferForFile(FullPathName)) - Inputs.Contents = Contents->get()->getBuffer().str(); - - return Inputs; - } - - std::unique_ptr - getCompilerInvocation(const ParseInputs &Inputs) { - std::vector CC1Args; - return buildCompilerInvocation(Inputs, DiagConsumer, &CC1Args); - } - - SmallString<256> TestDir; - // Noticed MockFS but its member variable 'OverlayRealFileSystemForModules' - // implies that it will better to use RealThreadsafeFS directly. - RealThreadsafeFS TFS; - - DiagnosticConsumer DiagConsumer; -}; -} // namespace clangd -} // namespace clang diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp index 69e8cfe05bb835..d98e207b1812d4 100644 --- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp @@ -378,8 +378,7 @@ TEST(ParsedASTTest, PatchesAdditionalIncludes) { auto CI = buildCompilerInvocation(Inputs, Diags); MockCompilationDatabase CDB; auto EmptyPreamble = - buildPreamble(testPath("foo.cpp"), *CI, Inputs, true, - /*RequiredModuleBuilder=*/nullptr, nullptr); + buildPreamble(testPath("foo.cpp"), *CI, Inputs, true, nullptr); ASSERT_TRUE(EmptyPreamble); EXPECT_THAT(EmptyPreamble->Includes.MainFileIncludes, IsEmpty()); @@ -422,8 +421,7 @@ TEST(ParsedASTTest, PatchesDeletedIncludes) { auto CI = buildCompilerInvocation(Inputs, Diags); MockCompilationDatabase CDB; auto BaselinePreamble = - buildPreamble(testPath("foo.cpp"), *CI, Inputs, true, - /*RequiredModuleBuilder=*/nullptr, nullptr); + buildPreamble(testPath("foo.cpp"), *CI, Inputs, true, nullptr); ASSERT_TRUE(BaselinePreamble); EXPECT_THAT(BaselinePreamble->Includes.MainFileIncludes, ElementsAre(testing::Field(&Inclusion::Written, ""))); diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp index 149ca7819947b3..4473325cebaa23 100644 --- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp +++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp @@ -195,7 +195,7 @@ TEST(PreamblePatchTest, PatchesPreambleIncludes) { MockCompilationDatabase CDB; auto BaselinePreamble = buildPreamble(TU.Filename, *buildCompilerInvocation(PI, Diags), PI, true, - /*RequiredModuleBuilder=*/nullptr, nullptr); + nullptr); // We drop c.h from modified and add a new header. Since the latter is patched // we should only get a.h in preamble includes. d.h shouldn't be part of the // preamble, as it's coming from a disabled region. diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp index 0bb5b8392e82d2..458f719768b333 100644 --- a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp +++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp @@ -12,13 +12,103 @@ #ifndef _WIN32 #include "ModulesBuilder.h" -#include "ModulesTestSetup.h" -using namespace clang; -using namespace clang::clangd; +#include "Annotations.h" +#include "CodeComplete.h" +#include "Compiler.h" +#include "TestTU.h" +#include "support/ThreadsafeFS.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/raw_ostream.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace clang::clangd { namespace { -class PrerequisiteModulesTests : public ModuleTestSetup {}; +class PrerequisiteModulesTests : public ::testing::Test { + protected: + void SetUp() override { + ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("modules-test", TestDir)); + } + + void TearDown() override { llvm::sys::fs::remove_directories(TestDir); } + +public: + // Add files to the working testing directory and repalce all the + // `__DIR__` to TestDir. + void addFile(llvm::StringRef Path, llvm::StringRef Contents) { + ASSERT_FALSE(llvm::sys::path::is_absolute(Path)); + + SmallString<256> AbsPath(TestDir); + llvm::sys::path::append(AbsPath, Path); + + ASSERT_FALSE(llvm::sys::fs::create_directories( + llvm::sys::path::parent_path(AbsPath))); + + std::error_code EC; + llvm::raw_fd_ostream OS(AbsPath, EC); + ASSERT_FALSE(EC); + + std::size_t Pos = Contents.find("__DIR__"); + while (Pos != llvm::StringRef::npos) { + OS << Contents.take_front(Pos); + OS << TestDir; + Contents = Contents.drop_front(Pos + sizeof("__DIR__") - 1); + Pos = Contents.find("__DIR__"); + } + + OS << Contents; + } + + // Get the absolute path for file specified by Path under testing working + // directory. + std::string getFullPath(llvm::StringRef Path) { + SmallString<128> Result(TestDir); + llvm::sys::path::append(Result, Path); + EXPECT_TRUE(llvm::sys::fs::exists(Result.str())); + return Result.str().str(); + } + + std::unique_ptr getGlobalCompilationDatabase() { + // The compilation flags with modules are much complex so it looks better + // to use DirectoryBasedGlobalCompilationDatabase than a mocked compilation + // database. + DirectoryBasedGlobalCompilationDatabase::Options Opts(TFS); + return std::make_unique(Opts); + } + + ParseInputs getInputs(llvm::StringRef FileName, + const GlobalCompilationDatabase &CDB) { + std::string FullPathName = getFullPath(FileName); + + ParseInputs Inputs; + std::optional Cmd = + CDB.getCompileCommand(FullPathName); + EXPECT_TRUE(Cmd); + Inputs.CompileCommand = std::move(*Cmd); + Inputs.TFS = &TFS; + + if (auto Contents = TFS.view(TestDir)->getBufferForFile(FullPathName)) + Inputs.Contents = Contents->get()->getBuffer().str(); + + return Inputs; + } + + std::unique_ptr + getCompilerInvocation(const ParseInputs &Inputs) { + std::vector CC1Args; + return buildCompilerInvocation(Inputs, DiagConsumer, &CC1Args); + } + + SmallString<256> TestDir; + // Noticed MockFS but its member variable 'OverlayRealFileSystemForModules' + // implies that it will better to use RealThreadsafeFS directly. + RealThreadsafeFS TFS; + + DiagnosticConsumer DiagConsumer; +}; TEST_F(PrerequisiteModulesTests, PrerequisiteModulesTest) { addFile("build/compile_commands.json", R"cpp( @@ -103,18 +193,16 @@ void use() { // NonModular.cpp is not related to modules. So nothing should be built. auto NonModularInfo = - Builder.buildPrerequisiteModulesFor(getFullPath("NonModular.cpp"), &TFS); + Builder.buildPrerequisiteModulesFor(getFullPath("NonModular.cpp"), TFS); EXPECT_FALSE(NonModularInfo); - auto MInfo = Builder.buildPrerequisiteModulesFor(getFullPath("M.cppm"), &TFS); + auto MInfo = Builder.buildPrerequisiteModulesFor(getFullPath("M.cppm"), TFS); // buildPrerequisiteModulesFor won't built the module itself. EXPECT_FALSE(MInfo); // Module N shouldn't be able to be built. - auto NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), &TFS); + auto NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), TFS); EXPECT_TRUE(NInfo); - EXPECT_TRUE(NInfo->isModuleUnitBuilt("M")); - EXPECT_TRUE(NInfo->isModuleUnitBuilt("N:Part")); ParseInputs NInput = getInputs("N.cppm", *CDB); std::vector CC1Args; @@ -150,7 +238,7 @@ export int mm = 44; )cpp"); EXPECT_FALSE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); - NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), &TFS); + NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), TFS); EXPECT_TRUE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); addFile("foo.h", R"cpp( @@ -159,7 +247,7 @@ inline void foo(int) {} )cpp"); EXPECT_FALSE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); - NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), &TFS); + NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), TFS); EXPECT_TRUE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); } @@ -169,7 +257,7 @@ export module N:Part; export int NPart = 4LIdjwldijaw )cpp"); EXPECT_FALSE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); - NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), &TFS); + NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), TFS); EXPECT_TRUE(NInfo); // So NInfo should be unreusable even after rebuild. EXPECT_FALSE(NInfo->canReuse(*Invocation, TFS.view(TestDir))); @@ -180,7 +268,7 @@ export int NPart = 43; )cpp"); EXPECT_TRUE(NInfo); EXPECT_FALSE(NInfo->canReuse(*Invocation, TFS.view(TestDir))); - NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), &TFS); + NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), TFS); // So NInfo should be unreusable even after rebuild. EXPECT_TRUE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); @@ -218,6 +306,59 @@ export int nn = 43; } } +// An End-to-End test for modules. +TEST_F(PrerequisiteModulesTests, ParsedASTTest) { + addFile("A.cppm", R"cpp( +export module A; +export void printA(); + )cpp"); + + addFile("Use.cpp", R"cpp( +import A; +)cpp"); + + addFile("build/compile_commands.json", R"cpp( +[ +{ + "directory": "__DIR__", + "command": "clang++ -std=c++20 __DIR__/A.cppm -fmodule-output=__DIR__/A.pcm -c -o __DIR__/A.o", + "file": "__DIR__/A.cppm", + "output": "__DIR__/A.o" +}, +{ + "directory": "__DIR__", + "command": "clang++ -std=c++20 __DIR__/Use.cpp -c -o __DIR__/Use.o", + "file": "__DIR__/Use.cpp", + "output": "__DIR__/Use.o" +} +] + )cpp"); + + std::unique_ptr CDB = + getGlobalCompilationDatabase(); + EXPECT_TRUE(CDB); + ModulesBuilder Builder(*CDB.get()); + + ParseInputs Use = getInputs("Use.cpp", *CDB); + Use.ModulesManager = &Builder; + + std::unique_ptr CI = getCompilerInvocation(Use); + EXPECT_TRUE(CI); + + auto Preamble = buildPreamble(getFullPath("Use.cpp"), *CI, Use, /*InMemory=*/true, + /*Callback=*/nullptr); + EXPECT_TRUE(Preamble); + EXPECT_TRUE(Preamble->RequiredModules); + + auto AST = ParsedAST::build(getFullPath("Use.cpp"), Use, + std::move(CI), {}, Preamble); + EXPECT_TRUE(AST); + + const NamedDecl &D = findDecl(*AST, "printA"); + EXPECT_TRUE(D.isFromASTFile()); +} + } // namespace +} // namespace clang::clangd #endif diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp index 1cddd976a28794..f0063829e08643 100644 --- a/clang-tools-extra/clangd/unittests/TestTU.cpp +++ b/clang-tools-extra/clangd/unittests/TestTU.cpp @@ -110,7 +110,6 @@ TestTU::preamble(PreambleParsedCallback PreambleCallback) const { MockCompilationDatabase CDB; return clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs, /*StoreInMemory=*/true, - /*RequiredModuleBuilder=*/nullptr, PreambleCallback); } @@ -130,7 +129,6 @@ ParsedAST TestTU::build() const { auto Preamble = clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs, /*StoreInMemory=*/true, - /*RequiredModuleBuilder=*/nullptr, /*PreambleCallback=*/nullptr); auto AST = ParsedAST::build(testPath(Filename), Inputs, std::move(CI), Diags.take(), Preamble); From 16541c8bf44cf38497ce6fd7377ee966d61be5aa Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Fri, 14 Jun 2024 14:41:01 +0800 Subject: [PATCH 4/6] update Update update update update update Update update fmt Add MockDirectoryCompilationDatabase add test --- clang-tools-extra/clangd/CMakeLists.txt | 2 +- clang-tools-extra/clangd/ClangdLSPServer.cpp | 10 +- clang-tools-extra/clangd/ClangdLSPServer.h | 3 + clang-tools-extra/clangd/ClangdServer.cpp | 6 +- clang-tools-extra/clangd/ClangdServer.h | 8 +- clang-tools-extra/clangd/ModulesBuilder.cpp | 51 ++-- clang-tools-extra/clangd/Preamble.cpp | 10 +- clang-tools-extra/clangd/Preamble.h | 1 - ...Modules.cpp => ScanningProjectModules.cpp} | 0 .../clangd/ScanningProjectModules.h | 2 +- clang-tools-extra/clangd/TUScheduler.cpp | 2 - clang-tools-extra/clangd/tool/Check.cpp | 7 +- clang-tools-extra/clangd/tool/ClangdMain.cpp | 2 +- .../clangd/unittests/CodeCompleteTests.cpp | 21 +- .../clangd/unittests/FileIndexTests.cpp | 2 - .../clangd/unittests/ParsedASTTests.cpp | 2 - .../clangd/unittests/PreambleTests.cpp | 6 +- .../unittests/PrerequisiteModulesTest.cpp | 247 +++++++++--------- clang-tools-extra/clangd/unittests/TestFS.h | 2 +- clang-tools-extra/clangd/unittests/TestTU.cpp | 12 +- 20 files changed, 179 insertions(+), 217 deletions(-) rename clang-tools-extra/clangd/{ProjectModules.cpp => ScanningProjectModules.cpp} (100%) diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt index 393aa795bda05d..c21d277d2ffcbd 100644 --- a/clang-tools-extra/clangd/CMakeLists.txt +++ b/clang-tools-extra/clangd/CMakeLists.txt @@ -99,12 +99,12 @@ add_clang_library(clangDaemon JSONTransport.cpp ModulesBuilder.cpp PathMapping.cpp - ProjectModules.cpp Protocol.cpp Quality.cpp ParsedAST.cpp Preamble.cpp RIFF.cpp + ScanningProjectModules.cpp Selection.cpp SemanticHighlighting.cpp SemanticSelection.cpp diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index c99f172dac4e42..20c52cbe78eb8c 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -52,6 +52,9 @@ namespace clang { namespace clangd { + +extern llvm::cl::opt ExperimentalModulesSupport; + namespace { // Tracks end-to-end latency of high level lsp calls. Measurements are in // seconds. @@ -565,8 +568,10 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params, CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags, std::move(Mangler)); - if (Opts.ExperimentalModulesSupport) + if (Opts.EnableExperimentalModulesSupport) { ModulesManager.emplace(*CDB); + Opts.ModulesManager = &*ModulesManager; + } { // Switch caller's context with LSPServer's background context. Since we @@ -577,8 +582,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params, if (Opts.Encoding) WithOffsetEncoding.emplace(kCurrentOffsetEncoding, *Opts.Encoding); Server.emplace(*CDB, TFS, Opts, - static_cast(this), - ModulesManager ? &*ModulesManager : nullptr); + static_cast(this)); } llvm::json::Object ServerCaps{ diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h index 0d2fea31cf5101..0b8e4720f53236 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -63,6 +63,9 @@ class ClangdLSPServer : private ClangdServer::Callbacks, /// Limit the number of references returned (0 means no limit). size_t ReferencesLimit = 0; + + /// Flag to hint the experimental modules support is enabled. + bool EnableExperimentalModulesSupport = false; }; ClangdLSPServer(Transport &Transp, const ThreadsafeFS &TFS, diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 0771a0d1a87b20..e910a80ba0bae9 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -213,10 +213,11 @@ ClangdServer::Options::operator TUScheduler::Options() const { ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS, const Options &Opts, - Callbacks *Callbacks, ModulesBuilder *ModulesManager) + Callbacks *Callbacks) : FeatureModules(Opts.FeatureModules), CDB(CDB), TFS(TFS), DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr), - ModulesManager(ModulesManager), ClangTidyProvider(Opts.ClangTidyProvider), + ModulesManager(Opts.ModulesManager), + ClangTidyProvider(Opts.ClangTidyProvider), UseDirtyHeaders(Opts.UseDirtyHeaders), LineFoldingOnly(Opts.LineFoldingOnly), PreambleParseForwardingFunctions(Opts.PreambleParseForwardingFunctions), @@ -228,7 +229,6 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB, DirtyFS(std::make_unique(TFS, DraftMgr)) { if (Opts.AsyncThreadsCount != 0) IndexTasks.emplace(); - // Pass a callback into `WorkScheduler` to extract symbols from a newly // parsed file and rebuild the file index synchronously each time an AST // is parsed. diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index d387652fb6a276..a653cdb56b751b 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -43,7 +43,6 @@ namespace clang { namespace clangd { - /// Manages a collection of source files and derived data (ASTs, indexes), /// and provides language-aware features such as code completion. /// @@ -114,8 +113,8 @@ class ClangdServer { /// This throttler controls which preambles may be built at a given time. clangd::PreambleThrottler *PreambleThrottler = nullptr; - /// Enable experimental support for modules. - bool ExperimentalModulesSupport = false; + /// Manages to build module files. + ModulesBuilder *ModulesManager = nullptr; /// If true, ClangdServer builds a dynamic in-memory index for symbols in /// opened files and uses the index to augment code completion results. @@ -204,8 +203,7 @@ class ClangdServer { /// those arguments for subsequent reparses. However, ClangdServer will check /// if compilation arguments changed on calls to forceReparse(). ClangdServer(const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS, - const Options &Opts, Callbacks *Callbacks = nullptr, - ModulesBuilder *ModulesManager = nullptr); + const Options &Opts, Callbacks *Callbacks = nullptr); ~ClangdServer(); /// Gets the installed feature module of a given type, if any. diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 4b40961042f93e..15641d205b3802 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -61,19 +61,6 @@ llvm::SmallString<256> getUniqueModuleFilesPath(PathRef MainFile) { return Result; } -// Get the absolute path for the filename from the compile command. -llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand &Cmd) { - llvm::SmallString<128> AbsolutePath; - if (llvm::sys::path::is_absolute(Cmd.Filename)) { - AbsolutePath = Cmd.Filename; - } else { - AbsolutePath = Cmd.Directory; - llvm::sys::path::append(AbsolutePath, Cmd.Filename); - llvm::sys::path::remove_dots(AbsolutePath, true); - } - return AbsolutePath; -} - // Get a unique module file path under \param ModuleFilesPrefix. std::string getModuleFilePath(llvm::StringRef ModuleName, PathRef ModuleFilesPrefix) { @@ -181,19 +168,17 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { llvm::StringSet<> BuiltModuleNames; }; -// Build a module file for module with `ModuleName`. Return false -// when there are problem happens. Return true when the -// module file exists or built successfully. The information of built +// Build a module file for module with `ModuleName`. The information of built // module file are stored in \param BuiltModuleFiles. llvm::Error buildModuleFile(llvm::StringRef ModuleName, const GlobalCompilationDatabase &CDB, - const ThreadsafeFS &TFS, ProjectModules *MDB, + const ThreadsafeFS &TFS, ProjectModules &MDB, PathRef ModuleFilesPrefix, StandalonePrerequisiteModules &BuiltModuleFiles) { if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) return llvm::Error::success(); - PathRef ModuleUnitFileName = MDB->getSourceForModuleName(ModuleName); + PathRef ModuleUnitFileName = MDB.getSourceForModuleName(ModuleName); // It is possible that we're meeting third party modules (modules whose // source are not in the project. e.g, the std module may be a third-party // module for most projects) or something wrong with the implementation of @@ -205,19 +190,22 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, return llvm::createStringError(llvm::formatv( "Failed to build '{0}': Failed to get the primary source", ModuleName)); - for (auto &RequiredModuleName : MDB->getRequiredModules(ModuleUnitFileName)) { - // Return early if there are errors building the module file. - if (llvm::Error Err = buildModuleFile(RequiredModuleName, CDB, TFS, MDB, - ModuleFilesPrefix, BuiltModuleFiles)) - return Err; - } - + // Try cheap operation earlier to boil-out cheaply if there are problems. auto Cmd = CDB.getCompileCommand(ModuleUnitFileName); if (!Cmd) return llvm::createStringError( llvm::formatv("Failed to build '{0}': No compile command for {1}", ModuleName, ModuleUnitFileName)); + for (auto &RequiredModuleName : MDB.getRequiredModules(ModuleUnitFileName)) { + // Return early if there are errors building the module file. + if (llvm::Error Err = buildModuleFile(RequiredModuleName, CDB, TFS, MDB, + ModuleFilesPrefix, BuiltModuleFiles)) + return llvm::createStringError( + llvm::formatv("Failed to build dependency {0}: {1}", + RequiredModuleName, toString(std::move(Err)))); + } + Cmd->Output = getModuleFilePath(ModuleName, ModuleFilesPrefix); ParseInputs Inputs; @@ -271,12 +259,14 @@ std::unique_ptr ModulesBuilder::buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS &TFS) const { std::unique_ptr MDB = CDB.getProjectModules(File); - if (!MDB) - return {}; + if (!MDB) { + elog("Failed to get Project Modules information for {0}", File); + return std::make_unique(); + } std::vector RequiredModuleNames = MDB->getRequiredModules(File); if (RequiredModuleNames.empty()) - return {}; + return std::make_unique(); llvm::SmallString<256> ModuleFilesPrefix = getUniqueModuleFilesPath(File); @@ -288,7 +278,7 @@ ModulesBuilder::buildPrerequisiteModulesFor(PathRef File, for (llvm::StringRef RequiredModuleName : RequiredModuleNames) { // Return early if there is any error. if (llvm::Error Err = - buildModuleFile(RequiredModuleName, CDB, TFS, MDB.get(), + buildModuleFile(RequiredModuleName, CDB, TFS, *MDB.get(), ModuleFilesPrefix, *RequiredModules.get())) { elog("Failed to build module {0}; due to {1}", RequiredModuleName, toString(std::move(Err))); @@ -304,6 +294,9 @@ ModulesBuilder::buildPrerequisiteModulesFor(PathRef File, bool StandalonePrerequisiteModules::canReuse( const CompilerInvocation &CI, llvm::IntrusiveRefCntPtr VFS) const { + if (RequiredModules.empty()) + return true; + CompilerInstance Clang; Clang.setInvocation(std::make_shared(CI)); diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp index d70c7ad245a42b..dd13b1a9e5613d 100644 --- a/clang-tools-extra/clangd/Preamble.cpp +++ b/clang-tools-extra/clangd/Preamble.cpp @@ -587,10 +587,11 @@ class DiagPatcher { }; } // namespace -std::shared_ptr buildPreamble( - PathRef FileName, CompilerInvocation CI, const ParseInputs &Inputs, - bool StoreInMemory, PreambleParsedCallback PreambleCallback, - PreambleBuildStats *Stats) { +std::shared_ptr +buildPreamble(PathRef FileName, CompilerInvocation CI, + const ParseInputs &Inputs, bool StoreInMemory, + PreambleParsedCallback PreambleCallback, + PreambleBuildStats *Stats) { // Note that we don't need to copy the input contents, preamble can live // without those. auto ContentsBuffer = @@ -659,7 +660,6 @@ std::shared_ptr buildPreamble( WallTimer PreambleTimer; PreambleTimer.startTimer(); - auto BuiltPreamble = PrecompiledPreamble::Build( CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, Stats ? TimedFS : StatCacheFS, std::make_shared(), diff --git a/clang-tools-extra/clangd/Preamble.h b/clang-tools-extra/clangd/Preamble.h index cfe74764e35000..be8fed4ab88cdd 100644 --- a/clang-tools-extra/clangd/Preamble.h +++ b/clang-tools-extra/clangd/Preamble.h @@ -27,7 +27,6 @@ #include "Diagnostics.h" #include "FS.h" #include "Headers.h" - #include "ModulesBuilder.h" #include "clang-include-cleaner/Record.h" diff --git a/clang-tools-extra/clangd/ProjectModules.cpp b/clang-tools-extra/clangd/ScanningProjectModules.cpp similarity index 100% rename from clang-tools-extra/clangd/ProjectModules.cpp rename to clang-tools-extra/clangd/ScanningProjectModules.cpp diff --git a/clang-tools-extra/clangd/ScanningProjectModules.h b/clang-tools-extra/clangd/ScanningProjectModules.h index b906f0f8355ee2..75fc7dbcebce54 100644 --- a/clang-tools-extra/clangd/ScanningProjectModules.h +++ b/clang-tools-extra/clangd/ScanningProjectModules.h @@ -15,7 +15,7 @@ namespace clang { namespace clangd { -// Providing modules information for the project by scanning every file. +/// Providing modules information for the project by scanning every file. std::unique_ptr scanningProjectModules( std::shared_ptr CDB, const ThreadsafeFS &TFS); diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp index f080f418da8faa..324ba1fc8cb895 100644 --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -710,7 +710,6 @@ class ASTWorker { TUScheduler::ASTCache &IdleASTs; TUScheduler::HeaderIncluderCache &HeaderIncluders; const bool RunSync; - /// Time to wait after an update to see whether another update obsoletes it. const DebouncePolicy UpdateDebounce; /// File that ASTWorker is responsible for. @@ -1083,7 +1082,6 @@ void PreambleThread::build(Request Req) { PreambleBuildStats Stats; bool IsFirstPreamble = !LatestBuild; - LatestBuild = clang::clangd::buildPreamble( FileName, *Req.CI, Inputs, StoreInMemory, [&](CapturedASTCtx ASTCtx, diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp index 994f582638b236..25005ec1bd0453 100644 --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -146,8 +146,6 @@ class Checker { ClangdLSPServer::Options Opts; // from buildCommand tooling::CompileCommand Cmd; - std::unique_ptr BaseCDB; - std::unique_ptr CDB; // from buildInvocation ParseInputs Inputs; std::unique_ptr Invocation; @@ -170,14 +168,14 @@ class Checker { DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS); CDBOpts.CompileCommandsDir = Config::current().CompileFlags.CDBSearch.FixedCDBPath; - BaseCDB = + std::unique_ptr BaseCDB = std::make_unique(CDBOpts); auto Mangler = CommandMangler::detect(); Mangler.SystemIncludeExtractor = getSystemIncludeExtractor(llvm::ArrayRef(Opts.QueryDriverGlobs)); if (Opts.ResourceDir) Mangler.ResourceDir = *Opts.ResourceDir; - CDB = std::make_unique( + auto CDB = std::make_unique( BaseCDB.get(), std::vector{}, std::move(Mangler)); if (auto TrueCmd = CDB->getCompileCommand(File)) { @@ -236,7 +234,6 @@ class Checker { // Build preamble and AST, and index them. bool buildAST() { log("Building preamble..."); - Preamble = buildPreamble( File, *Invocation, Inputs, /*StoreInMemory=*/true, [&](CapturedASTCtx Ctx, diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp index 0cb18b5ee86e23..8dbf3d1aa35ffe 100644 --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -869,7 +869,7 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var ClangdLSPServer::Options Opts; Opts.UseDirBasedCDB = (CompileArgsFrom == FilesystemCompileArgs); - Opts.ExperimentalModulesSupport = ExperimentalModulesSupport; + Opts.EnableExperimentalModulesSupport = ExperimentalModulesSupport; switch (PCHStorage) { case PCHStorageFlag::Memory: diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 86eb40745cac06..96d1ee1f0add73 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -132,11 +132,8 @@ CodeCompleteResult completions(const TestTU &TU, Position Point, ADD_FAILURE() << "Couldn't build CompilerInvocation"; return {}; } - - MockCompilationDatabase CDB; auto Preamble = buildPreamble(testPath(TU.Filename), *CI, Inputs, - /*InMemory=*/true, - /*Callback=*/nullptr); + /*InMemory=*/true, /*Callback=*/nullptr); return codeComplete(testPath(TU.Filename), Point, Preamble.get(), Inputs, Opts); } @@ -1366,12 +1363,8 @@ signatures(llvm::StringRef Text, Position Point, ADD_FAILURE() << "Couldn't build CompilerInvocation"; return {}; } - - MockCompilationDatabase CDB; - auto Preamble = - buildPreamble(testPath(TU.Filename), *CI, Inputs, - /*InMemory=*/true, - /*Callback=*/nullptr); + auto Preamble = buildPreamble(testPath(TU.Filename), *CI, Inputs, + /*InMemory=*/true, /*Callback=*/nullptr); if (!Preamble) { ADD_FAILURE() << "Couldn't build Preamble"; return {}; @@ -1669,12 +1662,8 @@ TEST(SignatureHelpTest, StalePreamble) { auto Inputs = TU.inputs(FS); auto CI = buildCompilerInvocation(Inputs, Diags); ASSERT_TRUE(CI); - - MockCompilationDatabase CDB; - auto EmptyPreamble = - buildPreamble(testPath(TU.Filename), *CI, Inputs, - /*InMemory=*/true, - /*Callback=*/nullptr); + auto EmptyPreamble = buildPreamble(testPath(TU.Filename), *CI, Inputs, + /*InMemory=*/true, /*Callback=*/nullptr); ASSERT_TRUE(EmptyPreamble); TU.AdditionalFiles["a.h"] = "int foo(int x);"; diff --git a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp index 8ffa5d7a3ba0f6..9f713564b2c01f 100644 --- a/clang-tools-extra/clangd/unittests/FileIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/FileIndexTests.cpp @@ -336,8 +336,6 @@ TEST(FileIndexTest, RebuildWithPreamble) { FileIndex Index; bool IndexUpdated = false; - - MockCompilationDatabase CDB; buildPreamble( FooCpp, *CI, PI, /*StoreInMemory=*/true, diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp index d98e207b1812d4..4bb76cd6ab8304 100644 --- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp @@ -376,7 +376,6 @@ TEST(ParsedASTTest, PatchesAdditionalIncludes) { MockFS FS; auto Inputs = TU.inputs(FS); auto CI = buildCompilerInvocation(Inputs, Diags); - MockCompilationDatabase CDB; auto EmptyPreamble = buildPreamble(testPath("foo.cpp"), *CI, Inputs, true, nullptr); ASSERT_TRUE(EmptyPreamble); @@ -419,7 +418,6 @@ TEST(ParsedASTTest, PatchesDeletedIncludes) { MockFS FS; auto Inputs = TU.inputs(FS); auto CI = buildCompilerInvocation(Inputs, Diags); - MockCompilationDatabase CDB; auto BaselinePreamble = buildPreamble(testPath("foo.cpp"), *CI, Inputs, true, nullptr); ASSERT_TRUE(BaselinePreamble); diff --git a/clang-tools-extra/clangd/unittests/PreambleTests.cpp b/clang-tools-extra/clangd/unittests/PreambleTests.cpp index 4473325cebaa23..6420516e785576 100644 --- a/clang-tools-extra/clangd/unittests/PreambleTests.cpp +++ b/clang-tools-extra/clangd/unittests/PreambleTests.cpp @@ -192,10 +192,8 @@ TEST(PreamblePatchTest, PatchesPreambleIncludes) { TU.AdditionalFiles["b.h"] = ""; TU.AdditionalFiles["c.h"] = ""; auto PI = TU.inputs(FS); - MockCompilationDatabase CDB; - auto BaselinePreamble = - buildPreamble(TU.Filename, *buildCompilerInvocation(PI, Diags), PI, true, - nullptr); + auto BaselinePreamble = buildPreamble( + TU.Filename, *buildCompilerInvocation(PI, Diags), PI, true, nullptr); // We drop c.h from modified and add a new header. Since the latter is patched // we should only get a.h in preamble includes. d.h shouldn't be part of the // preamble, as it's coming from a disabled region. diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp index 458f719768b333..8cbedcf0631a0b 100644 --- a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp +++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp @@ -12,6 +12,7 @@ #ifndef _WIN32 #include "ModulesBuilder.h" +#include "ScanningProjectModules.h" #include "Annotations.h" #include "CodeComplete.h" @@ -27,41 +28,80 @@ namespace clang::clangd { namespace { -class PrerequisiteModulesTests : public ::testing::Test { - protected: - void SetUp() override { - ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("modules-test", TestDir)); + +class MockDirectoryCompilationDatabase : public MockCompilationDatabase { +public: + MockDirectoryCompilationDatabase(StringRef TestDir, const ThreadsafeFS &TFS) + : MockCompilationDatabase(TestDir), + MockedCDBPtr(std::make_shared(*this)), + TFS(TFS) { + this->ExtraClangFlags.push_back("-std=c++20"); + this->ExtraClangFlags.push_back("-c"); } - void TearDown() override { llvm::sys::fs::remove_directories(TestDir); } + void addFile(llvm::StringRef Path, llvm::StringRef Contents); -public: - // Add files to the working testing directory and repalce all the - // `__DIR__` to TestDir. - void addFile(llvm::StringRef Path, llvm::StringRef Contents) { - ASSERT_FALSE(llvm::sys::path::is_absolute(Path)); - - SmallString<256> AbsPath(TestDir); - llvm::sys::path::append(AbsPath, Path); - - ASSERT_FALSE(llvm::sys::fs::create_directories( - llvm::sys::path::parent_path(AbsPath))); - - std::error_code EC; - llvm::raw_fd_ostream OS(AbsPath, EC); - ASSERT_FALSE(EC); - - std::size_t Pos = Contents.find("__DIR__"); - while (Pos != llvm::StringRef::npos) { - OS << Contents.take_front(Pos); - OS << TestDir; - Contents = Contents.drop_front(Pos + sizeof("__DIR__") - 1); - Pos = Contents.find("__DIR__"); + std::unique_ptr getProjectModules(PathRef) const override { + return scanningProjectModules(MockedCDBPtr, TFS); + } + +private: + class MockClangCompilationDatabase : public tooling::CompilationDatabase { + public: + MockClangCompilationDatabase(MockDirectoryCompilationDatabase &MCDB) + : MCDB(MCDB) {} + + std::vector + getCompileCommands(StringRef FilePath) const override { + std::optional Cmd = + MCDB.getCompileCommand(FilePath); + EXPECT_TRUE(Cmd); + return {*Cmd}; } - OS << Contents; + std::vector getAllFiles() const override { return Files; } + + void AddFile(StringRef File) { Files.push_back(File.str()); } + + private: + MockDirectoryCompilationDatabase &MCDB; + std::vector Files; + }; + + std::shared_ptr MockedCDBPtr; + const ThreadsafeFS &TFS; +}; + +// Add files to the working testing directory and the compilation database. +void MockDirectoryCompilationDatabase::addFile(llvm::StringRef Path, + llvm::StringRef Contents) { + ASSERT_FALSE(llvm::sys::path::is_absolute(Path)); + + SmallString<256> AbsPath(Directory); + llvm::sys::path::append(AbsPath, Path); + + ASSERT_FALSE( + llvm::sys::fs::create_directories(llvm::sys::path::parent_path(AbsPath))); + + std::error_code EC; + llvm::raw_fd_ostream OS(AbsPath, EC); + ASSERT_FALSE(EC); + OS << Contents; + + MockedCDBPtr->AddFile(Path); +} + +class PrerequisiteModulesTests : public ::testing::Test { +protected: + void SetUp() override { + ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("modules-test", TestDir)); + } + + void TearDown() override { + ASSERT_FALSE(llvm::sys::fs::remove_directories(TestDir)); } +public: // Get the absolute path for file specified by Path under testing working // directory. std::string getFullPath(llvm::StringRef Path) { @@ -96,12 +136,6 @@ class PrerequisiteModulesTests : public ::testing::Test { return Inputs; } - std::unique_ptr - getCompilerInvocation(const ParseInputs &Inputs) { - std::vector CC1Args; - return buildCompilerInvocation(Inputs, DiagConsumer, &CC1Args); - } - SmallString<256> TestDir; // Noticed MockFS but its member variable 'OverlayRealFileSystemForModules' // implies that it will better to use RealThreadsafeFS directly. @@ -111,73 +145,40 @@ class PrerequisiteModulesTests : public ::testing::Test { }; TEST_F(PrerequisiteModulesTests, PrerequisiteModulesTest) { - addFile("build/compile_commands.json", R"cpp( -[ -{ - "directory": "__DIR__", - "command": "clang++ -std=c++20 __DIR__/M.cppm -fmodule-output=__DIR__/M.pcm -c -o __DIR__/M.o", - "file": "__DIR__/M.cppm", - "output": "__DIR__/M.o" -}, -{ - "directory": "__DIR__", - "command": "clang++ -std=c++20 __DIR__/N.cppm -fmodule-file=M=__DIR__/M.pcm -fmodule-file=N:Part=__DIR__/N-partition.pcm -fprebuilt-module-path=__DIR__ -fmodule-output=__DIR__/N.pcm -c -o __DIR__/N.o", - "file": "__DIR__/N.cppm", - "output": "__DIR__/N.o" -}, -{ - "directory": "__DIR__", - "command": "clang++ -std=c++20 __DIR__/N-part.cppm -fmodule-output=__DIR__/N-partition.pcm -c -o __DIR__/N-part.o", - "file": "__DIR__/N-part.cppm", - "output": "__DIR__/N-part.o" -}, -{ - "directory": "__DIR__", - "command": "clang++ -std=c++20 __DIR__/L.cppm -fmodule-output=__DIR__/L.pcm -c -o __DIR__/L.o", - "file": "__DIR__/L.cppm", - "output": "__DIR__/L.o" -}, -{ - "directory": "__DIR__", - "command": "clang++ -std=c++20 __DIR__/NonModular.cpp -c -o __DIR__/NonModular.o", - "file": "__DIR__/NonModular.cpp", - "output": "__DIR__/NonModular.o" -} -] - )cpp"); + MockDirectoryCompilationDatabase CDB(TestDir, TFS); - addFile("foo.h", R"cpp( + CDB.addFile("foo.h", R"cpp( inline void foo() {} )cpp"); - addFile("M.cppm", R"cpp( + CDB.addFile("M.cppm", R"cpp( module; #include "foo.h" export module M; )cpp"); - addFile("N.cppm", R"cpp( + CDB.addFile("N.cppm", R"cpp( export module N; import :Part; import M; )cpp"); - addFile("N-part.cppm", R"cpp( + CDB.addFile("N-part.cppm", R"cpp( // Different name with filename intentionally. export module N:Part; )cpp"); - addFile("bar.h", R"cpp( + CDB.addFile("bar.h", R"cpp( inline void bar() {} )cpp"); - addFile("L.cppm", R"cpp( + CDB.addFile("L.cppm", R"cpp( module; #include "bar.h" export module L; )cpp"); - addFile("NonModular.cpp", R"cpp( + CDB.addFile("NonModular.cpp", R"cpp( #include "bar.h" #include "foo.h" void use() { @@ -186,34 +187,41 @@ void use() { } )cpp"); - std::unique_ptr CDB = - getGlobalCompilationDatabase(); - EXPECT_TRUE(CDB); - ModulesBuilder Builder(*CDB.get()); + ModulesBuilder Builder(CDB); // NonModular.cpp is not related to modules. So nothing should be built. - auto NonModularInfo = - Builder.buildPrerequisiteModulesFor(getFullPath("NonModular.cpp"), TFS); - EXPECT_FALSE(NonModularInfo); + { + auto NonModularInfo = + Builder.buildPrerequisiteModulesFor(getFullPath("NonModular.cpp"), TFS); + EXPECT_TRUE(NonModularInfo); + auto Invocation = + buildCompilerInvocation(getInputs("NonModular.cpp", CDB), DiagConsumer); + EXPECT_TRUE(NonModularInfo->canReuse(*Invocation, TFS.view(TestDir))); + } - auto MInfo = Builder.buildPrerequisiteModulesFor(getFullPath("M.cppm"), TFS); - // buildPrerequisiteModulesFor won't built the module itself. - EXPECT_FALSE(MInfo); + { + auto MInfo = + Builder.buildPrerequisiteModulesFor(getFullPath("M.cppm"), TFS); + // buildPrerequisiteModulesFor won't built the module itself. + EXPECT_TRUE(MInfo); + auto Invocation = + buildCompilerInvocation(getInputs("M.cppm", CDB), DiagConsumer); + EXPECT_TRUE(MInfo->canReuse(*Invocation, TFS.view(TestDir))); + } // Module N shouldn't be able to be built. auto NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), TFS); EXPECT_TRUE(NInfo); - ParseInputs NInput = getInputs("N.cppm", *CDB); - std::vector CC1Args; + ParseInputs NInput = getInputs("N.cppm", CDB); std::unique_ptr Invocation = - getCompilerInvocation(NInput); + buildCompilerInvocation(NInput, DiagConsumer); // Test that `PrerequisiteModules::canReuse` works basically. EXPECT_TRUE(NInfo->canReuse(*Invocation, TFS.view(TestDir))); // Test that we can still reuse the NInfo after we touch a unrelated file. { - addFile("L.cppm", R"cpp( + CDB.addFile("L.cppm", R"cpp( module; #include "bar.h" export module L; @@ -221,7 +229,7 @@ export int ll = 43; )cpp"); EXPECT_TRUE(NInfo->canReuse(*Invocation, TFS.view(TestDir))); - addFile("bar.h", R"cpp( + CDB.addFile("bar.h", R"cpp( inline void bar() {} inline void bar(int) {} )cpp"); @@ -230,7 +238,7 @@ inline void bar(int) {} // Test that we can't reuse the NInfo after we touch a related file. { - addFile("M.cppm", R"cpp( + CDB.addFile("M.cppm", R"cpp( module; #include "foo.h" export module M; @@ -241,7 +249,7 @@ export int mm = 44; NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), TFS); EXPECT_TRUE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); - addFile("foo.h", R"cpp( + CDB.addFile("foo.h", R"cpp( inline void foo() {} inline void foo(int) {} )cpp"); @@ -251,7 +259,7 @@ inline void foo(int) {} EXPECT_TRUE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); } - addFile("N-part.cppm", R"cpp( + CDB.addFile("N-part.cppm", R"cpp( export module N:Part; // Intentioned to make it uncompilable. export int NPart = 4LIdjwldijaw @@ -262,7 +270,7 @@ export int NPart = 4LIdjwldijaw // So NInfo should be unreusable even after rebuild. EXPECT_FALSE(NInfo->canReuse(*Invocation, TFS.view(TestDir))); - addFile("N-part.cppm", R"cpp( + CDB.addFile("N-part.cppm", R"cpp( export module N:Part; export int NPart = 43; )cpp"); @@ -274,13 +282,13 @@ export int NPart = 43; // Test that if we changed the modification time of the file, the module files // info is still reusable if its content doesn't change. - addFile("N-part.cppm", R"cpp( + CDB.addFile("N-part.cppm", R"cpp( export module N:Part; export int NPart = 43; )cpp"); EXPECT_TRUE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); - addFile("N.cppm", R"cpp( + CDB.addFile("N.cppm", R"cpp( export module N; import :Part; import M; @@ -294,10 +302,9 @@ export int nn = 43; // Check that // `PrerequisiteModules::adjustHeaderSearchOptions(HeaderSearchOptions&)` // can replace HeaderSearchOptions correctly. - ParseInputs NInput = getInputs("N.cppm", *CDB); - std::vector CC1Args; + ParseInputs NInput = getInputs("N.cppm", CDB); std::unique_ptr NInvocation = - getCompilerInvocation(NInput); + buildCompilerInvocation(NInput, DiagConsumer); HeaderSearchOptions &HSOpts = NInvocation->getHeaderSearchOpts(); NInfo->adjustHeaderSearchOptions(HSOpts); @@ -308,50 +315,34 @@ export int nn = 43; // An End-to-End test for modules. TEST_F(PrerequisiteModulesTests, ParsedASTTest) { - addFile("A.cppm", R"cpp( + MockDirectoryCompilationDatabase CDB(TestDir, TFS); + + CDB.addFile("A.cppm", R"cpp( export module A; export void printA(); )cpp"); - addFile("Use.cpp", R"cpp( + CDB.addFile("Use.cpp", R"cpp( import A; )cpp"); - addFile("build/compile_commands.json", R"cpp( -[ -{ - "directory": "__DIR__", - "command": "clang++ -std=c++20 __DIR__/A.cppm -fmodule-output=__DIR__/A.pcm -c -o __DIR__/A.o", - "file": "__DIR__/A.cppm", - "output": "__DIR__/A.o" -}, -{ - "directory": "__DIR__", - "command": "clang++ -std=c++20 __DIR__/Use.cpp -c -o __DIR__/Use.o", - "file": "__DIR__/Use.cpp", - "output": "__DIR__/Use.o" -} -] - )cpp"); - - std::unique_ptr CDB = - getGlobalCompilationDatabase(); - EXPECT_TRUE(CDB); - ModulesBuilder Builder(*CDB.get()); + ModulesBuilder Builder(CDB); - ParseInputs Use = getInputs("Use.cpp", *CDB); + ParseInputs Use = getInputs("Use.cpp", CDB); Use.ModulesManager = &Builder; - std::unique_ptr CI = getCompilerInvocation(Use); + std::unique_ptr CI = + buildCompilerInvocation(Use, DiagConsumer); EXPECT_TRUE(CI); - auto Preamble = buildPreamble(getFullPath("Use.cpp"), *CI, Use, /*InMemory=*/true, - /*Callback=*/nullptr); + auto Preamble = + buildPreamble(getFullPath("Use.cpp"), *CI, Use, /*InMemory=*/true, + /*Callback=*/nullptr); EXPECT_TRUE(Preamble); EXPECT_TRUE(Preamble->RequiredModules); - auto AST = ParsedAST::build(getFullPath("Use.cpp"), Use, - std::move(CI), {}, Preamble); + auto AST = ParsedAST::build(getFullPath("Use.cpp"), Use, std::move(CI), {}, + Preamble); EXPECT_TRUE(AST); const NamedDecl &D = findDecl(*AST, "printA"); diff --git a/clang-tools-extra/clangd/unittests/TestFS.h b/clang-tools-extra/clangd/unittests/TestFS.h index 6bdadc9c07439d..568533f3b3b911 100644 --- a/clang-tools-extra/clangd/unittests/TestFS.h +++ b/clang-tools-extra/clangd/unittests/TestFS.h @@ -67,7 +67,7 @@ class MockCompilationDatabase : public GlobalCompilationDatabase { std::vector ExtraClangFlags; -private: +protected: StringRef Directory; StringRef RelPathPrefix; }; diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp index f0063829e08643..1f02c04125b1ea 100644 --- a/clang-tools-extra/clangd/unittests/TestTU.cpp +++ b/clang-tools-extra/clangd/unittests/TestTU.cpp @@ -107,10 +107,8 @@ TestTU::preamble(PreambleParsedCallback PreambleCallback) const { initializeModuleCache(*CI); auto ModuleCacheDeleter = llvm::make_scope_exit( std::bind(deleteModuleCache, CI->getHeaderSearchOpts().ModuleCachePath)); - MockCompilationDatabase CDB; return clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs, - /*StoreInMemory=*/true, - PreambleCallback); + /*StoreInMemory=*/true, PreambleCallback); } ParsedAST TestTU::build() const { @@ -125,11 +123,9 @@ ParsedAST TestTU::build() const { auto ModuleCacheDeleter = llvm::make_scope_exit( std::bind(deleteModuleCache, CI->getHeaderSearchOpts().ModuleCachePath)); - MockCompilationDatabase CDB; - auto Preamble = - clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs, - /*StoreInMemory=*/true, - /*PreambleCallback=*/nullptr); + auto Preamble = clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs, + /*StoreInMemory=*/true, + /*PreambleCallback=*/nullptr); auto AST = ParsedAST::build(testPath(Filename), Inputs, std::move(CI), Diags.take(), Preamble); if (!AST) { From 12eeb750ccecb49c36e3b06cee7e4d78a548be48 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Tue, 16 Jul 2024 14:37:02 +0800 Subject: [PATCH 5/6] Address comments And use GenerateReducedModuleInterface and suppress ODR check in GMF --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 2 - clang-tools-extra/clangd/ModulesBuilder.cpp | 43 ++-- clang-tools-extra/clangd/ModulesBuilder.h | 4 - .../clangd/ScanningProjectModules.cpp | 9 +- clang-tools-extra/clangd/tool/Check.cpp | 12 +- .../unittests/PrerequisiteModulesTest.cpp | 228 +++++++++++------- 6 files changed, 178 insertions(+), 120 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 20c52cbe78eb8c..06573a57554245 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -53,8 +53,6 @@ namespace clang { namespace clangd { -extern llvm::cl::opt ExperimentalModulesSupport; - namespace { // Tracks end-to-end latency of high level lsp calls. Measurements are in // seconds. diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 15641d205b3802..3fcc6e61bb3d77 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -7,13 +7,10 @@ //===----------------------------------------------------------------------===// #include "ModulesBuilder.h" - #include "Compiler.h" #include "support/Logger.h" - #include "clang/Frontend/FrontendAction.h" #include "clang/Frontend/FrontendActions.h" - #include "clang/Serialization/ASTReader.h" namespace clang { @@ -142,8 +139,6 @@ class StandalonePrerequisiteModules : public PrerequisiteModules { ModuleFile(llvm::StringRef ModuleName, PathRef ModuleFilePath) : ModuleName(ModuleName.str()), ModuleFilePath(ModuleFilePath.str()) {} - ModuleFile() = delete; - ModuleFile(const ModuleFile &) = delete; ModuleFile operator=(const ModuleFile &) = delete; @@ -187,23 +182,21 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, // third party modules, we should return true instead of false here. // Currently we simply bail out. if (ModuleUnitFileName.empty()) - return llvm::createStringError(llvm::formatv( - "Failed to build '{0}': Failed to get the primary source", ModuleName)); + return llvm::createStringError( + llvm::formatv("Failed to get the primary source")); // Try cheap operation earlier to boil-out cheaply if there are problems. auto Cmd = CDB.getCompileCommand(ModuleUnitFileName); if (!Cmd) return llvm::createStringError( - llvm::formatv("Failed to build '{0}': No compile command for {1}", - ModuleName, ModuleUnitFileName)); + llvm::formatv("No compile command for {0}", ModuleUnitFileName)); for (auto &RequiredModuleName : MDB.getRequiredModules(ModuleUnitFileName)) { // Return early if there are errors building the module file. if (llvm::Error Err = buildModuleFile(RequiredModuleName, CDB, TFS, MDB, ModuleFilesPrefix, BuiltModuleFiles)) return llvm::createStringError( - llvm::formatv("Failed to build dependency {0}: {1}", - RequiredModuleName, toString(std::move(Err)))); + llvm::formatv("Failed to build dependency {0}", RequiredModuleName)); } Cmd->Output = getModuleFilePath(ModuleName, ModuleFilesPrefix); @@ -215,16 +208,17 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, IgnoreDiagnostics IgnoreDiags; auto CI = buildCompilerInvocation(Inputs, IgnoreDiags); if (!CI) - return llvm::createStringError(llvm::formatv( - "Failed to build '{0}': Failed to build compiler invocation for {1}", - ModuleName, ModuleUnitFileName)); + return llvm::createStringError( + llvm::formatv("Failed to build compiler invocation")); auto FS = Inputs.TFS->view(Inputs.CompileCommand.Directory); auto Buf = FS->getBufferForFile(Inputs.CompileCommand.Filename); if (!Buf) - return llvm::createStringError( - llvm::formatv("Failed to build '{0}': Failed to create buffer for {1}", - ModuleName, Inputs.CompileCommand.Filename)); + return llvm::createStringError(llvm::formatv("Failed to create buffer")); + + // In clang's driver, we will suppress the check for ODR violation in GMF. + // See the implementation of RenderModulesOptions in Clang.cpp. + CI->getLangOpts().SkipODRCheckInGMF = true; // Hash the contents of input files and store the hash value to the BMI files. // So that we can check if the files are still valid when we want to reuse the @@ -238,17 +232,14 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, prepareCompilerInstance(std::move(CI), /*Preamble=*/nullptr, std::move(*Buf), std::move(FS), IgnoreDiags); if (!Clang) - return llvm::createStringError(llvm::formatv( - "Failed to build '{0}': Failed to prepare compiler instance for {0}", - ModuleName, ModuleUnitFileName)); + return llvm::createStringError( + llvm::formatv("Failed to prepare compiler instance")); - GenerateModuleInterfaceAction Action; + GenerateReducedModuleInterfaceAction Action; Clang->ExecuteAction(Action); if (Clang->getDiagnostics().hasErrorOccurred()) - return llvm::createStringError( - llvm::formatv("Failed to build '{0}': Compilation for {1} failed", - ModuleName, ModuleUnitFileName)); + return llvm::createStringError(llvm::formatv("Compilation failed")); BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); return llvm::Error::success(); @@ -318,6 +309,10 @@ bool StandalonePrerequisiteModules::canReuse( Clang.getHeaderSearchOpts().ForceCheckCXX20ModulesInputFiles = true; Clang.getHeaderSearchOpts().ValidateASTInputFilesContent = true; + // Following the practice of clang's driver to suppres the checking for ODR + // violation in GMF. + Clang.getLangOpts().SkipODRCheckInGMF = true; + Clang.createASTReader(); for (auto &RequiredModule : RequiredModules) { llvm::StringRef BMIPath = RequiredModule.ModuleFilePath; diff --git a/clang-tools-extra/clangd/ModulesBuilder.h b/clang-tools-extra/clangd/ModulesBuilder.h index cece418bcf525c..0514e7486475d0 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.h +++ b/clang-tools-extra/clangd/ModulesBuilder.h @@ -22,14 +22,10 @@ #include "GlobalCompilationDatabase.h" #include "ProjectModules.h" - #include "support/Path.h" #include "support/ThreadsafeFS.h" - #include "clang/Frontend/CompilerInvocation.h" - #include "llvm/ADT/SmallString.h" - #include namespace clang { diff --git a/clang-tools-extra/clangd/ScanningProjectModules.cpp b/clang-tools-extra/clangd/ScanningProjectModules.cpp index 976582970842b6..8ffe5ddb786c49 100644 --- a/clang-tools-extra/clangd/ScanningProjectModules.cpp +++ b/clang-tools-extra/clangd/ScanningProjectModules.cpp @@ -7,9 +7,8 @@ //===----------------------------------------------------------------------===// #include "ProjectModules.h" - #include "support/Logger.h" - +#include "clang/Driver/Driver.h" #include "clang/Tooling/DependencyScanning/DependencyScanningService.h" #include "clang/Tooling/DependencyScanning/DependencyScanningTool.h" @@ -99,6 +98,12 @@ ModuleDependencyScanner::scan(PathRef FilePath) { // DirectoryBasedGlobalCompilationDatabase::getCompileCommand. tooling::CompileCommand Cmd = std::move(Candidates.front()); + Cmd.CommandLine.push_back("-I"); + llvm::SmallString<256> ResourceDir(clang::driver::Driver::GetResourcesPath( + llvm::sys::fs::getMainExecutable(nullptr, nullptr))); + llvm::sys::path::append(ResourceDir, "include"); + Cmd.CommandLine.push_back((std::string)ResourceDir); + using namespace clang::tooling::dependencies; llvm::SmallString<128> FilePathDir(FilePath); diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp index 25005ec1bd0453..bc2eaa77a66eec 100644 --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -146,10 +146,13 @@ class Checker { ClangdLSPServer::Options Opts; // from buildCommand tooling::CompileCommand Cmd; + std::unique_ptr BaseCDB; + std::unique_ptr CDB; // from buildInvocation ParseInputs Inputs; std::unique_ptr Invocation; format::FormatStyle Style; + std::optional ModulesManager; // from buildAST std::shared_ptr Preamble; std::optional AST; @@ -168,14 +171,14 @@ class Checker { DirectoryBasedGlobalCompilationDatabase::Options CDBOpts(TFS); CDBOpts.CompileCommandsDir = Config::current().CompileFlags.CDBSearch.FixedCDBPath; - std::unique_ptr BaseCDB = + BaseCDB = std::make_unique(CDBOpts); auto Mangler = CommandMangler::detect(); Mangler.SystemIncludeExtractor = getSystemIncludeExtractor(llvm::ArrayRef(Opts.QueryDriverGlobs)); if (Opts.ResourceDir) Mangler.ResourceDir = *Opts.ResourceDir; - auto CDB = std::make_unique( + CDB = std::make_unique( BaseCDB.get(), std::vector{}, std::move(Mangler)); if (auto TrueCmd = CDB->getCompileCommand(File)) { @@ -213,6 +216,11 @@ class Checker { return false; } } + if (Opts.EnableExperimentalModulesSupport) { + if (!ModulesManager) + ModulesManager.emplace(*CDB); + Inputs.ModulesManager = &*ModulesManager; + } log("Parsing command..."); Invocation = buildCompilerInvocation(Inputs, CaptureInvocationDiags, &CC1Args); diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp index 8cbedcf0631a0b..dc8856fcd4f07d 100644 --- a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp +++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp @@ -111,14 +111,6 @@ class PrerequisiteModulesTests : public ::testing::Test { return Result.str().str(); } - std::unique_ptr getGlobalCompilationDatabase() { - // The compilation flags with modules are much complex so it looks better - // to use DirectoryBasedGlobalCompilationDatabase than a mocked compilation - // database. - DirectoryBasedGlobalCompilationDatabase::Options Opts(TFS); - return std::make_unique(Opts); - } - ParseInputs getInputs(llvm::StringRef FileName, const GlobalCompilationDatabase &CDB) { std::string FullPathName = getFullPath(FileName); @@ -128,24 +120,82 @@ class PrerequisiteModulesTests : public ::testing::Test { CDB.getCompileCommand(FullPathName); EXPECT_TRUE(Cmd); Inputs.CompileCommand = std::move(*Cmd); - Inputs.TFS = &TFS; + Inputs.TFS = &FS; - if (auto Contents = TFS.view(TestDir)->getBufferForFile(FullPathName)) + if (auto Contents = FS.view(TestDir)->getBufferForFile(FullPathName)) Inputs.Contents = Contents->get()->getBuffer().str(); return Inputs; } SmallString<256> TestDir; - // Noticed MockFS but its member variable 'OverlayRealFileSystemForModules' - // implies that it will better to use RealThreadsafeFS directly. - RealThreadsafeFS TFS; + // FIXME: It will be better to use the MockFS if the scanning process and + // build module process doesn't depend on reading real IO. + RealThreadsafeFS FS; DiagnosticConsumer DiagConsumer; }; -TEST_F(PrerequisiteModulesTests, PrerequisiteModulesTest) { - MockDirectoryCompilationDatabase CDB(TestDir, TFS); +TEST_F(PrerequisiteModulesTests, NonModularTest) { + MockDirectoryCompilationDatabase CDB(TestDir, FS); + + CDB.addFile("foo.h", R"cpp( +inline void foo() {} + )cpp"); + + CDB.addFile("NonModular.cpp", R"cpp( +#include "foo.h" +void use() { + foo(); +} + )cpp"); + + ModulesBuilder Builder(CDB); + + // NonModular.cpp is not related to modules. So nothing should be built. + auto NonModularInfo = + Builder.buildPrerequisiteModulesFor(getFullPath("NonModular.cpp"), FS); + EXPECT_TRUE(NonModularInfo); + + HeaderSearchOptions HSOpts; + NonModularInfo->adjustHeaderSearchOptions(HSOpts); + EXPECT_TRUE(HSOpts.PrebuiltModuleFiles.empty()); + + auto Invocation = + buildCompilerInvocation(getInputs("NonModular.cpp", CDB), DiagConsumer); + EXPECT_TRUE(NonModularInfo->canReuse(*Invocation, FS.view(TestDir))); +} + +TEST_F(PrerequisiteModulesTests, ModuleWithoutDepTest) { + MockDirectoryCompilationDatabase CDB(TestDir, FS); + + CDB.addFile("foo.h", R"cpp( +inline void foo() {} + )cpp"); + + CDB.addFile("M.cppm", R"cpp( +module; +#include "foo.h" +export module M; + )cpp"); + + ModulesBuilder Builder(CDB); + + auto MInfo = Builder.buildPrerequisiteModulesFor(getFullPath("M.cppm"), FS); + EXPECT_TRUE(MInfo); + + // Nothing should be built since M doesn't dependent on anything. + HeaderSearchOptions HSOpts; + MInfo->adjustHeaderSearchOptions(HSOpts); + EXPECT_TRUE(HSOpts.PrebuiltModuleFiles.empty()); + + auto Invocation = + buildCompilerInvocation(getInputs("M.cppm", CDB), DiagConsumer); + EXPECT_TRUE(MInfo->canReuse(*Invocation, FS.view(TestDir))); +} + +TEST_F(PrerequisiteModulesTests, ModuleWithDepTest) { + MockDirectoryCompilationDatabase CDB(TestDir, FS); CDB.addFile("foo.h", R"cpp( inline void foo() {} @@ -164,76 +214,97 @@ import M; )cpp"); CDB.addFile("N-part.cppm", R"cpp( -// Different name with filename intentionally. +// Different module name with filename intentionally. export module N:Part; )cpp"); - CDB.addFile("bar.h", R"cpp( -inline void bar() {} - )cpp"); - - CDB.addFile("L.cppm", R"cpp( -module; -#include "bar.h" -export module L; - )cpp"); + ModulesBuilder Builder(CDB); - CDB.addFile("NonModular.cpp", R"cpp( -#include "bar.h" -#include "foo.h" -void use() { - foo(); - bar(); -} - )cpp"); + auto NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), FS); + EXPECT_TRUE(NInfo); - ModulesBuilder Builder(CDB); + ParseInputs NInput = getInputs("N.cppm", CDB); + std::unique_ptr Invocation = + buildCompilerInvocation(NInput, DiagConsumer); + // Test that `PrerequisiteModules::canReuse` works basically. + EXPECT_TRUE(NInfo->canReuse(*Invocation, FS.view(TestDir))); - // NonModular.cpp is not related to modules. So nothing should be built. { - auto NonModularInfo = - Builder.buildPrerequisiteModulesFor(getFullPath("NonModular.cpp"), TFS); - EXPECT_TRUE(NonModularInfo); - auto Invocation = - buildCompilerInvocation(getInputs("NonModular.cpp", CDB), DiagConsumer); - EXPECT_TRUE(NonModularInfo->canReuse(*Invocation, TFS.view(TestDir))); + // Check that + // `PrerequisiteModules::adjustHeaderSearchOptions(HeaderSearchOptions&)` + // can appending HeaderSearchOptions correctly. + HeaderSearchOptions HSOpts; + NInfo->adjustHeaderSearchOptions(HSOpts); + + EXPECT_TRUE(HSOpts.PrebuiltModuleFiles.count("M")); + EXPECT_TRUE(HSOpts.PrebuiltModuleFiles.count("N:Part")); } { - auto MInfo = - Builder.buildPrerequisiteModulesFor(getFullPath("M.cppm"), TFS); - // buildPrerequisiteModulesFor won't built the module itself. - EXPECT_TRUE(MInfo); - auto Invocation = - buildCompilerInvocation(getInputs("M.cppm", CDB), DiagConsumer); - EXPECT_TRUE(MInfo->canReuse(*Invocation, TFS.view(TestDir))); + // Check that + // `PrerequisiteModules::adjustHeaderSearchOptions(HeaderSearchOptions&)` + // can replace HeaderSearchOptions correctly. + HeaderSearchOptions HSOpts; + HSOpts.PrebuiltModuleFiles["M"] = "incorrect_path"; + HSOpts.PrebuiltModuleFiles["N:Part"] = "incorrect_path"; + NInfo->adjustHeaderSearchOptions(HSOpts); + + EXPECT_TRUE(StringRef(HSOpts.PrebuiltModuleFiles["M"]).ends_with(".pcm")); + EXPECT_TRUE( + StringRef(HSOpts.PrebuiltModuleFiles["N:Part"]).ends_with(".pcm")); } +} + +TEST_F(PrerequisiteModulesTests, ReusabilityTest) { + MockDirectoryCompilationDatabase CDB(TestDir, FS); + + CDB.addFile("foo.h", R"cpp( +inline void foo() {} + )cpp"); + + CDB.addFile("M.cppm", R"cpp( +module; +#include "foo.h" +export module M; + )cpp"); - // Module N shouldn't be able to be built. - auto NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), TFS); + CDB.addFile("N.cppm", R"cpp( +export module N; +import :Part; +import M; + )cpp"); + + CDB.addFile("N-part.cppm", R"cpp( +// Different module name with filename intentionally. +export module N:Part; + )cpp"); + + ModulesBuilder Builder(CDB); + + auto NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), FS); + EXPECT_TRUE(NInfo); EXPECT_TRUE(NInfo); ParseInputs NInput = getInputs("N.cppm", CDB); std::unique_ptr Invocation = buildCompilerInvocation(NInput, DiagConsumer); - // Test that `PrerequisiteModules::canReuse` works basically. - EXPECT_TRUE(NInfo->canReuse(*Invocation, TFS.view(TestDir))); + EXPECT_TRUE(NInfo->canReuse(*Invocation, FS.view(TestDir))); // Test that we can still reuse the NInfo after we touch a unrelated file. { CDB.addFile("L.cppm", R"cpp( module; -#include "bar.h" +#include "foo.h" export module L; export int ll = 43; )cpp"); - EXPECT_TRUE(NInfo->canReuse(*Invocation, TFS.view(TestDir))); + EXPECT_TRUE(NInfo->canReuse(*Invocation, FS.view(TestDir))); CDB.addFile("bar.h", R"cpp( inline void bar() {} inline void bar(int) {} )cpp"); - EXPECT_TRUE(NInfo->canReuse(*Invocation, TFS.view(TestDir))); + EXPECT_TRUE(NInfo->canReuse(*Invocation, FS.view(TestDir))); } // Test that we can't reuse the NInfo after we touch a related file. @@ -244,19 +315,19 @@ module; export module M; export int mm = 44; )cpp"); - EXPECT_FALSE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); + EXPECT_FALSE(NInfo->canReuse(*Invocation, FS.view(TestDir))); - NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), TFS); - EXPECT_TRUE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); + NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), FS); + EXPECT_TRUE(NInfo->canReuse(*Invocation, FS.view(TestDir))); CDB.addFile("foo.h", R"cpp( inline void foo() {} inline void foo(int) {} )cpp"); - EXPECT_FALSE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); + EXPECT_FALSE(NInfo->canReuse(*Invocation, FS.view(TestDir))); - NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), TFS); - EXPECT_TRUE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); + NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), FS); + EXPECT_TRUE(NInfo->canReuse(*Invocation, FS.view(TestDir))); } CDB.addFile("N-part.cppm", R"cpp( @@ -264,21 +335,20 @@ export module N:Part; // Intentioned to make it uncompilable. export int NPart = 4LIdjwldijaw )cpp"); - EXPECT_FALSE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); - NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), TFS); + EXPECT_FALSE(NInfo->canReuse(*Invocation, FS.view(TestDir))); + NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), FS); EXPECT_TRUE(NInfo); - // So NInfo should be unreusable even after rebuild. - EXPECT_FALSE(NInfo->canReuse(*Invocation, TFS.view(TestDir))); + EXPECT_FALSE(NInfo->canReuse(*Invocation, FS.view(TestDir))); CDB.addFile("N-part.cppm", R"cpp( export module N:Part; export int NPart = 43; )cpp"); EXPECT_TRUE(NInfo); - EXPECT_FALSE(NInfo->canReuse(*Invocation, TFS.view(TestDir))); - NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), TFS); - // So NInfo should be unreusable even after rebuild. - EXPECT_TRUE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); + EXPECT_FALSE(NInfo->canReuse(*Invocation, FS.view(TestDir))); + NInfo = Builder.buildPrerequisiteModulesFor(getFullPath("N.cppm"), FS); + EXPECT_TRUE(NInfo); + EXPECT_TRUE(NInfo->canReuse(*Invocation, FS.view(TestDir))); // Test that if we changed the modification time of the file, the module files // info is still reusable if its content doesn't change. @@ -286,7 +356,7 @@ export int NPart = 43; export module N:Part; export int NPart = 43; )cpp"); - EXPECT_TRUE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); + EXPECT_TRUE(NInfo->canReuse(*Invocation, FS.view(TestDir))); CDB.addFile("N.cppm", R"cpp( export module N; @@ -296,26 +366,12 @@ import M; export int nn = 43; )cpp"); // NInfo should be reusable after we change its content. - EXPECT_TRUE(NInfo && NInfo->canReuse(*Invocation, TFS.view(TestDir))); - - { - // Check that - // `PrerequisiteModules::adjustHeaderSearchOptions(HeaderSearchOptions&)` - // can replace HeaderSearchOptions correctly. - ParseInputs NInput = getInputs("N.cppm", CDB); - std::unique_ptr NInvocation = - buildCompilerInvocation(NInput, DiagConsumer); - HeaderSearchOptions &HSOpts = NInvocation->getHeaderSearchOpts(); - NInfo->adjustHeaderSearchOptions(HSOpts); - - EXPECT_TRUE(HSOpts.PrebuiltModuleFiles.count("M")); - EXPECT_TRUE(HSOpts.PrebuiltModuleFiles.count("N:Part")); - } + EXPECT_TRUE(NInfo->canReuse(*Invocation, FS.view(TestDir))); } // An End-to-End test for modules. TEST_F(PrerequisiteModulesTests, ParsedASTTest) { - MockDirectoryCompilationDatabase CDB(TestDir, TFS); + MockDirectoryCompilationDatabase CDB(TestDir, FS); CDB.addFile("A.cppm", R"cpp( export module A; From 358f1bd063e90a175057784cf01056f5fdd08d10 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Wed, 17 Jul 2024 10:46:19 +0800 Subject: [PATCH 6/6] Address comments --- clang-tools-extra/clangd/ModulesBuilder.cpp | 19 ++++++++++--------- .../clangd/ScanningProjectModules.cpp | 10 ++++------ .../unittests/PrerequisiteModulesTest.cpp | 3 --- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 3fcc6e61bb3d77..94c7eec2d09e4e 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -182,8 +182,7 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, // third party modules, we should return true instead of false here. // Currently we simply bail out. if (ModuleUnitFileName.empty()) - return llvm::createStringError( - llvm::formatv("Failed to get the primary source")); + return llvm::createStringError("Failed to get the primary source"); // Try cheap operation earlier to boil-out cheaply if there are problems. auto Cmd = CDB.getCompileCommand(ModuleUnitFileName); @@ -196,7 +195,8 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, if (llvm::Error Err = buildModuleFile(RequiredModuleName, CDB, TFS, MDB, ModuleFilesPrefix, BuiltModuleFiles)) return llvm::createStringError( - llvm::formatv("Failed to build dependency {0}", RequiredModuleName)); + llvm::formatv("Failed to build dependency {0}: {1}", + RequiredModuleName, llvm::toString(std::move(Err)))); } Cmd->Output = getModuleFilePath(ModuleName, ModuleFilesPrefix); @@ -208,13 +208,12 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, IgnoreDiagnostics IgnoreDiags; auto CI = buildCompilerInvocation(Inputs, IgnoreDiags); if (!CI) - return llvm::createStringError( - llvm::formatv("Failed to build compiler invocation")); + return llvm::createStringError("Failed to build compiler invocation"); auto FS = Inputs.TFS->view(Inputs.CompileCommand.Directory); auto Buf = FS->getBufferForFile(Inputs.CompileCommand.Filename); if (!Buf) - return llvm::createStringError(llvm::formatv("Failed to create buffer")); + return llvm::createStringError("Failed to create buffer"); // In clang's driver, we will suppress the check for ODR violation in GMF. // See the implementation of RenderModulesOptions in Clang.cpp. @@ -232,14 +231,13 @@ llvm::Error buildModuleFile(llvm::StringRef ModuleName, prepareCompilerInstance(std::move(CI), /*Preamble=*/nullptr, std::move(*Buf), std::move(FS), IgnoreDiags); if (!Clang) - return llvm::createStringError( - llvm::formatv("Failed to prepare compiler instance")); + return llvm::createStringError("Failed to prepare compiler instance"); GenerateReducedModuleInterfaceAction Action; Clang->ExecuteAction(Action); if (Clang->getDiagnostics().hasErrorOccurred()) - return llvm::createStringError(llvm::formatv("Compilation failed")); + return llvm::createStringError("Compilation failed"); BuiltModuleFiles.addModuleFile(ModuleName, Inputs.CompileCommand.Output); return llvm::Error::success(); @@ -311,6 +309,9 @@ bool StandalonePrerequisiteModules::canReuse( // Following the practice of clang's driver to suppres the checking for ODR // violation in GMF. + // See + // https://clang.llvm.org/docs/StandardCPlusPlusModules.html#object-definition-consistency + // for example. Clang.getLangOpts().SkipODRCheckInGMF = true; Clang.createASTReader(); diff --git a/clang-tools-extra/clangd/ScanningProjectModules.cpp b/clang-tools-extra/clangd/ScanningProjectModules.cpp index 8ffe5ddb786c49..92f75ef7d5c25a 100644 --- a/clang-tools-extra/clangd/ScanningProjectModules.cpp +++ b/clang-tools-extra/clangd/ScanningProjectModules.cpp @@ -8,7 +8,6 @@ #include "ProjectModules.h" #include "support/Logger.h" -#include "clang/Driver/Driver.h" #include "clang/Tooling/DependencyScanning/DependencyScanningService.h" #include "clang/Tooling/DependencyScanning/DependencyScanningTool.h" @@ -98,11 +97,10 @@ ModuleDependencyScanner::scan(PathRef FilePath) { // DirectoryBasedGlobalCompilationDatabase::getCompileCommand. tooling::CompileCommand Cmd = std::move(Candidates.front()); - Cmd.CommandLine.push_back("-I"); - llvm::SmallString<256> ResourceDir(clang::driver::Driver::GetResourcesPath( - llvm::sys::fs::getMainExecutable(nullptr, nullptr))); - llvm::sys::path::append(ResourceDir, "include"); - Cmd.CommandLine.push_back((std::string)ResourceDir); + static int StaticForMainAddr; // Just an address in this process. + Cmd.CommandLine.push_back("-resource-dir=" + + CompilerInvocation::GetResourcesPath( + "clangd", (void *)&StaticForMainAddr)); using namespace clang::tooling::dependencies; diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp index dc8856fcd4f07d..7bbb95c8b8d67f 100644 --- a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp +++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp @@ -13,16 +13,13 @@ #include "ModulesBuilder.h" #include "ScanningProjectModules.h" - #include "Annotations.h" #include "CodeComplete.h" #include "Compiler.h" #include "TestTU.h" #include "support/ThreadsafeFS.h" - #include "llvm/Support/FileSystem.h" #include "llvm/Support/raw_ostream.h" - #include "gmock/gmock.h" #include "gtest/gtest.h"