https://github.com/berkaysahiin updated https://github.com/llvm/llvm-project/pull/218958
>From 99dc8b26f2fe207e5147501a98c11628f19103ed Mon Sep 17 00:00:00 2001 From: Berkay Sahin <[email protected]> Date: Wed, 26 Aug 2026 18:18:47 +0300 Subject: [PATCH 1/2] [clangd] Discover opened module files outside the compilation database --- clang-tools-extra/clangd/ClangdServer.cpp | 14 +- clang-tools-extra/clangd/ModulesBuilder.cpp | 133 +++++++++++++++--- clang-tools-extra/clangd/ModulesBuilder.h | 10 ++ .../unittests/PrerequisiteModulesTest.cpp | 24 ++++ 4 files changed, 162 insertions(+), 19 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 37eb82116f3a9..d4d313b40d76b 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -299,6 +299,8 @@ ClangdServer::~ClangdServer() { void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents, llvm::StringRef Version, WantDiagnostics WantDiags, bool ForceRebuild) { + bool NewModule = ModulesManager && ModulesManager->observeSourcePath(File); + std::string ActualVersion = DraftMgr.addDraft(File, Version, Contents); ParseOptions Opts; Opts.PreambleParseForwardingFunctions = PreambleParseForwardingFunctions; @@ -320,6 +322,9 @@ void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents, // If we loaded Foo.h, we want to make sure Foo.cpp is indexed. if (NewFile && BackgroundIdx) BackgroundIdx->boostRelated(File); + if (NewModule) + reparseOpenFilesIfNeeded( + [&](PathRef OpenFile) { return !pathEqual(OpenFile, File); }); } void ClangdServer::reparseOpenFilesIfNeeded( @@ -942,8 +947,13 @@ void ClangdServer::outgoingCalls( } void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) { - // FIXME: Do nothing for now. This will be used for indexing and potentially - // invalidating other caches. + if (!ModulesManager) + return; + bool ModulesChanged = false; + for (const auto &Change : Params.changes) + ModulesChanged |= ModulesManager->onFileEvent(Change); + if (ModulesChanged) + reparseOpenFilesIfNeeded([](PathRef) { return true; }); } void ClangdServer::workspaceSymbols( diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 6e47122bdd82b..87fbbbe8ef5c4 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -8,8 +8,10 @@ #include "ModulesBuilder.h" #include "Compiler.h" +#include "Protocol.h" #include "SourceCode.h" #include "support/Logger.h" +#include "clang/Driver/Types.h" #include "clang/Frontend/FrontendAction.h" #include "clang/Frontend/FrontendActions.h" #include "clang/Serialization/ASTReader.h" @@ -25,6 +27,7 @@ #include <chrono> #include <ctime> +#include <optional> namespace clang { namespace clangd { @@ -865,11 +868,63 @@ class ModuleNameToSourceCache { llvm::StringMap<llvm::StringMap<std::string>> ModuleNameToMultipleSourceCache; }; +bool isCXXModuleFile(PathRef File) { + namespace types = clang::driver::types; + auto Lang = + types::lookupTypeForExtension(llvm::sys::path::extension(File).substr(1)); + return Lang == types::TY_CXXModule; +} + +class ObservedModuleFiles { +public: + explicit ObservedModuleFiles(const GlobalCompilationDatabase &CDB) + : CDB(CDB) {} + + bool add(PathRef File) { + std::lock_guard<std::mutex> Lock(Mutex); + return Sources.try_emplace(maybeCaseFoldPath(File), File.str()).second; + } + + bool remove(PathRef File) { + std::lock_guard<std::mutex> Lock(Mutex); + return Sources.erase(maybeCaseFoldPath(File)); + } + + std::vector<Path> sourcesFor(PathRef File) const { + auto PI = CDB.getProjectInfo(File); + if (!PI || PI->SourceRoot.empty()) + return {}; + const std::string ProjectRoot = maybeCaseFoldPath(PI->SourceRoot); + + std::vector<Path> Observed; + { + std::lock_guard<std::mutex> Lock(Mutex); + Observed.reserve(Sources.size()); + for (const auto &Source : Sources) + Observed.push_back(Source.second); + } + + std::vector<Path> Result; + for (const auto &Source : Observed) { + auto SourcePI = CDB.getProjectInfo(Source); + if (SourcePI && maybeCaseFoldPath(SourcePI->SourceRoot) == ProjectRoot) + Result.push_back(Source); + } + return Result; + } + +private: + const GlobalCompilationDatabase &CDB; + mutable std::mutex Mutex; + llvm::StringMap<Path> Sources; +}; + class CachingProjectModules : public ProjectModules { public: CachingProjectModules(std::unique_ptr<ProjectModules> MDB, - ModuleNameToSourceCache &Cache) - : MDB(std::move(MDB)), Cache(Cache) { + ModuleNameToSourceCache &Cache, + const ObservedModuleFiles &ObservedFiles) + : MDB(std::move(MDB)), Cache(Cache), ObservedFiles(ObservedFiles) { assert(this->MDB && "CachingProjectModules should only be created with a " "valid underlying ProjectModules"); } @@ -906,10 +961,11 @@ class CachingProjectModules : public ProjectModules { Cache.eraseMultipleEntry(ModuleName, RequiredSrcFile); } - auto Result = MDB->getSourceForModuleName(ModuleName, RequiredSrcFile); - if (!Result.empty()) - Cache.addMultipleEntry(ModuleName, RequiredSrcFile, Result); - return Result; + auto Result = findSourceForModuleName(ModuleName, RequiredSrcFile); + if (!Result) + return {}; + Cache.addMultipleEntry(ModuleName, RequiredSrcFile, *Result); + return *Result; } // For unknown module name state, assume it is unique. This may give user @@ -930,16 +986,28 @@ class CachingProjectModules : public ProjectModules { Cache.eraseUniqueEntry(ModuleName); } + auto Result = findSourceForModuleName(ModuleName, RequiredSrcFile); + if (!Result) + return {}; + Cache.addUniqueEntry(ModuleName, *Result); + return *Result; + } + +private: + std::optional<std::string> findSourceForModuleName(llvm::StringRef ModuleName, + PathRef RequiredSrcFile) { auto Result = MDB->getSourceForModuleName(ModuleName, RequiredSrcFile); if (!Result.empty()) - Cache.addUniqueEntry(ModuleName, Result); - - return Result; + return Result; + for (const auto &Source : ObservedFiles.sourcesFor(RequiredSrcFile)) + if (MDB->getModuleNameForSource(Source) == ModuleName) + return Source; + return std::nullopt; } -private: std::unique_ptr<ProjectModules> MDB; ModuleNameToSourceCache &Cache; + const ObservedModuleFiles &ObservedFiles; }; /// Collect the directly and indirectly required module names for \param @@ -1021,13 +1089,35 @@ void garbageCollectModuleCache(PathRef CacheRoot) { class ModulesBuilder::ModulesBuilderImpl { public: - ModulesBuilderImpl(const GlobalCompilationDatabase &CDB) : Cache(CDB) {} + ModulesBuilderImpl(const GlobalCompilationDatabase &CDB) + : Cache(CDB), ObservedFiles(CDB) {} ModuleNameToSourceCache &getProjectModulesCache() { return ProjectModulesCache; } + const ObservedModuleFiles &getObservedFiles() const { return ObservedFiles; } const GlobalCompilationDatabase &getCDB() const { return Cache.getCDB(); } + bool observeSourcePath(PathRef File) { + return isCXXModuleFile(File) && ObservedFiles.add(File); + } + + bool onFileEvent(const FileEvent &Event) { + const llvm::StringRef File = Event.uri.file(); + switch (Event.type) { + case FileChangeType::Created: + return isCXXModuleFile(File) && ObservedFiles.add(File); + case FileChangeType::Changed: + if (!isCXXModuleFile(File)) + return false; + ObservedFiles.add(File); + return true; + case FileChangeType::Deleted: + return ObservedFiles.remove(File); + } + llvm_unreachable("Unhandled FileChangeType"); + } + llvm::Error getOrBuildModuleFile(PathRef RequiredSource, StringRef ModuleName, const ThreadsafeFS &TFS, CachingProjectModules &MDB, @@ -1043,6 +1133,7 @@ class ModulesBuilder::ModulesBuilderImpl { void garbageCollectModuleCacheForProjectRoot(PathRef ProjectRoot); ModuleFileCache Cache; + ObservedModuleFiles ObservedFiles; ModuleNameToSourceCache ProjectModulesCache; std::mutex GarbageCollectedProjectRootsMutex; llvm::StringSet<> GarbageCollectedProjectRoots; @@ -1236,8 +1327,8 @@ bool ModulesBuilder::hasRequiredModules(PathRef File) { if (!MDB) return false; - CachingProjectModules CachedMDB(std::move(MDB), - Impl->getProjectModulesCache()); + CachingProjectModules CachedMDB( + std::move(MDB), Impl->getProjectModulesCache(), Impl->getObservedFiles()); return !CachedMDB.getRequiredModules(File).empty(); } @@ -1246,8 +1337,8 @@ std::vector<std::string> ModulesBuilder::getRequiredModuleNames(PathRef File) { if (!MDB) return {}; - CachingProjectModules CachedMDB(std::move(MDB), - Impl->getProjectModulesCache()); + CachingProjectModules CachedMDB( + std::move(MDB), Impl->getProjectModulesCache(), Impl->getObservedFiles()); return CachedMDB.getRequiredModules(File); } @@ -1259,8 +1350,8 @@ ModulesBuilder::buildPrerequisiteModulesFor(PathRef File, elog("Failed to get Project Modules information for {0}", File); return std::make_unique<FailedPrerequisiteModules>(); } - CachingProjectModules CachedMDB(std::move(MDB), - Impl->getProjectModulesCache()); + CachingProjectModules CachedMDB( + std::move(MDB), Impl->getProjectModulesCache(), Impl->getObservedFiles()); std::vector<std::string> RequiredModuleNames = CachedMDB.getRequiredModules(File); @@ -1288,5 +1379,13 @@ ModulesBuilder::ModulesBuilder(const GlobalCompilationDatabase &CDB) { ModulesBuilder::~ModulesBuilder() {} +bool ModulesBuilder::observeSourcePath(PathRef File) { + return Impl->observeSourcePath(File); +} + +bool ModulesBuilder::onFileEvent(const FileEvent &Event) { + return Impl->onFileEvent(Event); +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/ModulesBuilder.h b/clang-tools-extra/clangd/ModulesBuilder.h index d8c8c446fd592..58275854706be 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.h +++ b/clang-tools-extra/clangd/ModulesBuilder.h @@ -32,6 +32,8 @@ namespace clang { namespace clangd { +struct FileEvent; + /// Store all the needed module files information to parse a single /// source file. e.g., /// @@ -98,6 +100,14 @@ class ModulesBuilder { ModulesBuilder &operator=(const ModulesBuilder &) = delete; ModulesBuilder &operator=(ModulesBuilder &&) = delete; + /// Makes an opened module interface available to project module queries. + /// Returns whether a new module path was observed. + bool observeSourcePath(PathRef File); + + /// Updates project module queries after a watched file event. Returns whether + /// the event may have changed an observed module. + bool onFileEvent(const FileEvent &Event); + std::unique_ptr<PrerequisiteModules> buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS &TFS); diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp index f10f3e4976cf9..88d3af3ddf09e 100644 --- a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp +++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp @@ -341,6 +341,30 @@ export module M; EXPECT_TRUE(MInfo->canReuse(*Invocation, FS.view(TestDir))); } +TEST_F(PrerequisiteModulesTests, ObservedModuleOutsideCompilationDatabase) { + MockDirectoryCompilationDatabase CDB(TestDir, FS); + CDB.addFile("Use.cpp", R"cpp( +import M; + )cpp"); + + SmallString<256> ModulePath(TestDir); + llvm::sys::path::append(ModulePath, "M.cppm"); + std::error_code EC; + llvm::raw_fd_ostream OS(ModulePath, EC); + ASSERT_FALSE(EC); + OS << "export module M;"; + OS.close(); + + ModulesBuilder Builder(CDB); + EXPECT_TRUE(Builder.observeSourcePath(ModulePath)); + EXPECT_FALSE(Builder.observeSourcePath(ModulePath)); + auto Info = Builder.buildPrerequisiteModulesFor(getFullPath("Use.cpp"), FS); + + HeaderSearchOptions HSOpts; + Info->adjustHeaderSearchOptions(HSOpts); + EXPECT_EQ(HSOpts.PrebuiltModuleFiles.count("M"), 1u); +} + TEST_F(PrerequisiteModulesTests, ModuleWithArgumentPatch) { MockDirectoryCompilationDatabase CDB(TestDir, FS); >From 204a006163ee55249676240e3b3832b4297f22cb Mon Sep 17 00:00:00 2001 From: Berkay Sahin <[email protected]> Date: Sun, 30 Aug 2026 18:12:23 +0300 Subject: [PATCH 2/2] [clangd] add comments --- clang-tools-extra/clangd/ModulesBuilder.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 87fbbbe8ef5c4..d90afea465155 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -868,6 +868,11 @@ class ModuleNameToSourceCache { llvm::StringMap<llvm::StringMap<std::string>> ModuleNameToMultipleSourceCache; }; +// Heuristic: identifies candidate module interface files by extension only. +// Not authoritative. A module interface may use a non-module extension. A file +// with a module extension may not declare a module or may be a non-importable +// implementation unit. The later scan by the underlying ProjectModules confirms +// the actual module declaration. bool isCXXModuleFile(PathRef File) { namespace types = clang::driver::types; auto Lang = @@ -875,6 +880,9 @@ bool isCXXModuleFile(PathRef File) { return Lang == types::TY_CXXModule; } +// Module files discovered by opening or file-watching, before they are known to +// the compilation database. Used as a fallback when normal module resolution +// cannot find the source for a module name. class ObservedModuleFiles { public: explicit ObservedModuleFiles(const GlobalCompilationDatabase &CDB) _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
