Skip to content

Commit 63c7b6e

Browse files
committed
[clangd] [Modules] Fix to correctly handle module dependencies
- 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.
1 parent fd452da commit 63c7b6e

File tree

1 file changed

+12
-10
lines changed

1 file changed

+12
-10
lines changed

clang-tools-extra/clangd/ModulesBuilder.cpp

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -430,10 +430,10 @@ class CachingProjectModules : public ProjectModules {
430430
/// Collect the directly and indirectly required module names for \param
431431
/// ModuleName in topological order. The \param ModuleName is guaranteed to
432432
/// be the last element in \param ModuleNames.
433-
llvm::SmallVector<StringRef> getAllRequiredModules(PathRef RequiredSource,
434-
CachingProjectModules &MDB,
435-
StringRef ModuleName) {
436-
llvm::SmallVector<llvm::StringRef> ModuleNames;
433+
llvm::SmallVector<std::string> getAllRequiredModules(PathRef RequiredSource,
434+
CachingProjectModules &MDB,
435+
StringRef ModuleName) {
436+
llvm::SmallVector<std::string> ModuleNames;
437437
llvm::StringSet<> ModuleNamesSet;
438438

439439
auto VisitDeps = [&](StringRef ModuleName, auto Visitor) -> void {
@@ -444,7 +444,7 @@ llvm::SmallVector<StringRef> getAllRequiredModules(PathRef RequiredSource,
444444
if (ModuleNamesSet.insert(RequiredModuleName).second)
445445
Visitor(RequiredModuleName, Visitor);
446446

447-
ModuleNames.push_back(ModuleName);
447+
ModuleNames.push_back(ModuleName.str());
448448
};
449449
VisitDeps(ModuleName, VisitDeps);
450450

@@ -494,28 +494,30 @@ llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
494494
// Get Required modules in topological order.
495495
auto ReqModuleNames = getAllRequiredModules(RequiredSource, MDB, ModuleName);
496496
for (llvm::StringRef ReqModuleName : ReqModuleNames) {
497-
if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName))
497+
if (BuiltModuleFiles.isModuleUnitBuilt(ReqModuleName))
498498
continue;
499499

500500
if (auto Cached = Cache.getModule(ReqModuleName)) {
501501
if (IsModuleFileUpToDate(Cached->getModuleFilePath(), BuiltModuleFiles,
502502
TFS.view(std::nullopt))) {
503-
log("Reusing module {0} from {1}", ModuleName,
503+
log("Reusing module {0} from {1}", ReqModuleName,
504504
Cached->getModuleFilePath());
505505
BuiltModuleFiles.addModuleFile(std::move(Cached));
506506
continue;
507507
}
508508
Cache.remove(ReqModuleName);
509509
}
510510

511+
std::string ReqFileName =
512+
MDB.getSourceForModuleName(ReqModuleName, RequiredSource);
511513
llvm::Expected<ModuleFile> MF = buildModuleFile(
512-
ModuleName, ModuleUnitFileName, getCDB(), TFS, BuiltModuleFiles);
514+
ReqModuleName, ReqFileName, getCDB(), TFS, BuiltModuleFiles);
513515
if (llvm::Error Err = MF.takeError())
514516
return Err;
515517

516-
log("Built module {0} to {1}", ModuleName, MF->getModuleFilePath());
518+
log("Built module {0} to {1}", ReqModuleName, MF->getModuleFilePath());
517519
auto BuiltModuleFile = std::make_shared<const ModuleFile>(std::move(*MF));
518-
Cache.add(ModuleName, BuiltModuleFile);
520+
Cache.add(ReqModuleName, BuiltModuleFile);
519521
BuiltModuleFiles.addModuleFile(std::move(BuiltModuleFile));
520522
}
521523

0 commit comments

Comments
 (0)