Skip to content

Commit 4e0f1e3

Browse files
committed
Address comments
1 parent 370b01f commit 4e0f1e3

File tree

1 file changed

+109
-157
lines changed

1 file changed

+109
-157
lines changed

clang-tools-extra/clangd/ModulesBuilder.cpp

Lines changed: 109 additions & 157 deletions
Original file line numberDiff line numberDiff line change
@@ -95,47 +95,9 @@ class FailedPrerequisiteModules : public PrerequisiteModules {
9595
}
9696
};
9797

98-
struct ModuleFile;
99-
100-
// ReusablePrerequisiteModules - stands for PrerequisiteModules for which all
101-
// the required modules are built successfully. All the module files
102-
// are owned by the modules builder.
103-
class ReusablePrerequisiteModules : public PrerequisiteModules {
104-
public:
105-
ReusablePrerequisiteModules() = default;
106-
107-
ReusablePrerequisiteModules(const ReusablePrerequisiteModules &Other) =
108-
default;
109-
ReusablePrerequisiteModules &
110-
operator=(const ReusablePrerequisiteModules &) = default;
111-
ReusablePrerequisiteModules(ReusablePrerequisiteModules &&) = delete;
112-
ReusablePrerequisiteModules
113-
operator=(ReusablePrerequisiteModules &&) = delete;
114-
115-
~ReusablePrerequisiteModules() override = default;
116-
117-
void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override;
118-
119-
bool canReuse(const CompilerInvocation &CI,
120-
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>) const override;
121-
122-
bool isModuleUnitBuilt(llvm::StringRef ModuleName) const {
123-
return BuiltModuleNames.contains(ModuleName);
124-
}
125-
126-
void addModuleFile(std::shared_ptr<const ModuleFile> ModuleFile);
127-
128-
private:
129-
llvm::SmallVector<std::shared_ptr<const ModuleFile>, 8> RequiredModules;
130-
// A helper class to speedup the query if a module is built.
131-
llvm::StringSet<> BuiltModuleNames;
132-
};
133-
13498
struct ModuleFile {
135-
ModuleFile(StringRef ModuleName, PathRef ModuleFilePath,
136-
const ReusablePrerequisiteModules &RequiredModuleFiles)
137-
: ModuleName(ModuleName.str()), ModuleFilePath(ModuleFilePath.str()),
138-
RequiredModuleFiles(RequiredModuleFiles) {}
99+
ModuleFile(StringRef ModuleName, PathRef ModuleFilePath)
100+
: ModuleName(ModuleName.str()), ModuleFilePath(ModuleFilePath.str()) {}
139101

140102
ModuleFile() = delete;
141103

@@ -171,26 +133,50 @@ struct ModuleFile {
171133
private:
172134
std::string ModuleName;
173135
std::string ModuleFilePath;
174-
175-
// The required module files. We need to share the ownership for required
176-
// module files.
177-
ReusablePrerequisiteModules RequiredModuleFiles;
178136
};
179137

180-
void ReusablePrerequisiteModules::adjustHeaderSearchOptions(
181-
HeaderSearchOptions &Options) const {
182-
// Appending all built module files.
183-
for (const auto &RequiredModule : RequiredModules)
184-
Options.PrebuiltModuleFiles.insert_or_assign(
185-
RequiredModule->getModuleName().str(),
186-
RequiredModule->getModuleFilePath().str());
187-
}
138+
// ReusablePrerequisiteModules - stands for PrerequisiteModules for which all
139+
// the required modules are built successfully. All the module files
140+
// are owned by the modules builder.
141+
class ReusablePrerequisiteModules : public PrerequisiteModules {
142+
public:
143+
ReusablePrerequisiteModules() = default;
188144

189-
void ReusablePrerequisiteModules::addModuleFile(
190-
std::shared_ptr<const ModuleFile> ModuleFile) {
191-
BuiltModuleNames.insert(ModuleFile->getModuleName());
192-
RequiredModules.emplace_back(std::move(ModuleFile));
193-
}
145+
ReusablePrerequisiteModules(const ReusablePrerequisiteModules &Other) =
146+
default;
147+
ReusablePrerequisiteModules &
148+
operator=(const ReusablePrerequisiteModules &) = default;
149+
ReusablePrerequisiteModules(ReusablePrerequisiteModules &&) = delete;
150+
ReusablePrerequisiteModules
151+
operator=(ReusablePrerequisiteModules &&) = delete;
152+
153+
~ReusablePrerequisiteModules() override = default;
154+
155+
void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {
156+
// Appending all built module files.
157+
for (const auto &RequiredModule : RequiredModules)
158+
Options.PrebuiltModuleFiles.insert_or_assign(
159+
RequiredModule->getModuleName().str(),
160+
RequiredModule->getModuleFilePath().str());
161+
}
162+
163+
bool canReuse(const CompilerInvocation &CI,
164+
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>) const override;
165+
166+
bool isModuleUnitBuilt(llvm::StringRef ModuleName) const {
167+
return BuiltModuleNames.contains(ModuleName);
168+
}
169+
170+
void addModuleFile(std::shared_ptr<const ModuleFile> ModuleFile) {
171+
BuiltModuleNames.insert(ModuleFile->getModuleName());
172+
RequiredModules.emplace_back(std::move(ModuleFile));
173+
}
174+
175+
private:
176+
llvm::SmallVector<std::shared_ptr<const ModuleFile>, 8> RequiredModules;
177+
// A helper class to speedup the query if a module is built.
178+
llvm::StringSet<> BuiltModuleNames;
179+
};
194180

195181
bool IsModuleFileUpToDate(PathRef ModuleFilePath,
196182
const PrerequisiteModules &RequisiteModules,
@@ -309,7 +295,7 @@ buildModuleFile(llvm::StringRef ModuleName, PathRef ModuleUnitFileName,
309295
if (Clang->getDiagnostics().hasErrorOccurred())
310296
return llvm::createStringError("Compilation failed");
311297

312-
return ModuleFile{ModuleName, Inputs.CompileCommand.Output, BuiltModuleFiles};
298+
return ModuleFile{ModuleName, Inputs.CompileCommand.Output};
313299
}
314300

315301
bool ReusablePrerequisiteModules::canReuse(
@@ -327,24 +313,18 @@ bool ReusablePrerequisiteModules::canReuse(
327313
class ModuleFileCache {
328314
public:
329315
ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {}
330-
331-
llvm::Error
332-
getOrBuildModuleFile(StringRef ModuleName, const ThreadsafeFS &TFS,
333-
ProjectModules &MDB,
334-
ReusablePrerequisiteModules &RequiredModules);
335316
const GlobalCompilationDatabase &getCDB() const { return CDB; }
336317

337-
std::shared_ptr<const ModuleFile>
338-
getValidModuleFile(StringRef ModuleName, ProjectModules &MDB,
339-
const ThreadsafeFS &TFS,
340-
PrerequisiteModules &BuiltModuleFiles);
318+
std::shared_ptr<const ModuleFile> getModule(StringRef ModuleName);
341319

342320
void add(StringRef ModuleName, std::shared_ptr<const ModuleFile> ModuleFile) {
343321
std::lock_guard<std::mutex> Lock(ModuleFilesMutex);
344322

345323
ModuleFiles.insert_or_assign(ModuleName, ModuleFile);
346324
}
347325

326+
void remove(StringRef ModuleName);
327+
348328
private:
349329
const GlobalCompilationDatabase &CDB;
350330

@@ -353,86 +333,55 @@ class ModuleFileCache {
353333
std::mutex ModuleFilesMutex;
354334
};
355335

356-
/// Collect the directly and indirectly required module names for \param
357-
/// ModuleName. The \param ModuleName is guaranteed to be the first element in
358-
/// \param ModuleNames.
359-
llvm::SmallVector<StringRef> getAllRequiredModules(ProjectModules &MDB,
360-
StringRef ModuleName) {
361-
llvm::SmallVector<StringRef> ModuleNames;
362-
363-
std::queue<StringRef> Worklist;
364-
llvm::StringSet<> ModuleNamesSet;
365-
Worklist.push(ModuleName);
366-
367-
while (!Worklist.empty()) {
368-
StringRef CurrentModule = Worklist.front();
369-
Worklist.pop();
370-
371-
if (!ModuleNamesSet.insert(CurrentModule).second)
372-
continue;
373-
374-
ModuleNames.push_back(CurrentModule);
375-
376-
for (StringRef RequiredModuleName :
377-
MDB.getRequiredModules(MDB.getSourceForModuleName(CurrentModule)))
378-
if (!ModuleNamesSet.contains(RequiredModuleName))
379-
Worklist.push(RequiredModuleName);
380-
}
381-
382-
return ModuleNames;
383-
}
384-
385336
std::shared_ptr<const ModuleFile>
386-
ModuleFileCache::getValidModuleFile(StringRef ModuleName, ProjectModules &MDB,
387-
const ThreadsafeFS &TFS,
388-
PrerequisiteModules &BuiltModuleFiles) {
389-
{
390-
std::lock_guard<std::mutex> Lock(ModuleFilesMutex);
337+
ModuleFileCache::getModule(StringRef ModuleName) {
338+
std::lock_guard<std::mutex> Lock(ModuleFilesMutex);
391339

392-
auto Iter = ModuleFiles.find(ModuleName);
393-
if (Iter == ModuleFiles.end())
394-
return nullptr;
340+
auto Iter = ModuleFiles.find(ModuleName);
341+
if (Iter == ModuleFiles.end())
342+
return nullptr;
395343

396-
if (Iter->second.expired()) {
397-
ModuleFiles.erase(Iter);
398-
return nullptr;
399-
}
344+
if (Iter->second.expired()) {
345+
ModuleFiles.erase(Iter);
346+
return nullptr;
400347
}
401348

402-
llvm::SmallVector<StringRef> ModuleNames =
403-
getAllRequiredModules(MDB, ModuleName);
349+
return Iter->second.lock();
350+
}
404351

405-
llvm::SmallVector<std::shared_ptr<const ModuleFile>> RequiredModuleFiles;
352+
void ModuleFileCache::remove(StringRef ModuleName) {
353+
std::lock_guard<std::mutex> Lock(ModuleFilesMutex);
406354

407-
{
408-
std::lock_guard<std::mutex> Lock(ModuleFilesMutex);
355+
auto Iter = ModuleFiles.find(ModuleName);
356+
if (Iter == ModuleFiles.end())
357+
return;
409358

410-
for (StringRef ModuleName : ModuleNames) {
411-
auto Iter = ModuleFiles.find(ModuleName);
412-
if (Iter == ModuleFiles.end())
413-
return nullptr;
359+
ModuleFiles.erase(Iter);
360+
}
414361

415-
if (Iter->second.expired()) {
416-
ModuleFiles.erase(Iter);
417-
return nullptr;
418-
}
362+
/// Collect the directly and indirectly required module names for \param
363+
/// ModuleName in topological order. The \param ModuleName is guaranteed to
364+
/// be the last element in \param ModuleNames.
365+
llvm::SmallVector<StringRef> getAllRequiredModules(ProjectModules &MDB,
366+
StringRef ModuleName) {
367+
llvm::SmallVector<StringRef> ModuleNames;
368+
llvm::StringSet<> ModuleNamesSet;
419369

420-
RequiredModuleFiles.push_back(Iter->second.lock());
421-
}
422-
}
370+
auto traversal = [&](StringRef ModuleName, auto recursionHelper) -> void {
371+
ModuleNamesSet.insert(ModuleName);
423372

424-
assert(!RequiredModuleFiles.empty());
373+
for (StringRef RequiredModuleName :
374+
MDB.getRequiredModules(MDB.getSourceForModuleName(ModuleName)))
375+
if (ModuleNamesSet.insert(RequiredModuleName).second)
376+
recursionHelper(RequiredModuleName, recursionHelper);
425377

426-
if (llvm::any_of(RequiredModuleFiles,
427-
[&](std::shared_ptr<const ModuleFile> MF) {
428-
return !IsModuleFileUpToDate(MF->getModuleFilePath(),
429-
BuiltModuleFiles,
430-
TFS.view(std::nullopt));
431-
}))
432-
return nullptr;
378+
ModuleNames.push_back(ModuleName);
379+
};
380+
traversal(ModuleName, traversal);
433381

434-
return RequiredModuleFiles[0];
382+
return ModuleNames;
435383
}
384+
436385
} // namespace
437386

438387
class ModulesBuilder::ModulesBuilderImpl {
@@ -468,34 +417,37 @@ llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
468417
return llvm::createStringError(
469418
llvm::formatv("Don't get the module unit for module {0}", ModuleName));
470419

471-
for (auto &RequiredModuleName : MDB.getRequiredModules(ModuleUnitFileName))
472-
// Return early if there are errors building the module file.
473-
if (!getOrBuildModuleFile(RequiredModuleName, TFS, MDB, BuiltModuleFiles))
474-
return llvm::createStringError(
475-
llvm::formatv("Failed to build module {0}", RequiredModuleName));
476-
477-
if (std::shared_ptr<const ModuleFile> Cached =
478-
Cache.getValidModuleFile(ModuleName, MDB, TFS, BuiltModuleFiles)) {
479-
log("Reusing module {0} from {1}", ModuleName, Cached->getModuleFilePath());
480-
BuiltModuleFiles.addModuleFile(Cached);
481-
return llvm::Error::success();
482-
}
420+
// Get Required modules in topological order.
421+
auto ReqModuleNames = getAllRequiredModules(MDB, ModuleName);
422+
for (llvm::StringRef ReqModuleName : ReqModuleNames) {
423+
if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName))
424+
continue;
483425

484-
log("Building module {0}", ModuleName);
426+
if (auto Cached = Cache.getModule(ReqModuleName)) {
427+
if (IsModuleFileUpToDate(Cached->getModuleFilePath(), BuiltModuleFiles,
428+
TFS.view(std::nullopt))) {
429+
log("Reusing module {0} from {1}", ModuleName,
430+
Cached->getModuleFilePath());
431+
BuiltModuleFiles.addModuleFile(std::move(Cached));
432+
continue;
433+
}
434+
Cache.remove(ReqModuleName);
435+
}
485436

486-
llvm::SmallString<256> ModuleFilesPrefix =
487-
getUniqueModuleFilesPath(ModuleUnitFileName);
437+
llvm::SmallString<256> ModuleFilesPrefix =
438+
getUniqueModuleFilesPath(ModuleUnitFileName);
488439

489-
llvm::Expected<ModuleFile> MF =
490-
buildModuleFile(ModuleName, ModuleUnitFileName, getCDB(), TFS,
491-
ModuleFilesPrefix, BuiltModuleFiles);
492-
if (llvm::Error Err = MF.takeError())
493-
return Err;
440+
llvm::Expected<ModuleFile> MF =
441+
buildModuleFile(ModuleName, ModuleUnitFileName, getCDB(), TFS,
442+
ModuleFilesPrefix, BuiltModuleFiles);
443+
if (llvm::Error Err = MF.takeError())
444+
return Err;
494445

495-
log("Built module {0} to {1}", ModuleName, MF->getModuleFilePath());
496-
auto BuiltModuleFile = std::make_shared<const ModuleFile>(std::move(*MF));
497-
Cache.add(ModuleName, BuiltModuleFile);
498-
BuiltModuleFiles.addModuleFile(std::move(BuiltModuleFile));
446+
log("Built module {0} to {1}", ModuleName, MF->getModuleFilePath());
447+
auto BuiltModuleFile = std::make_shared<const ModuleFile>(std::move(*MF));
448+
Cache.add(ModuleName, BuiltModuleFile);
449+
BuiltModuleFiles.addModuleFile(std::move(BuiltModuleFile));
450+
}
499451

500452
return llvm::Error::success();
501453
}

0 commit comments

Comments
 (0)