[clang] [clang] Specialize invocation path visitation to the cow (PR #205686)
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 26 09:47:49 PDT 2026
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/205686
>From 102acd7e44c2351e9887d53b80abc93655d15a82 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Tue, 23 Jun 2026 15:53:19 -0700
Subject: [PATCH 1/4] [clang] Specialize invocation path visitation to the cow
---
.../clang/Frontend/CompilerInvocation.h | 50 ++++---
.../include/clang/Frontend/FrontendOptions.h | 2 +-
.../DependencyScanning/ModuleDepCollector.cpp | 4 +-
clang/lib/Frontend/CompilerInvocation.cpp | 123 ++++++++----------
4 files changed, 77 insertions(+), 102 deletions(-)
diff --git a/clang/include/clang/Frontend/CompilerInvocation.h b/clang/include/clang/Frontend/CompilerInvocation.h
index a3bd41a70a4ec..f71969d515be0 100644
--- a/clang/include/clang/Frontend/CompilerInvocation.h
+++ b/clang/include/clang/Frontend/CompilerInvocation.h
@@ -165,13 +165,6 @@ class CompilerInvocationBase {
const ssaf::SSAFOptions &getSSAFOpts() const { return *SSAFOpts; }
/// @}
- /// Visitation.
- /// @{
- /// Visits paths stored in the invocation. The callback may return true to
- /// short-circuit the visitation, or return false to continue visiting.
- void visitPaths(llvm::function_ref<bool(StringRef)> Callback) const;
- /// @}
-
/// Command line generation.
/// @{
using StringAllocator = llvm::function_ref<const char *(const Twine &)>;
@@ -206,12 +199,6 @@ class CompilerInvocationBase {
/// This is a (less-efficient) wrapper over generateCC1CommandLine().
std::vector<std::string> getCC1CommandLine() const;
-protected:
- /// Visits paths stored in the invocation. This is generally unsafe to call
- /// directly, and each sub-class need to ensure calling this doesn't violate
- /// its invariants.
- void visitPathsImpl(llvm::function_ref<bool(std::string &)> Predicate);
-
private:
/// Generate command line options from DiagnosticOptions.
static void GenerateDiagnosticArgs(const DiagnosticOptions &Opts,
@@ -315,14 +302,6 @@ class CompilerInvocation : public CompilerInvocationBase {
template <class R>
R withCowRef(llvm::function_ref<R(const CowCompilerInvocation &)> Fn) const;
- /// Visitation.
- /// @{
- /// Visits paths stored in the invocation. The callback may return true to
- /// short-circuit the visitation, or return false to continue visiting. This
- /// is allowed to mutate the visited paths.
- void visitPaths(llvm::function_ref<bool(std::string &)> Callback);
- /// @}
-
/// Create a compiler invocation from a list of input options.
/// \returns true on success.
///
@@ -445,13 +424,30 @@ class CowCompilerInvocation : public CompilerInvocationBase {
ssaf::SSAFOptions &getMutSSAFOpts();
/// @}
+ /// The result of mutable visitation.
+ struct VisitMutResult {
+ /// Whether to replace the given StringRef with the modified std::string &.
+ bool Replace = false;
+ /// Whether to short-circuit the visitation.
+ bool Terminate = false;
+ };
+
/// Visits paths stored in the invocation, allowing the callback to mutate
- /// them. To preserve the copy-on-write invariant for groups whose paths the
- /// caller might modify, this ensures unique ownership of every option group
- /// up front; if the callback only inspects (and does not mutate) the paths,
- /// the const \c visitPaths overload should be used instead to avoid those
- /// per-group copies.
- void visitMutPaths(llvm::function_ref<bool(std::string &)> Callback);
+ /// them via the out-param. This upholds the same copy-on-write semantics as
+ /// the mutable getters.
+ void visitMutPaths(
+ llvm::function_ref<VisitMutResult(StringRef, std::string &)> Cb);
+
+ /// The result of const visitation.
+ struct VisitConstResult {
+ /// Whether to short-circuit the visitation.
+ bool Terminate = false;
+
+ operator VisitMutResult() const { return {/*Replace=*/false, Terminate}; }
+ };
+
+ /// Visits paths stored in the invocation.
+ void visitPaths(llvm::function_ref<VisitConstResult(StringRef)> Cb) const;
};
template <class R>
diff --git a/clang/include/clang/Frontend/FrontendOptions.h b/clang/include/clang/Frontend/FrontendOptions.h
index a8627ea5d47a4..f65547e68b29d 100644
--- a/clang/include/clang/Frontend/FrontendOptions.h
+++ b/clang/include/clang/Frontend/FrontendOptions.h
@@ -241,7 +241,7 @@ class FrontendInputFile {
/// Whether we're dealing with a 'system' input (vs. a 'user' input).
bool IsSystem = false;
- friend class CompilerInvocationBase;
+ friend class CowCompilerInvocation;
public:
FrontendInputFile() = default;
diff --git a/clang/lib/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/DependencyScanning/ModuleDepCollector.cpp
index c4ca0a35f4c30..c7b7fcc8750c3 100644
--- a/clang/lib/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/DependencyScanning/ModuleDepCollector.cpp
@@ -471,9 +471,9 @@ static bool isSafeToIgnoreCWD(const CowCompilerInvocation &CI) {
// command line inputs use relative paths.
bool AnyRelative = false;
CI.visitPaths([&](StringRef Path) {
- assert(!AnyRelative && "Continuing path visitation despite returning true");
+ assert(!AnyRelative && "Continuing path visitation despite relative path");
AnyRelative |= !Path.empty() && !llvm::sys::path::is_absolute(Path);
- return AnyRelative;
+ return CowCompilerInvocation::VisitConstResult{/*Terminate=*/AnyRelative};
});
return !AnyRelative;
}
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index e2260eb0d078a..7da6a06ad2c73 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -5354,108 +5354,87 @@ std::string CompilerInvocation::computeContextHash() const {
return toString(llvm::APInt(64, Hash), 36, /*Signed=*/false);
}
-void CompilerInvocationBase::visitPathsImpl(
- llvm::function_ref<bool(std::string &)> Predicate) {
-#define RETURN_IF(PATH) \
+void CowCompilerInvocation::visitMutPaths(
+ llvm::function_ref<VisitMutResult(StringRef, std::string &)> Cb) {
+ std::string NewValue;
+
+#define RETURN_IF(OPTS, PATH) \
do { \
- if (Predicate(PATH)) \
+ VisitMutResult Res = Cb(PATH, NewValue); \
+ if (Res.Replace) { \
+ (void)ensureOwned(OPTS); \
+ PATH = ""; \
+ std::swap(PATH, NewValue); \
+ } \
+ if (Res.Terminate) \
return; \
} while (0)
-#define RETURN_IF_MANY(PATHS) \
+#define RETURN_IF_MANY(OPTS, PATHS) \
do { \
- if (llvm::any_of(PATHS, Predicate)) \
- return; \
+ for (unsigned I = 0, E = PATHS.size(); I != E; ++I) \
+ RETURN_IF(OPTS, PATHS[I]); \
} while (0)
- auto &HeaderSearchOpts = *this->HSOpts;
// Header search paths.
- RETURN_IF(HeaderSearchOpts.Sysroot);
- for (auto &Entry : HeaderSearchOpts.UserEntries)
+ RETURN_IF(HSOpts, HSOpts->Sysroot);
+ for (auto &Entry : HSOpts->UserEntries)
if (Entry.IgnoreSysRoot)
- RETURN_IF(Entry.Path);
- RETURN_IF(HeaderSearchOpts.ResourceDir);
- RETURN_IF(HeaderSearchOpts.ModuleCachePath);
- RETURN_IF(HeaderSearchOpts.ModuleUserBuildPath);
- for (auto &[Name, File] : HeaderSearchOpts.PrebuiltModuleFiles)
- RETURN_IF(File);
- RETURN_IF_MANY(HeaderSearchOpts.PrebuiltModulePaths);
- RETURN_IF_MANY(HeaderSearchOpts.VFSOverlayFiles);
+ RETURN_IF(HSOpts, Entry.Path);
+ RETURN_IF(HSOpts, HSOpts->ResourceDir);
+ RETURN_IF(HSOpts, HSOpts->ModuleCachePath);
+ RETURN_IF(HSOpts, HSOpts->ModuleUserBuildPath);
+ for (auto &[Name, File] : HSOpts->PrebuiltModuleFiles)
+ RETURN_IF(HSOpts, File);
+ RETURN_IF_MANY(HSOpts, HSOpts->PrebuiltModulePaths);
+ RETURN_IF_MANY(HSOpts, HSOpts->VFSOverlayFiles);
// Preprocessor options.
- auto &PPOpts = *this->PPOpts;
- RETURN_IF_MANY(PPOpts.MacroIncludes);
- RETURN_IF_MANY(PPOpts.Includes);
- RETURN_IF(PPOpts.ImplicitPCHInclude);
+ RETURN_IF_MANY(PPOpts, PPOpts->MacroIncludes);
+ RETURN_IF_MANY(PPOpts, PPOpts->Includes);
+ RETURN_IF(PPOpts, PPOpts->ImplicitPCHInclude);
// Frontend options.
- auto &FrontendOpts = *this->FrontendOpts;
- for (auto &Input : FrontendOpts.Inputs) {
+ for (auto &Input : FrontendOpts->Inputs) {
if (Input.isBuffer())
continue;
- RETURN_IF(Input.File);
+ RETURN_IF(FrontendOpts, Input.File);
}
- // TODO: Also report output files such as FrontendOpts.OutputFile;
- RETURN_IF(FrontendOpts.CodeCompletionAt.FileName);
- RETURN_IF_MANY(FrontendOpts.ModuleMapFiles);
- RETURN_IF_MANY(FrontendOpts.ModuleFiles);
- RETURN_IF_MANY(FrontendOpts.ModulesEmbedFiles);
- RETURN_IF_MANY(FrontendOpts.ASTMergeFiles);
- RETURN_IF(FrontendOpts.OverrideRecordLayoutsFile);
- RETURN_IF(FrontendOpts.StatsFile);
+ // TODO: Also report output files such as FrontendOpts->OutputFile;
+ RETURN_IF(FrontendOpts, FrontendOpts->CodeCompletionAt.FileName);
+ RETURN_IF_MANY(FrontendOpts, FrontendOpts->ModuleMapFiles);
+ RETURN_IF_MANY(FrontendOpts, FrontendOpts->ModuleFiles);
+ RETURN_IF_MANY(FrontendOpts, FrontendOpts->ModulesEmbedFiles);
+ RETURN_IF_MANY(FrontendOpts, FrontendOpts->ASTMergeFiles);
+ RETURN_IF(FrontendOpts, FrontendOpts->OverrideRecordLayoutsFile);
+ RETURN_IF(FrontendOpts, FrontendOpts->StatsFile);
// Filesystem options.
- auto &FileSystemOpts = *this->FSOpts;
- RETURN_IF(FileSystemOpts.WorkingDir);
+ RETURN_IF(FSOpts, FSOpts->WorkingDir);
// Codegen options.
- auto &CodeGenOpts = *this->CodeGenOpts;
- RETURN_IF(CodeGenOpts.DebugCompilationDir);
- RETURN_IF(CodeGenOpts.CoverageCompilationDir);
+ RETURN_IF(CodeGenOpts, CodeGenOpts->DebugCompilationDir);
+ RETURN_IF(CodeGenOpts, CodeGenOpts->CoverageCompilationDir);
// Sanitizer options.
- RETURN_IF_MANY(LangOpts->NoSanitizeFiles);
+ RETURN_IF_MANY(LangOpts, LangOpts->NoSanitizeFiles);
// Coverage mappings.
- RETURN_IF(CodeGenOpts.ProfileInstrumentUsePath);
- RETURN_IF(CodeGenOpts.SampleProfileFile);
- RETURN_IF(CodeGenOpts.ProfileRemappingFile);
+ RETURN_IF(CodeGenOpts, CodeGenOpts->ProfileInstrumentUsePath);
+ RETURN_IF(CodeGenOpts, CodeGenOpts->SampleProfileFile);
+ RETURN_IF(CodeGenOpts, CodeGenOpts->ProfileRemappingFile);
// Dependency output options.
for (auto &ExtraDep : DependencyOutputOpts->ExtraDeps)
- RETURN_IF(ExtraDep.first);
+ RETURN_IF(DependencyOutputOpts, ExtraDep.first);
}
-void CompilerInvocationBase::visitPaths(
- llvm::function_ref<bool(StringRef)> Callback) const {
- // The const_cast here is OK, because visitPathsImpl() itself doesn't modify
- // the invocation, and our callback takes immutable StringRefs.
- return const_cast<CompilerInvocationBase *>(this)->visitPathsImpl(
- [&Callback](std::string &Path) { return Callback(StringRef(Path)); });
-}
-
-void CowCompilerInvocation::visitMutPaths(
- llvm::function_ref<bool(std::string &)> Callback) {
- // Ensure exclusive ownership of every option group, so that visitPathsImpl()
- // doesn't affect any other invocations.
- // FIXME: Do this only if \c Callback does decide to modify any strings in an
- // option group.
- (void)ensureOwned(LangOpts);
- (void)ensureOwned(TargetOpts);
- (void)ensureOwned(DiagnosticOpts);
- (void)ensureOwned(HSOpts);
- (void)ensureOwned(PPOpts);
- (void)ensureOwned(AnalyzerOpts);
- (void)ensureOwned(MigratorOpts);
- (void)ensureOwned(APINotesOpts);
- (void)ensureOwned(CodeGenOpts);
- (void)ensureOwned(FSOpts);
- (void)ensureOwned(FrontendOpts);
- (void)ensureOwned(DependencyOutputOpts);
- (void)ensureOwned(PreprocessorOutputOpts);
- (void)ensureOwned(SSAFOpts);
- visitPathsImpl(Callback);
+void CowCompilerInvocation::visitPaths(
+ llvm::function_ref<VisitConstResult(StringRef)> Cb) const {
+ // The const_cast here is OK, because our callback never tries to modify.
+ return const_cast<CowCompilerInvocation *>(this)->visitMutPaths(
+ [&Cb](StringRef Path, std::string &) { return Cb(Path); });
}
void CompilerInvocationBase::generateCC1CommandLine(
>From 0625af631b5af876267d0ecb3eb6f9a8d77ce695 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Fri, 26 Jun 2026 09:10:05 -0700
Subject: [PATCH 2/4] Unit test
---
.../unittests/Frontend/CompilerInvocationTest.cpp | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/clang/unittests/Frontend/CompilerInvocationTest.cpp b/clang/unittests/Frontend/CompilerInvocationTest.cpp
index 2c0c6972cef3f..be537ae88bdf9 100644
--- a/clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ b/clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -212,12 +212,14 @@ TEST(CompilerInvocationTest, CopyOnWriteVisitPaths) {
const HeaderSearchOptions *HSOpts = &B.getHeaderSearchOpts();
const LangOptions *LangOpts = &B.getLangOpts();
- B.visitMutPaths([](std::string &Path) {
+ B.visitMutPaths([](StringRef Path, std::string &NewPath) {
+ CowCompilerInvocation::VisitMutResult Res;
if (Path == "mcp") {
- Path = "pcm";
- return true;
+ NewPath = "pcm";
+ Res.Replace = true;
+ Res.Terminate = true;
}
- return false;
+ return Res;
});
// Modifying a path copies and modifies only one instance of the invocation.
@@ -227,9 +229,8 @@ TEST(CompilerInvocationTest, CopyOnWriteVisitPaths) {
EXPECT_EQ(HSOpts, &A.getHeaderSearchOpts());
EXPECT_EQ(A.getHeaderSearchOpts().ModuleCachePath, "mcp");
- // FIXME: Make this work: Unmodified options are not copied.
- // EXPECT_EQ(LangOpts, &B.getLangOpts());
- (void)LangOpts;
+ // Unmodified options are not copied.
+ EXPECT_EQ(LangOpts, &B.getLangOpts());
}
// Boolean option with a keypath that defaults to true.
>From f6246207b4ddf4f4ad68d91561d8684763a60dcc Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Fri, 26 Jun 2026 09:15:44 -0700
Subject: [PATCH 3/4] std::string::clear()
---
clang/lib/Frontend/CompilerInvocation.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 7da6a06ad2c73..ce2a6cb7a4574 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -5363,8 +5363,8 @@ void CowCompilerInvocation::visitMutPaths(
VisitMutResult Res = Cb(PATH, NewValue); \
if (Res.Replace) { \
(void)ensureOwned(OPTS); \
- PATH = ""; \
- std::swap(PATH, NewValue); \
+ PATH = NewValue; \
+ NewValue.clear(); \
} \
if (Res.Terminate) \
return; \
>From fafe2a8d382401a7c2b262767f296084be08f0b4 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Fri, 26 Jun 2026 09:45:57 -0700
Subject: [PATCH 4/4] clear + swap
---
clang/lib/Frontend/CompilerInvocation.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index ce2a6cb7a4574..ce1d7dc98e99c 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -5363,8 +5363,8 @@ void CowCompilerInvocation::visitMutPaths(
VisitMutResult Res = Cb(PATH, NewValue); \
if (Res.Replace) { \
(void)ensureOwned(OPTS); \
- PATH = NewValue; \
- NewValue.clear(); \
+ PATH.clear(); \
+ std::swap(PATH, NewValue); \
} \
if (Res.Terminate) \
return; \
More information about the cfe-commits
mailing list