[clang] Specialize invocation path visitation to the cow#205686
Merged
jansvoboda11 merged 4 commits intoJun 26, 2026
Conversation
Contributor
Author
|
Depends on #205632. |
llvmorg-github-actions
Bot
added
the
clang
|
@llvm/pr-subscribers-clang Author: Jan Svoboda (jansvoboda11) ChangesAfter swiftlang#13201, the only clients of the compiler invocation's Full diff: https://github.com/llvm/llvm-project/pull/205686.diff 7 Files Affected:
diff --git a/clang/include/clang/DependencyScanning/DependencyActionController.h b/clang/include/clang/DependencyScanning/DependencyActionController.h
index 024b0de9048ec..023f080b767cc 100644
--- a/clang/include/clang/DependencyScanning/DependencyActionController.h
+++ b/clang/include/clang/DependencyScanning/DependencyActionController.h
@@ -61,7 +61,7 @@ class DependencyActionController {
/// Finalizes the scan instance and modifies the resulting TU invocation.
/// Returns true on success, false on failure.
virtual bool finalize(CompilerInstance &ScanInstance,
- CompilerInvocation &NewInvocation) {
+ CowCompilerInvocation &NewInvocation) {
return true;
}
diff --git a/clang/include/clang/Frontend/CompilerInvocation.h b/clang/include/clang/Frontend/CompilerInvocation.h
index 03097aefacf50..f71969d515be0 100644
--- a/clang/include/clang/Frontend/CompilerInvocation.h
+++ b/clang/include/clang/Frontend/CompilerInvocation.h
@@ -21,8 +21,10 @@
#include "clang/Frontend/MigratorOptions.h"
#include "clang/Frontend/PreprocessorOutputOptions.h"
#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
-#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/ScopeExit.h"
+
#include <memory>
#include <string>
@@ -127,6 +129,9 @@ class CompilerInvocationBase {
/// prevent creation of the reference-counted option objects.
struct EmptyConstructor {};
+ /// Tag for the shallow-copy constructor below.
+ struct ShallowConstructor {};
+
CompilerInvocationBase();
CompilerInvocationBase(EmptyConstructor) {}
CompilerInvocationBase(const CompilerInvocationBase &X) = delete;
@@ -160,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 &)>;
@@ -201,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,
@@ -251,6 +243,15 @@ class CompilerInvocation : public CompilerInvocationBase {
explicit CompilerInvocation(const CowCompilerInvocation &X);
CompilerInvocation &operator=(const CowCompilerInvocation &X);
+ /// Move-construct/move-assign from a \c CowCompilerInvocation. Steals the
+ /// (potentially copy-on-written) option group pointers without deep-copying;
+ /// \p X is left empty. Useful to receive results of mutating a temporary
+ /// Cow alias back into a \c CompilerInvocation.
+ /// @{
+ explicit CompilerInvocation(CowCompilerInvocation &&X);
+ CompilerInvocation &operator=(CowCompilerInvocation &&X);
+ /// @}
+
/// Const getters.
/// @{
// Note: These need to be pulled in manually. Otherwise, they get hidden by
@@ -293,6 +294,14 @@ class CompilerInvocation : public CompilerInvocationBase {
ssaf::SSAFOptions &getSSAFOpts() { return *SSAFOpts; }
/// @}
+ /// Invokes the \a Fn with CowCompilerInvocation representing \c this.
+ /// The \a Fn must not directly modify \c this.
+ /// The provided \c CowCompilerInvocation must not escape \a Fn.
+ template <class R>
+ R withCowRef(llvm::function_ref<R(CowCompilerInvocation &)> Fn);
+ template <class R>
+ R withCowRef(llvm::function_ref<R(const CowCompilerInvocation &)> Fn) const;
+
/// Create a compiler invocation from a list of input options.
/// \returns true on success.
///
@@ -385,6 +394,16 @@ class CowCompilerInvocation : public CompilerInvocationBase {
CowCompilerInvocation(CompilerInvocation &&X)
: CompilerInvocationBase(std::move(X)) {}
+ /// Construct a CowCompilerInvocation that aliases the option storage of \p
+ /// X without deep-copying. Subsequent mutations through getMut*Opts() will
+ /// copy-on-write per group as usual, leaving \p X unaffected. The caller
+ /// must guarantee that \p X is not mutated for the lifetime of the
+ /// constructed invocation.
+ CowCompilerInvocation(ShallowConstructor, const CompilerInvocation &X)
+ : CompilerInvocationBase(EmptyConstructor{}) {
+ shallow_copy_assign(X);
+ }
+
// Const getters are inherited from the base class.
/// Mutable getters.
@@ -404,8 +423,63 @@ class CowCompilerInvocation : public CompilerInvocationBase {
PreprocessorOutputOptions &getMutPreprocessorOutputOpts();
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 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>
+R CompilerInvocation::withCowRef(
+ llvm::function_ref<R(CowCompilerInvocation &)> Fn) {
+ // We use moves to avoid bumping the ref-count of the shared_ptr that holds
+ // individual options. Since we expect \a Fn to actually modify \c CowRef,
+ // this prevents temporary copies.
+ CowCompilerInvocation CowRef = std::move(*this);
+ llvm::scope_exit Mutate([&]() { *this = std::move(CowRef); });
+ return Fn(CowRef);
+}
+
+template <class R>
+R CompilerInvocation::withCowRef(
+ llvm::function_ref<R(const CowCompilerInvocation &)> Fn) const {
+ // We use the shallow constructor. Since \a Fn cannot modify \c CowRef, no
+ // copies will be created, despite the bump to the ref-count of the shared_ptr
+ // that holds individual options.
+ CowCompilerInvocation CowRef(ShallowConstructor{}, *this);
+ return Fn(CowRef);
+}
+
+inline CompilerInvocation::CompilerInvocation(CowCompilerInvocation &&X)
+ : CompilerInvocationBase(std::move(X)) {}
+
+inline CompilerInvocation &
+CompilerInvocation::operator=(CowCompilerInvocation &&X) {
+ CompilerInvocationBase::operator=(std::move(X));
+ return *this;
+}
+
IntrusiveRefCntPtr<llvm::vfs::FileSystem>
createVFSFromCompilerInvocation(const CompilerInvocation &CI,
DiagnosticsEngine &Diags);
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/DependencyScannerImpl.cpp b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp
index dc3dbe3603c01..68fda9227dfcb 100644
--- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp
+++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp
@@ -713,7 +713,11 @@ bool DependencyScanningAction::runInvocation(
if (MDC)
MDC->applyDiscoveredDependencies(*OriginalInvocation);
- if (!Controller.finalize(ScanInstance, *OriginalInvocation))
+ bool Success = OriginalInvocation->withCowRef<bool>(
+ [&](CowCompilerInvocation &CowOriginalInvocation) {
+ return Controller.finalize(ScanInstance, CowOriginalInvocation);
+ });
+ if (!Success)
return false;
Consumer.handleBuildCommand(
@@ -791,7 +795,11 @@ bool DependencyScanningAction::runInvocation(
MDC->applyDiscoveredDependencies(*OriginalInvocation);
}
- if (!Controller.finalize(ScanInstance, *OriginalInvocation))
+ bool Success = OriginalInvocation->withCowRef<bool>(
+ [&](CowCompilerInvocation &CowOriginalInvocation) {
+ return Controller.finalize(ScanInstance, CowOriginalInvocation);
+ });
+ if (!Success)
return false;
Consumer.handleBuildCommand(
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 dfde7b756dbff..7da6a06ad2c73 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -5354,85 +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::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(
diff --git a/clang/lib/Tooling/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanningTool.cpp
index d55367107862d..11b225830c2fc 100644
--- a/clang/lib/Tooling/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanningTool.cpp
@@ -587,7 +587,11 @@ bool CompilerInstanceWithContext::computeDependencies(
MDC->run(Consumer);
MDC->applyDiscoveredDependencies(ModuleInvocation);
- if (!Controller.finalize(CI, ModuleInvocation))
+ bool Success = ModuleInvocation.withCowRef<bool>(
+ [&](CowCompilerInvocation &CowModuleInvocation) {
+ return Controller.finalize(CI, CowModuleInvocation);
+ });
+ if (!Success)
return false;
Consumer.handleBuildCommand(
|
benlangmuir
reviewed
Jun 26, 2026
benlangmuir
left a comment
benlangmuir
left a comment
Contributor
There was a problem hiding this comment.
Would be good to have a simple test that we get a copy of only the modified options object.
jansvoboda11
force-pushed
the
invocation-copies-visitation
branch
from
June 26, 2026 16:10
f5cc14d to
0d1ec78
Compare
benlangmuir
reviewed
Jun 26, 2026
jansvoboda11
force-pushed
the
invocation-copies-visitation
branch
from
June 26, 2026 16:47
bf6672b to
fafe2a8
Compare
benlangmuir
approved these changes
Jun 26, 2026
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/30810 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/154/builds/33160 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/94/builds/18803 Here is the relevant piece of the build log for the reference |
LouisLu060211
pushed a commit
to LouisLu060211/llvm-project
that referenced
this pull request
Jun 30, 2026
After llvm#205632, the only clients of the compiler invocation's `visitPaths()` APIs call it on the cow variant. This PR moves the implementation from the base class into the cow, allowing specialized copy-on-write behavior for mutating visitation. If the callback requests a path to be mutated, exclusive ownership of the containing `*Options` instance is established and the string gets modified. This should be performance win for dependency scans using prefix mapping, although admittedly I haven't benchmarked.
maarcosrmz
pushed a commit
to maarcosrmz/llvm-project
that referenced
this pull request
Jul 1, 2026
After llvm#205632, the only clients of the compiler invocation's `visitPaths()` APIs call it on the cow variant. This PR moves the implementation from the base class into the cow, allowing specialized copy-on-write behavior for mutating visitation. If the callback requests a path to be mutated, exclusive ownership of the containing `*Options` instance is established and the string gets modified. This should be performance win for dependency scans using prefix mapping, although admittedly I haven't benchmarked.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After #205632, the only clients of the compiler invocation's
visitPaths()APIs call it on the cow variant. This PR moves the implementation from the base class into the cow, allowing specialized copy-on-write behavior for mutating visitation. If the callback requests a path to be mutated, exclusive ownership of the containing*Optionsinstance is established and the string gets modified. This should be performance win for dependency scans using prefix mapping, although admittedly I haven't benchmarked.