Skip to content

[clang][modules] Timestamp PCM files when writing #112452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 22, 2024
Merged

[clang][modules] Timestamp PCM files when writing #112452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6550eec
[clang][modules] Timestamp PCM files when writing
jansvoboda11 Oct 15, 2024
27abdf7
Only write timestamp file with `-fmodules-validate-once-per-build-ses…
jansvoboda11 Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions clang/include/clang/Serialization/ModuleFile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#define LLVM_CLANG_SERIALIZATION_MODULEFILE_H

#include "clang/Basic/FileManager.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/Module.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Serialization/ASTBitCodes.h"
Expand Down Expand Up @@ -144,8 +145,8 @@ class ModuleFile {
/// The base directory of the module.
std::string BaseDirectory;

std::string getTimestampFilename() const {
return FileName + ".timestamp";
static std::string getTimestampFilename(StringRef FileName) {
return (FileName + ".timestamp").str();
}

/// The original source file name that was used to build the
Expand Down
15 changes: 15 additions & 0 deletions clang/lib/Serialization/ASTCommon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
#include "clang/AST/DeclObjC.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Serialization/ASTDeserializationListener.h"
#include "clang/Serialization/ModuleFile.h"
#include "llvm/Support/DJB.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/raw_ostream.h"

using namespace clang;

Expand Down Expand Up @@ -503,3 +506,15 @@ bool serialization::needsAnonymousDeclarationNumber(const NamedDecl *D) {
return false;
return isa<TagDecl, FieldDecl>(D);
}

void serialization::updateModuleTimestamp(StringRef ModuleFilename) {
// Overwrite the timestamp file contents so that file's mtime changes.
std::error_code EC;
llvm::raw_fd_ostream OS(ModuleFile::getTimestampFilename(ModuleFilename), EC,
llvm::sys::fs::OF_TextWithCRLF);
if (EC)
return;
OS << "Timestamp file\n";
OS.close();
OS.clear_error(); // Avoid triggering a fatal error.
}
3 changes: 3 additions & 0 deletions clang/lib/Serialization/ASTCommon.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

#include "clang/AST/ASTContext.h"
#include "clang/AST/DeclFriend.h"
#include "clang/Basic/LLVM.h"
#include "clang/Serialization/ASTBitCodes.h"

namespace clang {
Expand Down Expand Up @@ -100,6 +101,8 @@ inline bool isPartOfPerModuleInitializer(const Decl *D) {
return false;
}

void updateModuleTimestamp(StringRef ModuleFilename);

} // namespace serialization

} // namespace clang
Expand Down
15 changes: 1 addition & 14 deletions clang/lib/Serialization/ASTReader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4416,19 +4416,6 @@ bool ASTReader::isGlobalIndexUnavailable() const {
!hasGlobalIndex() && TriedLoadingGlobalIndex;
}

static void updateModuleTimestamp(ModuleFile &MF) {
// Overwrite the timestamp file contents so that file's mtime changes.
std::string TimestampFilename = MF.getTimestampFilename();
std::error_code EC;
llvm::raw_fd_ostream OS(TimestampFilename, EC,
llvm::sys::fs::OF_TextWithCRLF);
if (EC)
return;
OS << "Timestamp file\n";
OS.close();
OS.clear_error(); // Avoid triggering a fatal error.
}

/// Given a cursor at the start of an AST file, scan ahead and drop the
/// cursor into the start of the given block ID, returning false on success and
/// true on failure.
Expand Down Expand Up @@ -4707,7 +4694,7 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName, ModuleKind Type,
ImportedModule &M = Loaded[I];
if (M.Mod->Kind == MK_ImplicitModule &&
M.Mod->InputFilesValidationTimestamp < HSOpts.BuildSessionTimestamp)
updateModuleTimestamp(*M.Mod);
updateModuleTimestamp(M.Mod->FileName);
}
}

Expand Down
6 changes: 6 additions & 0 deletions clang/lib/Serialization/ASTWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4905,6 +4905,12 @@ ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef, StringRef OutputFile,
this->BaseDirectory.clear();

WritingAST = false;

if (WritingModule && SemaRef.PP.getHeaderSearchInfo()
.getHeaderSearchOpts()
.ModulesValidateOncePerBuildSession)
updateModuleTimestamp(OutputFile);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling this here is a bit annoying, since it will create timestamp files even for explicitly-built PCM files. I might move it somewhere into CompilerInstance where it specifically handle implicit builds. That will increase the delay between accessing the input files and writing the timestamp file. What do people think about that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'd like to make such changes doesn't affect C++20 modules if possible.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't have a strong opinion about the location of updating the timestamp.

To be consistent with another usage of updateModuleTimestamp shouldn't you check

HeaderSearchOptions &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts();
if (HSOpts.ModulesValidateOncePerBuildSession)

? And I hope it can help with the explicitly-built .pcm files.


if (ShouldCacheASTInMemory) {
// Construct MemoryBuffer and update buffer manager.
ModuleCache.addBuiltPCM(OutputFile,
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Serialization/ModuleManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
NewModule->InputFilesValidationTimestamp = 0;

if (NewModule->Kind == MK_ImplicitModule) {
std::string TimestampFilename = NewModule->getTimestampFilename();
std::string TimestampFilename =
ModuleFile::getTimestampFilename(NewModule->FileName);
llvm::vfs::Status Status;
// A cached stat value would be fine as well.
if (!FileMgr.getNoncachedStatValue(TimestampFilename, Status))
Expand Down
Loading