Skip to content

Commit 7a24238

Browse files
committed
Reland "[clang][modules] Timestamp-less validation API (#139987)"
This reverts commit 18b885f, effectively reapplying #139987. This commit fixes unit tests (for example ASTUnitTest.SaveLoadPreservesLangOptionsInPrintingPolicy) where the `ASTUnit::ModCache` pointer dereferenced within `ASTUnit::serialize()` was null. This commit makes sure each factory function does initialize `ASTUnit::ModCache`.
1 parent 1b44eb2 commit 7a24238

File tree

14 files changed

+55
-60
lines changed

14 files changed

+55
-60
lines changed

clang-tools-extra/clangd/ModulesBuilder.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,8 @@ bool IsModuleFileUpToDate(PathRef ModuleFilePath,
207207
Preprocessor PP(PPOpts, *Diags, LangOpts, SourceMgr, HeaderInfo,
208208
ModuleLoader);
209209

210-
IntrusiveRefCntPtr<ModuleCache> ModCache = createCrossProcessModuleCache();
210+
IntrusiveRefCntPtr<ModuleCache> ModCache =
211+
createCrossProcessModuleCache(HSOpts.BuildSessionTimestamp);
211212
PCHContainerOperations PCHOperations;
212213
ASTReader Reader(PP, *ModCache, /*ASTContext=*/nullptr,
213214
PCHOperations.getRawReader(), {});

clang/include/clang/Serialization/ModuleCache.h

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,17 +33,14 @@ class ModuleCache : public RefCountedBase<ModuleCache> {
3333
virtual std::unique_ptr<llvm::AdvisoryLock>
3434
getLock(StringRef ModuleFilename) = 0;
3535

36-
// TODO: Abstract away timestamps with isUpToDate() and markUpToDate().
3736
// TODO: Consider exposing a "validation lock" API to prevent multiple clients
3837
// concurrently noticing an out-of-date module file and validating its inputs.
3938

40-
/// Returns the timestamp denoting the last time inputs of the module file
41-
/// were validated.
42-
virtual std::time_t getModuleTimestamp(StringRef ModuleFilename) = 0;
39+
/// Checks whether the inputs of the module file were marked as validated.
40+
virtual bool isMarkedUpToDate(StringRef ModuleFilename) = 0;
4341

44-
/// Updates the timestamp denoting the last time inputs of the module file
45-
/// were validated.
46-
virtual void updateModuleTimestamp(StringRef ModuleFilename) = 0;
42+
/// Marks the inputs of the module file as validated.
43+
virtual void markUpToDate(StringRef ModuleFilename) = 0;
4744

4845
/// Returns this process's view of the module cache.
4946
virtual InMemoryModuleCache &getInMemoryModuleCache() = 0;
@@ -58,7 +55,8 @@ class ModuleCache : public RefCountedBase<ModuleCache> {
5855
/// operated on by multiple processes. This instance must be used across all
5956
/// \c CompilerInstance instances participating in building modules for single
6057
/// translation unit in order to share the same \c InMemoryModuleCache.
61-
IntrusiveRefCntPtr<ModuleCache> createCrossProcessModuleCache();
58+
IntrusiveRefCntPtr<ModuleCache>
59+
createCrossProcessModuleCache(std::time_t BuildSessionTimestamp);
6260
} // namespace clang
6361

6462
#endif

clang/include/clang/Serialization/ModuleFile.h

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -270,11 +270,9 @@ class ModuleFile {
270270
// system input files reside at [NumUserInputFiles, InputFilesLoaded.size()).
271271
unsigned NumUserInputFiles = 0;
272272

273-
/// If non-zero, specifies the time when we last validated input
274-
/// files. Zero means we never validated them.
275-
///
276-
/// The time is specified in seconds since the start of the Epoch.
277-
uint64_t InputFilesValidationTimestamp = 0;
273+
/// Specifies whether the input files have been validated (i.e. checked
274+
/// whether they are up-to-date).
275+
bool InputFilesValidated = false;
278276

279277
// === Source Locations ===
280278

clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
#include "clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h"
1313
#include "clang/Tooling/DependencyScanning/InProcessModuleCache.h"
1414
#include "llvm/ADT/BitmaskEnum.h"
15-
#include "llvm/Support/Chrono.h"
1615

1716
namespace clang {
1817
namespace tooling {
@@ -85,9 +84,7 @@ class DependencyScanningService {
8584
DependencyScanningService(
8685
ScanningMode Mode, ScanningOutputFormat Format,
8786
ScanningOptimizations OptimizeArgs = ScanningOptimizations::Default,
88-
bool EagerLoadModules = false, bool TraceVFS = false,
89-
std::time_t BuildSessionTimestamp =
90-
llvm::sys::toTimeT(std::chrono::system_clock::now()));
87+
bool EagerLoadModules = false, bool TraceVFS = false);
9188

9289
ScanningMode getMode() const { return Mode; }
9390

@@ -105,8 +102,6 @@ class DependencyScanningService {
105102

106103
ModuleCacheEntries &getModuleCacheEntries() { return ModCacheEntries; }
107104

108-
std::time_t getBuildSessionTimestamp() const { return BuildSessionTimestamp; }
109-
110105
private:
111106
const ScanningMode Mode;
112107
const ScanningOutputFormat Format;
@@ -120,8 +115,6 @@ class DependencyScanningService {
120115
DependencyScanningFilesystemSharedCache SharedCache;
121116
/// The global module cache entries.
122117
ModuleCacheEntries ModCacheEntries;
123-
/// The build session timestamp.
124-
std::time_t BuildSessionTimestamp;
125118
};
126119

127120
} // end namespace dependencies

clang/include/clang/Tooling/DependencyScanning/InProcessModuleCache.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ namespace tooling {
2020
namespace dependencies {
2121
struct ModuleCacheEntry {
2222
std::shared_mutex CompilationMutex;
23-
std::atomic<std::time_t> Timestamp = 0;
23+
std::atomic<bool> UpToDate = false;
2424
};
2525

2626
struct ModuleCacheEntries {

clang/lib/Frontend/ASTUnit.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -829,9 +829,10 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile(
829829
AST->SourceMgr = new SourceManager(AST->getDiagnostics(),
830830
AST->getFileManager(),
831831
UserFilesAreVolatile);
832-
AST->ModCache = createCrossProcessModuleCache();
833832
AST->HSOpts = std::make_unique<HeaderSearchOptions>(HSOpts);
834833
AST->HSOpts->ModuleFormat = std::string(PCHContainerRdr.getFormats().front());
834+
AST->ModCache =
835+
createCrossProcessModuleCache(AST->HSOpts->BuildSessionTimestamp);
835836
AST->HeaderInfo.reset(new HeaderSearch(AST->getHeaderSearchOpts(),
836837
AST->getSourceManager(),
837838
AST->getDiagnostics(),
@@ -1548,7 +1549,8 @@ ASTUnit::create(std::shared_ptr<CompilerInvocation> CI,
15481549
AST->UserFilesAreVolatile = UserFilesAreVolatile;
15491550
AST->SourceMgr = new SourceManager(AST->getDiagnostics(), *AST->FileMgr,
15501551
UserFilesAreVolatile);
1551-
AST->ModCache = createCrossProcessModuleCache();
1552+
AST->ModCache = createCrossProcessModuleCache(
1553+
AST->Invocation->getHeaderSearchOpts().BuildSessionTimestamp);
15521554

15531555
return AST;
15541556
}
@@ -1745,6 +1747,8 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromCompilerInvocation(
17451747
AST->IncludeBriefCommentsInCodeCompletion
17461748
= IncludeBriefCommentsInCodeCompletion;
17471749
AST->Invocation = std::move(CI);
1750+
AST->ModCache = createCrossProcessModuleCache(
1751+
AST->Invocation->getHeaderSearchOpts().BuildSessionTimestamp);
17481752
AST->FileSystemOpts = FileMgr->getFileSystemOpts();
17491753
AST->FileMgr = FileMgr;
17501754
AST->UserFilesAreVolatile = UserFilesAreVolatile;
@@ -1834,7 +1838,6 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromCommandLine(
18341838
AST->FileMgr = new FileManager(AST->FileSystemOpts, VFS);
18351839
AST->StorePreamblesInMemory = StorePreamblesInMemory;
18361840
AST->PreambleStoragePath = PreambleStoragePath;
1837-
AST->ModCache = createCrossProcessModuleCache();
18381841
AST->OnlyLocalDecls = OnlyLocalDecls;
18391842
AST->CaptureDiagnostics = CaptureDiagnostics;
18401843
AST->TUKind = TUKind;
@@ -1843,6 +1846,8 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromCommandLine(
18431846
= IncludeBriefCommentsInCodeCompletion;
18441847
AST->UserFilesAreVolatile = UserFilesAreVolatile;
18451848
AST->Invocation = CI;
1849+
AST->ModCache = createCrossProcessModuleCache(
1850+
AST->Invocation->getHeaderSearchOpts().BuildSessionTimestamp);
18461851
AST->SkipFunctionBodies = SkipFunctionBodies;
18471852
if (ForSerialization)
18481853
AST->WriterData.reset(new ASTWriterData(*AST->ModCache));
@@ -2378,7 +2383,6 @@ bool ASTUnit::serialize(raw_ostream &OS) {
23782383

23792384
SmallString<128> Buffer;
23802385
llvm::BitstreamWriter Stream(Buffer);
2381-
IntrusiveRefCntPtr<ModuleCache> ModCache = createCrossProcessModuleCache();
23822386
ASTWriter Writer(Stream, Buffer, *ModCache, {});
23832387
return serializeUnit(Writer, Buffer, getSema(), OS);
23842388
}

clang/lib/Frontend/CompilerInstance.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@ CompilerInstance::CompilerInstance(
7272
ModuleCache *ModCache)
7373
: ModuleLoader(/*BuildingModule=*/ModCache),
7474
Invocation(std::move(Invocation)),
75-
ModCache(ModCache ? ModCache : createCrossProcessModuleCache()),
75+
ModCache(ModCache ? ModCache
76+
: createCrossProcessModuleCache(
77+
getHeaderSearchOpts().BuildSessionTimestamp)),
7678
ThePCHContainerOperations(std::move(PCHContainerOps)) {
7779
assert(this->Invocation && "Invocation must not be null");
7880
}

clang/lib/Serialization/ASTReader.cpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3103,8 +3103,7 @@ ASTReader::ReadControlBlock(ModuleFile &F,
31033103

31043104
unsigned N = ValidateSystemInputs ? NumInputs : NumUserInputs;
31053105
if (HSOpts.ModulesValidateOncePerBuildSession &&
3106-
F.InputFilesValidationTimestamp > HSOpts.BuildSessionTimestamp &&
3107-
F.Kind == MK_ImplicitModule)
3106+
F.InputFilesValidated && F.Kind == MK_ImplicitModule)
31083107
N = ForceValidateUserInputs ? NumUserInputs : 0;
31093108

31103109
for (unsigned I = 0; I < N; ++I) {
@@ -4950,10 +4949,8 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName, ModuleKind Type,
49504949
// timestamp files are up-to-date in this build session.
49514950
for (unsigned I = 0, N = Loaded.size(); I != N; ++I) {
49524951
ImportedModule &M = Loaded[I];
4953-
if (M.Mod->Kind == MK_ImplicitModule &&
4954-
M.Mod->InputFilesValidationTimestamp < HSOpts.BuildSessionTimestamp)
4955-
getModuleManager().getModuleCache().updateModuleTimestamp(
4956-
M.Mod->FileName);
4952+
if (M.Mod->Kind == MK_ImplicitModule && !M.Mod->InputFilesValidated)
4953+
getModuleManager().getModuleCache().markUpToDate(M.Mod->FileName);
49574954
}
49584955
}
49594956

clang/lib/Serialization/ASTWriter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5391,7 +5391,7 @@ ASTWriter::WriteAST(llvm::PointerUnion<Sema *, Preprocessor *> Subject,
53915391
if (WritingModule && PPRef.getHeaderSearchInfo()
53925392
.getHeaderSearchOpts()
53935393
.ModulesValidateOncePerBuildSession)
5394-
ModCache.updateModuleTimestamp(OutputFile);
5394+
ModCache.markUpToDate(OutputFile);
53955395

53965396
if (ShouldCacheASTInMemory) {
53975397
// Construct MemoryBuffer and update buffer manager.

clang/lib/Serialization/ModuleCache.cpp

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,12 @@ namespace {
2020
class CrossProcessModuleCache : public ModuleCache {
2121
InMemoryModuleCache InMemory;
2222

23+
std::time_t BuildSessionTimestamp;
24+
2325
public:
26+
explicit CrossProcessModuleCache(std::time_t BuildSessionTimestamp)
27+
: BuildSessionTimestamp(BuildSessionTimestamp) {}
28+
2429
void prepareForGetLock(StringRef ModuleFilename) override {
2530
// FIXME: Do this in LockFileManager and only if the directory doesn't
2631
// exist.
@@ -33,16 +38,17 @@ class CrossProcessModuleCache : public ModuleCache {
3338
return std::make_unique<llvm::LockFileManager>(ModuleFilename);
3439
}
3540

36-
std::time_t getModuleTimestamp(StringRef ModuleFilename) override {
41+
bool isMarkedUpToDate(StringRef ModuleFilename) override {
3742
std::string TimestampFilename =
3843
serialization::ModuleFile::getTimestampFilename(ModuleFilename);
3944
llvm::sys::fs::file_status Status;
4045
if (llvm::sys::fs::status(ModuleFilename, Status) != std::error_code{})
41-
return 0;
42-
return llvm::sys::toTimeT(Status.getLastModificationTime());
46+
return false;
47+
return llvm::sys::toTimeT(Status.getLastModificationTime()) >
48+
BuildSessionTimestamp;
4349
}
4450

45-
void updateModuleTimestamp(StringRef ModuleFilename) override {
51+
void markUpToDate(StringRef ModuleFilename) override {
4652
// Overwrite the timestamp file contents so that file's mtime changes.
4753
std::error_code EC;
4854
llvm::raw_fd_ostream OS(
@@ -62,6 +68,8 @@ class CrossProcessModuleCache : public ModuleCache {
6268
};
6369
} // namespace
6470

65-
IntrusiveRefCntPtr<ModuleCache> clang::createCrossProcessModuleCache() {
66-
return llvm::makeIntrusiveRefCnt<CrossProcessModuleCache>();
71+
IntrusiveRefCntPtr<ModuleCache>
72+
clang::createCrossProcessModuleCache(std::time_t BuildSessionTimestamp) {
73+
return llvm::makeIntrusiveRefCnt<CrossProcessModuleCache>(
74+
BuildSessionTimestamp);
6775
}

clang/lib/Serialization/ModuleManager.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,11 +174,11 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
174174
NewModule->Index = Chain.size();
175175
NewModule->FileName = FileName.str();
176176
NewModule->ImportLoc = ImportLoc;
177-
NewModule->InputFilesValidationTimestamp = 0;
177+
NewModule->InputFilesValidated = false;
178178

179179
if (NewModule->Kind == MK_ImplicitModule)
180-
NewModule->InputFilesValidationTimestamp =
181-
ModCache->getModuleTimestamp(NewModule->FileName);
180+
NewModule->InputFilesValidated =
181+
ModCache->isMarkedUpToDate(NewModule->FileName);
182182

183183
// Load the contents of the module
184184
if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) {

clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@ using namespace dependencies;
1414

1515
DependencyScanningService::DependencyScanningService(
1616
ScanningMode Mode, ScanningOutputFormat Format,
17-
ScanningOptimizations OptimizeArgs, bool EagerLoadModules, bool TraceVFS,
18-
std::time_t BuildSessionTimestamp)
17+
ScanningOptimizations OptimizeArgs, bool EagerLoadModules, bool TraceVFS)
1918
: Mode(Mode), Format(Format), OptimizeArgs(OptimizeArgs),
20-
EagerLoadModules(EagerLoadModules), TraceVFS(TraceVFS),
21-
BuildSessionTimestamp(BuildSessionTimestamp) {}
19+
EagerLoadModules(EagerLoadModules), TraceVFS(TraceVFS) {}

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -428,10 +428,6 @@ class DependencyScanningAction : public tooling::ToolAction {
428428
ScanInstance.getPreprocessorOpts().AllowPCHWithDifferentModulesCachePath =
429429
true;
430430

431-
if (ScanInstance.getHeaderSearchOpts().ModulesValidateOncePerBuildSession)
432-
ScanInstance.getHeaderSearchOpts().BuildSessionTimestamp =
433-
Service.getBuildSessionTimestamp();
434-
435431
ScanInstance.getFrontendOpts().GenerateGlobalModuleIndex = false;
436432
ScanInstance.getFrontendOpts().UseGlobalModuleIndex = false;
437433
// This will prevent us compiling individual modules asynchronously since

clang/lib/Tooling/DependencyScanning/InProcessModuleCache.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,29 +75,29 @@ class InProcessModuleCache : public ModuleCache {
7575
return std::make_unique<ReaderWriterLock>(CompilationMutex);
7676
}
7777

78-
std::time_t getModuleTimestamp(StringRef Filename) override {
79-
auto &Timestamp = [&]() -> std::atomic<std::time_t> & {
78+
bool isMarkedUpToDate(StringRef Filename) override {
79+
auto &IsUpToDate = [&]() -> std::atomic<bool> & {
8080
std::lock_guard<std::mutex> Lock(Entries.Mutex);
8181
auto &Entry = Entries.Map[Filename];
8282
if (!Entry)
8383
Entry = std::make_unique<ModuleCacheEntry>();
84-
return Entry->Timestamp;
84+
return Entry->UpToDate;
8585
}();
8686

87-
return Timestamp.load();
87+
return IsUpToDate;
8888
}
8989

90-
void updateModuleTimestamp(StringRef Filename) override {
90+
void markUpToDate(StringRef Filename) override {
9191
// Note: This essentially replaces FS contention with mutex contention.
92-
auto &Timestamp = [&]() -> std::atomic<std::time_t> & {
92+
auto &IsUpToDate = [&]() -> std::atomic<bool> & {
9393
std::lock_guard<std::mutex> Lock(Entries.Mutex);
9494
auto &Entry = Entries.Map[Filename];
9595
if (!Entry)
9696
Entry = std::make_unique<ModuleCacheEntry>();
97-
return Entry->Timestamp;
97+
return Entry->UpToDate;
9898
}();
9999

100-
Timestamp.store(llvm::sys::toTimeT(std::chrono::system_clock::now()));
100+
IsUpToDate = true;
101101
}
102102

103103
InMemoryModuleCache &getInMemoryModuleCache() override { return InMemory; }

0 commit comments

Comments
 (0)