Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 0 additions & 5 deletions clang/include/clang/Frontend/CompilerInstance.h
Original file line number Diff line number Diff line change
Expand Up @@ -746,11 +746,6 @@ class CompilerInstance : public ModuleLoader {
GetDependencyDirectives = std::move(Getter);
}

std::string getSpecificModuleCachePath(StringRef ContextHash);
std::string getSpecificModuleCachePath() {
return getSpecificModuleCachePath(getInvocation().computeContextHash());
}

/// Create the AST context.
void createASTContext();

Expand Down
28 changes: 20 additions & 8 deletions clang/include/clang/Lex/HeaderSearch.h
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,10 @@ class HeaderSearch {
/// The specific module cache path containing ContextHash (unless suppressed).
std::string SpecificModuleCachePath;

/// The length of the normalized module cache path at the start of \c
/// SpecificModuleCachePath.
size_t NormalizedModuleCachePathLen = 0;

/// All of the preprocessor-specific data about files that are
/// included, indexed by the FileEntry's UID.
mutable std::vector<HeaderFileInfo> FileInfo;
Expand Down Expand Up @@ -467,20 +471,23 @@ class HeaderSearch {
return {};
}

/// Set the context hash to use for module cache paths.
void setContextHash(StringRef Hash) { ContextHash = std::string(Hash); }
/// Initialize the module cache path.
void initializeModuleCachePath(std::string ContextHash);

/// Set the module cache path with the context hash (unless suppressed).
void setSpecificModuleCachePath(StringRef Path) {
SpecificModuleCachePath = std::string(Path);
/// Retrieve the specific module cache path. This is the normalized module
/// cache path plus the context hash (unless suppressed).
StringRef getSpecificModuleCachePath() const {
return SpecificModuleCachePath;
}

/// Retrieve the context hash.
StringRef getContextHash() const { return ContextHash; }

/// Retrieve the module cache path with the context hash (unless suppressed).
StringRef getSpecificModuleCachePath() const {
return SpecificModuleCachePath;
/// Retrieve the normalized module cache path. This is the path as provided on
/// the command line, but absolute, without './' components, and with
/// preferred path separators. Note that this does not have the context hash.
StringRef getNormalizedModuleCachePath() const {
return getSpecificModuleCachePath().substr(0, NormalizedModuleCachePathLen);
}

/// Forget everything we know about headers so far.
Expand Down Expand Up @@ -1042,6 +1049,11 @@ void ApplyHeaderSearchOptions(HeaderSearch &HS,
void normalizeModuleCachePath(FileManager &FileMgr, StringRef Path,
SmallVectorImpl<char> &NormalizedPath);

std::string createSpecificModuleCachePath(FileManager &FileMgr,
Copy link
Member

Choose a reason for hiding this comment

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

I notice some callsites now call this vs previously calling getSpecificModuleCachePath which always computes the SpecificModuleCachePath. How come? Naively I'd expect we only need to compute it once. Is it because HeaderSearch is not always reachable? IMO it's better to always reuse to reduce chances of misalignment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the call to getSpecificModuleCachePath() (now createSpecificModuleCachePath()) is in "clang/lib/Frontend/FrontendAction.cpp". There we don't have a Preprocessor or HeaderSearch set up yet, since we're only in the process of massaging PreprocessorOptions. This is why we need to compute it ourselves.

The second call is done from HeaderSearch::initializeModuleCachePath() which we call in "clang/lib/Frontend/CompilerInstance.cpp". I think this is indeed redundant with the first call, but I'm not 100% sure.

I personally don't mind this that much, but if we can find an elegant way to de-duplicate these calls, that'd be a good follow-up.

Copy link
Member

Choose a reason for hiding this comment

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

Could the path be saved within HeaderSearchOptions objects? AFAICT, that's always available at callsites of createSpecificModuleCachePath. That way HeaderSearch still has access to it since it's passed in the constructor. If it ever needed to be updated, it would need to go through the HeaderSearchOption instance.

This is not a big enough concern to block this PR, but I'm more generally worried about what kinds of bugs would happen if accidental misalignment happened.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, my long-term goal is to prevent any mutation of CompilerInvocation within the compiler, so that we can consistently leverage its CoW behavior, at least in the scanner. We can't do this earlier, during command-line parsing, because we haven't initialized the VFS yet.

TBH I'm not 100% sure these two calls do (or are supposed to) produce the same specific module cache path, so I'd like to defer this change to a separate PR if possible.

Copy link
Member

@cyndyishida cyndyishida Mar 12, 2026

Choose a reason for hiding this comment

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

I'm not 100% sure these two calls do (or are supposed to)

Can you add a FIXME in that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in ad7c29d.

StringRef ModuleCachePath,
bool DisableModuleHash,
std::string ContextHash);

} // namespace clang

#endif // LLVM_CLANG_LEX_HEADERSEARCH_H
2 changes: 1 addition & 1 deletion clang/include/clang/Serialization/ASTBitCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ namespace serialization {
/// Version 4 of AST files also requires that the version control branch and
/// revision match exactly, since there is no backward compatibility of
/// AST files at this time.
const unsigned VERSION_MAJOR = 35;
const unsigned VERSION_MAJOR = 36;

/// AST file minor version number supported by this version of
/// Clang.
Expand Down
9 changes: 3 additions & 6 deletions clang/include/clang/Serialization/ASTReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,7 @@ class ASTReaderListener {
/// otherwise.
virtual bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
StringRef ModuleFilename,
StringRef SpecificModuleCachePath,
bool Complain) {
StringRef ContextHash, bool Complain) {
return false;
}

Expand Down Expand Up @@ -304,8 +303,7 @@ class ChainedASTReaderListener : public ASTReaderListener {
bool Complain) override;

bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
StringRef ModuleFilename,
StringRef SpecificModuleCachePath,
StringRef ModuleFilename, StringRef ContextHash,
bool Complain) override;
bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
StringRef ModuleFilename, bool ReadMacros,
Expand Down Expand Up @@ -349,8 +347,7 @@ class PCHValidator : public ASTReaderListener {
bool Complain,
std::string &SuggestedPredefines) override;
bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
StringRef ModuleFilename,
StringRef SpecificModuleCachePath,
StringRef ModuleFilename, StringRef ContextHash,
bool Complain) override;
void ReadCounter(const serialization::ModuleFile &M, uint32_t Value) override;
};
Expand Down
3 changes: 1 addition & 2 deletions clang/lib/DependencyScanning/DependencyScannerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,7 @@ class PrebuiltModuleListener : public ASTReaderListener {
/// Check the header search options for a given module when considering
/// if the module comes from stable directories.
bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
StringRef ModuleFilename,
StringRef SpecificModuleCachePath,
StringRef ModuleFilename, StringRef ContextHash,
bool Complain) override {

auto PrebuiltMapEntry = PrebuiltModulesASTMap.try_emplace(CurrentFile);
Expand Down
23 changes: 11 additions & 12 deletions clang/lib/Frontend/ASTUnit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -505,22 +505,21 @@ namespace {
/// a Preprocessor.
class ASTInfoCollector : public ASTReaderListener {
HeaderSearchOptions &HSOpts;
std::string &SpecificModuleCachePath;
std::string &ContextHash;
PreprocessorOptions &PPOpts;
LangOptions &LangOpts;
CodeGenOptions &CodeGenOpts;
TargetOptions &TargetOpts;
uint32_t &Counter;

public:
ASTInfoCollector(HeaderSearchOptions &HSOpts,
std::string &SpecificModuleCachePath,
ASTInfoCollector(HeaderSearchOptions &HSOpts, std::string &ContextHash,
PreprocessorOptions &PPOpts, LangOptions &LangOpts,
CodeGenOptions &CodeGenOpts, TargetOptions &TargetOpts,
uint32_t &Counter)
: HSOpts(HSOpts), SpecificModuleCachePath(SpecificModuleCachePath),
PPOpts(PPOpts), LangOpts(LangOpts), CodeGenOpts(CodeGenOpts),
TargetOpts(TargetOpts), Counter(Counter) {}
: HSOpts(HSOpts), ContextHash(ContextHash), PPOpts(PPOpts),
LangOpts(LangOpts), CodeGenOpts(CodeGenOpts), TargetOpts(TargetOpts),
Counter(Counter) {}

bool ReadLanguageOptions(const LangOptions &NewLangOpts,
StringRef ModuleFilename, bool Complain,
Expand All @@ -538,10 +537,10 @@ class ASTInfoCollector : public ASTReaderListener {

bool ReadHeaderSearchOptions(const HeaderSearchOptions &NewHSOpts,
StringRef ModuleFilename,
StringRef NewSpecificModuleCachePath,
StringRef NewContextHash,
bool Complain) override {
HSOpts = NewHSOpts;
SpecificModuleCachePath = NewSpecificModuleCachePath;
ContextHash = NewContextHash;
return false;
}

Expand Down Expand Up @@ -733,13 +732,13 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile(
AST->ModCache = createCrossProcessModuleCache();

// Gather info for preprocessor construction later on.
std::string SpecificModuleCachePath;
std::string ContextHash;
unsigned Counter = 0;
// Using a temporary FileManager since the AST file might specify custom
// HeaderSearchOptions::VFSOverlayFiles that affect the underlying VFS.
FileManager TmpFileMgr(FileSystemOpts, VFS);
ASTInfoCollector Collector(*AST->HSOpts, SpecificModuleCachePath,
*AST->PPOpts, *AST->LangOpts, *AST->CodeGenOpts,
ASTInfoCollector Collector(*AST->HSOpts, ContextHash, *AST->PPOpts,
*AST->LangOpts, *AST->CodeGenOpts,
*AST->TargetOpts, Counter);
if (ASTReader::readASTFileControlBlock(
Filename, TmpFileMgr, *AST->ModCache, PCHContainerRdr,
Expand All @@ -763,7 +762,7 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile(
AST->getHeaderSearchOpts(), AST->getSourceManager(),
AST->getDiagnostics(), AST->getLangOpts(),
/*Target=*/nullptr);
AST->HeaderInfo->setSpecificModuleCachePath(SpecificModuleCachePath);
AST->HeaderInfo->initializeModuleCachePath(std::move(ContextHash));

AST->PP = std::make_shared<Preprocessor>(
*AST->PPOpts, AST->getDiagnostics(), *AST->LangOpts,
Expand Down
22 changes: 5 additions & 17 deletions clang/lib/Frontend/CompilerInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -487,10 +487,11 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
PP->setPreprocessedOutput(getPreprocessorOutputOpts().ShowCPP);

if (PP->getLangOpts().Modules && PP->getLangOpts().ImplicitModules) {
std::string ContextHash = getInvocation().computeContextHash();
PP->getHeaderSearchInfo().setContextHash(ContextHash);
PP->getHeaderSearchInfo().setSpecificModuleCachePath(
getSpecificModuleCachePath(ContextHash));
// FIXME: We already might've computed the context hash and the specific
// module cache path in `FrontendAction::BeginSourceFile()` when turning
// "-include-pch <DIR>" into "-include-pch <DIR>/<FILE>". Reuse those here.
PP->getHeaderSearchInfo().initializeModuleCachePath(
getInvocation().computeContextHash());
}

// Handle generating dependencies, if requested.
Expand Down Expand Up @@ -546,19 +547,6 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
PP->setDependencyDirectivesGetter(*GetDependencyDirectives);
}

std::string
CompilerInstance::getSpecificModuleCachePath(StringRef ContextHash) {
assert(FileMgr && "Specific module cache path requires a FileManager");

// Set up the module path, including the hash for the module-creation options.
SmallString<256> SpecificModuleCache;
normalizeModuleCachePath(*FileMgr, getHeaderSearchOpts().ModuleCachePath,
SpecificModuleCache);
if (!SpecificModuleCache.empty() && !getHeaderSearchOpts().DisableModuleHash)
llvm::sys::path::append(SpecificModuleCache, ContextHash);
return std::string(SpecificModuleCache);
}

// ASTContext

void CompilerInstance::createASTContext() {
Expand Down
5 changes: 4 additions & 1 deletion clang/lib/Frontend/FrontendAction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,10 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
llvm::sys::path::native(PCHDir->getName(), DirNative);
bool Found = false;
llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem();
std::string SpecificModuleCachePath = CI.getSpecificModuleCachePath();
std::string SpecificModuleCachePath = createSpecificModuleCachePath(
CI.getFileManager(), CI.getHeaderSearchOpts().ModuleCachePath,
CI.getHeaderSearchOpts().DisableModuleHash,
CI.getInvocation().computeContextHash());
for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC),
DirEnd;
Dir != DirEnd && !EC; Dir.increment(EC)) {
Expand Down
12 changes: 9 additions & 3 deletions clang/lib/Frontend/FrontendActions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -623,9 +623,11 @@ namespace {
/// file.
class DumpModuleInfoListener : public ASTReaderListener {
llvm::raw_ostream &Out;
FileManager &FileMgr;

public:
DumpModuleInfoListener(llvm::raw_ostream &Out) : Out(Out) { }
DumpModuleInfoListener(llvm::raw_ostream &Out, FileManager &FileMgr)
: Out(Out), FileMgr(FileMgr) {}

#define DUMP_BOOLEAN(Value, Text) \
Out.indent(4) << Text << ": " << (Value? "Yes" : "No") << "\n"
Expand Down Expand Up @@ -716,8 +718,12 @@ namespace {

bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
StringRef ModuleFilename,
StringRef SpecificModuleCachePath,
StringRef ContextHash,
bool Complain) override {
std::string SpecificModuleCachePath = createSpecificModuleCachePath(
FileMgr, HSOpts.ModuleCachePath, HSOpts.DisableModuleHash,
std::string(ContextHash));

Out.indent(2) << "Header search options:\n";
Out.indent(4) << "System root [-isysroot=]: '" << HSOpts.Sysroot << "'\n";
Out.indent(4) << "Resource dir [ -resource-dir=]: '" << HSOpts.ResourceDir << "'\n";
Expand Down Expand Up @@ -905,7 +911,7 @@ void DumpModuleInfoAction::ExecuteAction() {
Out << " Module format: " << (IsRaw ? "raw" : "obj") << "\n";

Preprocessor &PP = CI.getPreprocessor();
DumpModuleInfoListener Listener(Out);
DumpModuleInfoListener Listener(Out, CI.getFileManager());
const HeaderSearchOptions &HSOpts =
PP.getHeaderSearchInfo().getHeaderSearchOpts();

Expand Down
28 changes: 28 additions & 0 deletions clang/lib/Lex/HeaderSearch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2471,3 +2471,31 @@ void clang::normalizeModuleCachePath(FileManager &FileMgr, StringRef Path,
llvm::sys::path::remove_dots(NormalizedPath);
}
}

static std::string createSpecificModuleCachePathImpl(
FileManager &FileMgr, StringRef ModuleCachePath, bool DisableModuleHash,
std::string ContextHash, size_t &NormalizedModuleCachePathLen) {
SmallString<256> SpecificModuleCachePath;
normalizeModuleCachePath(FileMgr, ModuleCachePath, SpecificModuleCachePath);
NormalizedModuleCachePathLen = SpecificModuleCachePath.size();
if (!SpecificModuleCachePath.empty() && !DisableModuleHash)
llvm::sys::path::append(SpecificModuleCachePath, ContextHash);
return std::string(SpecificModuleCachePath);
}

void HeaderSearch::initializeModuleCachePath(std::string NewContextHash) {
ContextHash = std::move(NewContextHash);
SpecificModuleCachePath = createSpecificModuleCachePathImpl(
FileMgr, HSOpts.ModuleCachePath, HSOpts.DisableModuleHash, ContextHash,
NormalizedModuleCachePathLen);
}

std::string clang::createSpecificModuleCachePath(FileManager &FileMgr,
StringRef ModuleCachePath,
bool DisableModuleHash,
std::string ContextHash) {
size_t NormalizedModuleCachePathLen;
return createSpecificModuleCachePathImpl(
FileMgr, ModuleCachePath, DisableModuleHash, std::move(ContextHash),
NormalizedModuleCachePathLen);
}
Loading