Skip to content

Commit dcdd18a

Browse files
committed
Update
1 parent 4ed7bf5 commit dcdd18a

File tree

3 files changed

+135
-56
lines changed

3 files changed

+135
-56
lines changed

clang-tools-extra/clangd/ModulesBuilder.cpp

Lines changed: 97 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "clang/Serialization/ASTReader.h"
1515
#include "clang/Serialization/InMemoryModuleCache.h"
1616
#include "llvm/ADT/ScopeExit.h"
17+
#include <queue>
1718

1819
namespace clang {
1920
namespace clangd {
@@ -125,6 +126,11 @@ struct ModuleFile {
125126
llvm::sys::fs::remove(ModuleFilePath);
126127
}
127128

129+
StringRef getModuleName() const { return ModuleName; }
130+
131+
StringRef getModuleFilePath() const { return ModuleFilePath; }
132+
133+
private:
128134
std::string ModuleName;
129135
std::string ModuleFilePath;
130136
};
@@ -213,7 +219,8 @@ class ReusablePrerequisiteModules : public PrerequisiteModules {
213219
// Appending all built module files.
214220
for (auto &RequiredModule : RequiredModules)
215221
Options.PrebuiltModuleFiles.insert_or_assign(
216-
RequiredModule->ModuleName, RequiredModule->ModuleFilePath);
222+
RequiredModule->getModuleName().str(),
223+
RequiredModule->getModuleFilePath().str());
217224
}
218225

219226
bool canReuse(const CompilerInvocation &CI,
@@ -224,12 +231,12 @@ class ReusablePrerequisiteModules : public PrerequisiteModules {
224231
}
225232

226233
void addModuleFile(std::shared_ptr<ModuleFile> BMI) {
227-
BuiltModuleNames.insert(BMI->ModuleName);
234+
BuiltModuleNames.insert(BMI->getModuleName());
228235
RequiredModules.emplace_back(std::move(BMI));
229236
}
230237

231238
private:
232-
mutable llvm::SmallVector<std::shared_ptr<ModuleFile>, 8> RequiredModules;
239+
llvm::SmallVector<std::shared_ptr<ModuleFile>, 8> RequiredModules;
233240
// A helper class to speedup the query if a module is built.
234241
llvm::StringSet<> BuiltModuleNames;
235242
};
@@ -298,12 +305,11 @@ bool ReusablePrerequisiteModules::canReuse(
298305

299306
SmallVector<StringRef> BMIPaths;
300307
for (auto &MF : RequiredModules)
301-
BMIPaths.push_back(MF->ModuleFilePath);
308+
BMIPaths.push_back(MF->getModuleFilePath());
302309
return IsModuleFilesUpToDate(BMIPaths, *this, VFS);
303310
}
304-
} // namespace
305311

306-
class ModulesBuilder::ModuleFileCache {
312+
class ModuleFileCache {
307313
public:
308314
ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {}
309315

@@ -313,68 +319,107 @@ class ModulesBuilder::ModuleFileCache {
313319
ReusablePrerequisiteModules &RequiredModules);
314320
const GlobalCompilationDatabase &getCDB() const { return CDB; }
315321

316-
private:
317322
std::shared_ptr<ModuleFile>
318323
getValidModuleFile(StringRef ModuleName, ProjectModules &MDB,
319324
const ThreadsafeFS &TFS,
320325
PrerequisiteModules &BuiltModuleFiles);
321326

322-
/// This should only be called by getValidModuleFile. This is unlocked version
323-
/// of getValidModuleFile. The function is extracted to avoid dead locks when
324-
/// recursing.
325-
std::shared_ptr<ModuleFile>
326-
isValidModuleFileUnlocked(StringRef ModuleName, ProjectModules &MDB,
327-
const ThreadsafeFS &TFS,
328-
PrerequisiteModules &BuiltModuleFiles);
327+
void add(StringRef ModuleName, std::shared_ptr<ModuleFile> ModuleFile) {
328+
std::lock_guard<std::mutex> _(ModuleFilesMutex);
329+
330+
ModuleFiles.insert_or_assign(ModuleName, ModuleFile);
331+
}
329332

333+
private:
330334
const GlobalCompilationDatabase &CDB;
331335

332336
llvm::StringMap<std::shared_ptr<ModuleFile>> ModuleFiles;
333337
// Mutex to guard accesses to ModuleFiles.
334338
std::mutex ModuleFilesMutex;
335339
};
336340

341+
/// Collect the directly and indirectly required module names for \param
342+
/// ModuleName. The \param ModuleName is guaranteed to be the first element in
343+
/// \param ModuleNames.
344+
void getAllRequiredModules(ProjectModules &MDB, StringRef ModuleName,
345+
llvm::SmallVector<StringRef> &ModuleNames) {
346+
std::queue<StringRef> Worklist;
347+
llvm::StringSet<> ModuleNamesSet;
348+
Worklist.push(ModuleName);
349+
350+
while (!Worklist.empty()) {
351+
StringRef CurrentModule = Worklist.front();
352+
Worklist.pop();
353+
354+
if (!ModuleNamesSet.insert(CurrentModule).second)
355+
continue;
356+
357+
ModuleNames.push_back(CurrentModule);
358+
359+
for (StringRef RequiredModuleName :
360+
MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule)))
361+
if (!ModuleNamesSet.contains(RequiredModuleName))
362+
Worklist.push(RequiredModuleName);
363+
}
364+
}
365+
337366
std::shared_ptr<ModuleFile>
338-
ModulesBuilder::ModuleFileCache::isValidModuleFileUnlocked(
339-
StringRef ModuleName, ProjectModules &MDB, const ThreadsafeFS &TFS,
340-
PrerequisiteModules &BuiltModuleFiles) {
341-
auto Iter = ModuleFiles.find(ModuleName);
342-
if (Iter != ModuleFiles.end()) {
343-
if (!IsModuleFileUpToDate(Iter->second->ModuleFilePath, BuiltModuleFiles,
344-
TFS.view(std::nullopt))) {
345-
log("Found not-up-date module file {0} for module {1} in cache",
346-
Iter->second->ModuleFilePath, ModuleName);
347-
ModuleFiles.erase(Iter);
348-
return nullptr;
349-
}
367+
ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB,
368+
const ThreadsafeFS &TFS,
369+
PrerequisiteModules &BuiltModuleFiles) {
370+
{
371+
std::lock_guard<std::mutex> _(ModuleFilesMutex);
350372

351-
if (llvm::any_of(
352-
MDB.getRequiredModules(MDB.getSourceForModuleName(ModuleName)),
353-
[&MDB, &TFS, &BuiltModuleFiles, this](auto &&RequiredModuleName) {
354-
return !isValidModuleFileUnlocked(RequiredModuleName, MDB, TFS,
355-
BuiltModuleFiles);
356-
})) {
357-
ModuleFiles.erase(Iter);
373+
if (ModuleFiles.find(ModuleName) == ModuleFiles.end())
358374
return nullptr;
359-
}
375+
}
376+
377+
llvm::SmallVector<StringRef> ModuleNames;
378+
getAllRequiredModules(MDB, ModuleName, ModuleNames);
379+
380+
llvm::SmallVector<std::shared_ptr<ModuleFile>> RequiredModuleFiles;
381+
382+
{
383+
std::lock_guard<std::mutex> _(ModuleFilesMutex);
384+
385+
for (StringRef ModuleName : ModuleNames) {
386+
auto Iter = ModuleFiles.find(ModuleName);
387+
if (Iter == ModuleFiles.end())
388+
return nullptr;
360389

361-
return Iter->second;
390+
RequiredModuleFiles.push_back(Iter->second);
391+
}
362392
}
363393

364-
log("Don't find {0} in cache", ModuleName);
394+
if (RequiredModuleFiles.empty())
395+
return nullptr;
396+
397+
if (llvm::all_of(RequiredModuleFiles, [&](auto MF) {
398+
return IsModuleFileUpToDate(MF->getModuleFilePath(), BuiltModuleFiles,
399+
TFS.view(std::nullopt));
400+
}))
401+
return RequiredModuleFiles[0];
365402

366403
return nullptr;
367404
}
405+
} // namespace
368406

369-
std::shared_ptr<ModuleFile> ModulesBuilder::ModuleFileCache::getValidModuleFile(
370-
StringRef ModuleName, ProjectModules &MDB, const ThreadsafeFS &TFS,
371-
PrerequisiteModules &BuiltModuleFiles) {
372-
std::lock_guard<std::mutex> _(ModuleFilesMutex);
407+
class ModulesBuilder::ModulesBuilderImpl {
408+
public:
409+
ModulesBuilderImpl(const GlobalCompilationDatabase &CDB) : Cache(CDB) {}
373410

374-
return isValidModuleFileUnlocked(ModuleName, MDB, TFS, BuiltModuleFiles);
375-
}
411+
const GlobalCompilationDatabase &getCDB() const { return Cache.getCDB(); }
376412

377-
llvm::Error ModulesBuilder::ModuleFileCache::getOrBuildModuleFile(
413+
llvm::Error
414+
getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS,
415+
ProjectModules &MDB,
416+
ReusablePrerequisiteModules &BuiltModuleFiles);
417+
418+
private:
419+
ModuleFileCache Cache;
420+
};
421+
422+
llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
378423
StringRef ModuleName, const ThreadsafeFS &TFS, ProjectModules &MDB,
379424
ReusablePrerequisiteModules &BuiltModuleFiles) {
380425
if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName))
@@ -399,8 +444,8 @@ llvm::Error ModulesBuilder::ModuleFileCache::getOrBuildModuleFile(
399444
llvm::formatv("Failed to build module {0}", RequiredModuleName));
400445

401446
if (std::shared_ptr<ModuleFile> Cached =
402-
getValidModuleFile(ModuleName, MDB, TFS, BuiltModuleFiles)) {
403-
log("Reusing module {0} from {1}", ModuleName, Cached->ModuleFilePath);
447+
Cache.getValidModuleFile(ModuleName, MDB, TFS, BuiltModuleFiles)) {
448+
log("Reusing module {0} from {1}", ModuleName, Cached->getModuleFilePath());
404449
BuiltModuleFiles.addModuleFile(Cached);
405450
return llvm::Error::success();
406451
}
@@ -411,25 +456,23 @@ llvm::Error ModulesBuilder::ModuleFileCache::getOrBuildModuleFile(
411456
getUniqueModuleFilesPath(ModuleUnitFileName);
412457

413458
llvm::Expected<ModuleFile> MF =
414-
buildModuleFile(ModuleName, ModuleUnitFileName, CDB, TFS,
459+
buildModuleFile(ModuleName, ModuleUnitFileName, getCDB(), TFS,
415460
ModuleFilesPrefix, BuiltModuleFiles);
416461
if (llvm::Error Err = MF.takeError())
417462
return Err;
418463

419-
log("Built module {0} to {1}", ModuleName, MF->ModuleFilePath);
420-
421-
std::lock_guard<std::mutex> __(ModuleFilesMutex);
464+
log("Built module {0} to {1}", ModuleName, MF->getModuleFilePath());
422465
auto BuiltModuleFile = std::make_shared<ModuleFile>(std::move(*MF));
423-
ModuleFiles.insert_or_assign(ModuleName, BuiltModuleFile);
466+
Cache.add(ModuleName, BuiltModuleFile);
424467
BuiltModuleFiles.addModuleFile(std::move(BuiltModuleFile));
425468

426469
return llvm::Error::success();
427470
}
471+
428472
std::unique_ptr<PrerequisiteModules>
429473
ModulesBuilder::buildPrerequisiteModulesFor(PathRef File,
430474
const ThreadsafeFS &TFS) {
431-
std::unique_ptr<ProjectModules> MDB =
432-
MFCache->getCDB().getProjectModules(File);
475+
std::unique_ptr<ProjectModules> MDB = Impl->getCDB().getProjectModules(File);
433476
if (!MDB) {
434477
elog("Failed to get Project Modules information for {0}", File);
435478
return std::make_unique<FailedPrerequisiteModules>();
@@ -448,7 +491,7 @@ ModulesBuilder::buildPrerequisiteModulesFor(PathRef File,
448491

449492
for (llvm::StringRef RequiredModuleName : RequiredModuleNames) {
450493
// Return early if there is any error.
451-
if (llvm::Error Err = MFCache->getOrBuildModuleFile(
494+
if (llvm::Error Err = Impl->getOrBuildModuleFile(
452495
RequiredModuleName, TFS, *MDB.get(), *RequiredModules.get())) {
453496
elog("Failed to build module {0}; due to {1}", RequiredModuleName,
454497
toString(std::move(Err)));
@@ -462,7 +505,7 @@ ModulesBuilder::buildPrerequisiteModulesFor(PathRef File,
462505
}
463506

464507
ModulesBuilder::ModulesBuilder(const GlobalCompilationDatabase &CDB) {
465-
MFCache = std::make_unique<ModuleFileCache>(CDB);
508+
Impl = std::make_unique<ModulesBuilderImpl>(CDB);
466509
}
467510

468511
ModulesBuilder::~ModulesBuilder() {}

clang-tools-extra/clangd/ModulesBuilder.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ class ModulesBuilder {
9898
buildPrerequisiteModulesFor(PathRef File, const ThreadsafeFS &TFS);
9999

100100
private:
101-
class ModuleFileCache;
102-
std::unique_ptr<ModuleFileCache> MFCache;
101+
class ModulesBuilderImpl;
102+
std::unique_ptr<ModulesBuilderImpl> Impl;
103103
};
104104

105105
} // namespace clangd

clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,42 @@ export int B = 44 + M;
516516

517517
// Check that we're reusing the module files.
518518
EXPECT_EQ(HSOptsA.PrebuiltModuleFiles, HSOptsB.PrebuiltModuleFiles);
519+
520+
// Update M.cppm to check if the modules builder can update correctly.
521+
CDB.addFile("M.cppm", R"cpp(
522+
export module M;
523+
export constexpr int M = 43;
524+
)cpp");
525+
526+
ParseInputs AUse = getInputs("A.cppm", CDB);
527+
AUse.ModulesManager = &Builder;
528+
std::unique_ptr<CompilerInvocation> AInvocation =
529+
buildCompilerInvocation(AUse, DiagConsumer);
530+
EXPECT_FALSE(AInfo->canReuse(*AInvocation, FS.view(TestDir)));
531+
532+
ParseInputs BUse = getInputs("B.cppm", CDB);
533+
AUse.ModulesManager = &Builder;
534+
std::unique_ptr<CompilerInvocation> BInvocation =
535+
buildCompilerInvocation(BUse, DiagConsumer);
536+
EXPECT_FALSE(BInfo->canReuse(*BInvocation, FS.view(TestDir)));
537+
538+
auto NewAInfo =
539+
Builder.buildPrerequisiteModulesFor(getFullPath("A.cppm"), FS);
540+
auto NewBInfo =
541+
Builder.buildPrerequisiteModulesFor(getFullPath("B.cppm"), FS);
542+
EXPECT_TRUE(NewAInfo);
543+
EXPECT_TRUE(NewBInfo);
544+
HeaderSearchOptions NewHSOptsA(TestDir);
545+
HeaderSearchOptions NewHSOptsB(TestDir);
546+
NewAInfo->adjustHeaderSearchOptions(NewHSOptsA);
547+
NewBInfo->adjustHeaderSearchOptions(NewHSOptsB);
548+
549+
EXPECT_FALSE(NewHSOptsA.PrebuiltModuleFiles.empty());
550+
EXPECT_FALSE(NewHSOptsB.PrebuiltModuleFiles.empty());
551+
552+
EXPECT_EQ(NewHSOptsA.PrebuiltModuleFiles, NewHSOptsB.PrebuiltModuleFiles);
553+
// Check that we didn't reuse the old and stale module files.
554+
EXPECT_NE(NewHSOptsA.PrebuiltModuleFiles, HSOptsA.PrebuiltModuleFiles);
519555
}
520556

521557
} // namespace

0 commit comments

Comments
 (0)