llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clangd Author: None (fleeting-xx) <details> <summary>Changes</summary> This is a re-application of llvm/llvm-project#<!-- -->142090 without the unit test changes. A subsequent PR will follow that adds a unit test for module dependencies. ### Changes - Fix dangling string references in the return value of getAllRequiredModules() - Change a couple of calls in getOrBuildModuleFile() to use the loop variable instead of the ModuleName parameter. @<!-- -->ChuanqiXu9 for review --- Full diff: https://github.com/llvm/llvm-project/pull/142828.diff 1 Files Affected: - (modified) clang-tools-extra/clangd/ModulesBuilder.cpp (+12-10) ``````````diff diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index bf77f43bd28bb..d88aa01aad05d 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -430,10 +430,10 @@ class CachingProjectModules : public ProjectModules { /// Collect the directly and indirectly required module names for \param /// ModuleName in topological order. The \param ModuleName is guaranteed to /// be the last element in \param ModuleNames. -llvm::SmallVector<StringRef> getAllRequiredModules(PathRef RequiredSource, - CachingProjectModules &MDB, - StringRef ModuleName) { - llvm::SmallVector<llvm::StringRef> ModuleNames; +llvm::SmallVector<std::string> getAllRequiredModules(PathRef RequiredSource, + CachingProjectModules &MDB, + StringRef ModuleName) { + llvm::SmallVector<std::string> ModuleNames; llvm::StringSet<> ModuleNamesSet; auto VisitDeps = [&](StringRef ModuleName, auto Visitor) -> void { @@ -444,7 +444,7 @@ llvm::SmallVector<StringRef> getAllRequiredModules(PathRef RequiredSource, if (ModuleNamesSet.insert(RequiredModuleName).second) Visitor(RequiredModuleName, Visitor); - ModuleNames.push_back(ModuleName); + ModuleNames.push_back(ModuleName.str()); }; VisitDeps(ModuleName, VisitDeps); @@ -494,13 +494,13 @@ llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile( // Get Required modules in topological order. auto ReqModuleNames = getAllRequiredModules(RequiredSource, MDB, ModuleName); for (llvm::StringRef ReqModuleName : ReqModuleNames) { - if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName)) + if (BuiltModuleFiles.isModuleUnitBuilt(ReqModuleName)) continue; if (auto Cached = Cache.getModule(ReqModuleName)) { if (IsModuleFileUpToDate(Cached->getModuleFilePath(), BuiltModuleFiles, TFS.view(std::nullopt))) { - log("Reusing module {0} from {1}", ModuleName, + log("Reusing module {0} from {1}", ReqModuleName, Cached->getModuleFilePath()); BuiltModuleFiles.addModuleFile(std::move(Cached)); continue; @@ -508,14 +508,16 @@ llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile( Cache.remove(ReqModuleName); } + std::string ReqFileName = + MDB.getSourceForModuleName(ReqModuleName, RequiredSource); llvm::Expected<ModuleFile> MF = buildModuleFile( - ModuleName, ModuleUnitFileName, getCDB(), TFS, BuiltModuleFiles); + ReqModuleName, ReqFileName, getCDB(), TFS, BuiltModuleFiles); if (llvm::Error Err = MF.takeError()) return Err; - log("Built module {0} to {1}", ModuleName, MF->getModuleFilePath()); + log("Built module {0} to {1}", ReqModuleName, MF->getModuleFilePath()); auto BuiltModuleFile = std::make_shared<const ModuleFile>(std::move(*MF)); - Cache.add(ModuleName, BuiltModuleFile); + Cache.add(ReqModuleName, BuiltModuleFile); BuiltModuleFiles.addModuleFile(std::move(BuiltModuleFile)); } `````````` </details> https://github.com/llvm/llvm-project/pull/142828 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
