https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/205686
>From bc6447f27fdf093191b8fa1f9db57d62abbb01b7 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Wed, 24 Jun 2026 11:03:05 -0700 Subject: [PATCH 1/5] [clang][deps] Avoid `CompilerInvocation` copies When constructing the dependency graph for compilation caching, the dependency scanner needs to do some extra operations on the compiler invocations. Historically, these have not utilized the copy-on-write variant well. This patch takes care to minimize `CompilerInvocation` copies, which improves incremental scans with populated up-to-date scanning module cache by 16-18%. Together with https://github.com/llvm/llvm-project/pull/203350 which operates in the same space, wall-times are improved by 1.54x and instruction counts by 1.66x. --- .../DependencyActionController.h | 2 +- .../clang/Frontend/CompilerInvocation.h | 80 ++++++++++++++++++- .../DependencyScannerImpl.cpp | 12 ++- clang/lib/Frontend/CompilerInvocation.cpp | 23 ++++++ clang/lib/Tooling/DependencyScanningTool.cpp | 6 +- 5 files changed, 118 insertions(+), 5 deletions(-) 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..a3bd41a70a4ec 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; @@ -251,6 +256,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 +307,22 @@ 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; + + /// 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. /// @@ -385,6 +415,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 +444,46 @@ class CowCompilerInvocation : public CompilerInvocationBase { PreprocessorOutputOptions &getMutPreprocessorOutputOpts(); ssaf::SSAFOptions &getMutSSAFOpts(); /// @} + + /// 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); }; +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/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/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index dfde7b756dbff..e2260eb0d078a 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -5435,6 +5435,29 @@ void CompilerInvocationBase::visitPaths( [&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 CompilerInvocationBase::generateCC1CommandLine( ArgumentConsumer Consumer) const { llvm::Triple T(getTargetOpts().Triple); 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( >From ce82c7d9780e917f7b2b30a56cca0921ffbab4e4 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Fri, 26 Jun 2026 09:00:09 -0700 Subject: [PATCH 2/5] Unit tests --- .../Frontend/CompilerInvocationTest.cpp | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/clang/unittests/Frontend/CompilerInvocationTest.cpp b/clang/unittests/Frontend/CompilerInvocationTest.cpp index 887fbc5938333..2c0c6972cef3f 100644 --- a/clang/unittests/Frontend/CompilerInvocationTest.cpp +++ b/clang/unittests/Frontend/CompilerInvocationTest.cpp @@ -165,6 +165,73 @@ TEST(CompilerInvocationTest, CopyOnWriteAssignment) { EXPECT_EQ(B.getFrontendOpts().OutputFile, "x.o"); } +TEST(CompilerInvocationTest, WithConstCowRef) { + CompilerInvocation CI; + CI.getHeaderSearchOpts().ModuleCachePath = "mcp"; + + HeaderSearchOptions *HSOpts = &CI.getHeaderSearchOpts(); + + CI.withCowRef<void>([](const CowCompilerInvocation &CowCI) { + // Values stored in the original invocation are reflected in cow. + EXPECT_EQ(CowCI.getHeaderSearchOpts().ModuleCachePath, "mcp"); + }); + + // Creating const cow reference does not make a copy. + EXPECT_EQ(HSOpts, &CI.getHeaderSearchOpts()); +} + +TEST(CompilerInvocationTest, WithMutCowRef) { + CompilerInvocation CI; + CI.getHeaderSearchOpts().ModuleCachePath = "mcp"; + CI.getLangOpts().Modules = true; + + HeaderSearchOptions *HSOpts = &CI.getHeaderSearchOpts(); + LangOptions *LangOpts = &CI.getLangOpts(); + + CI.withCowRef<void>([](CowCompilerInvocation &CowCI) { + // Values stored in the original invocation are reflected in cow. + EXPECT_EQ(CowCI.getHeaderSearchOpts().ModuleCachePath, "mcp"); + // Values can be mutated. + CowCI.getMutLangOpts().Modules = false; + }); + + // Reading options class on a non-const cow reference does not make a copy. + EXPECT_EQ(HSOpts, &CI.getHeaderSearchOpts()); + // Writing options class on a non-const cow reference does not make a copy. + EXPECT_EQ(LangOpts, &CI.getLangOpts()); + // Writing options class on a non-const cow reference modifies the original. + EXPECT_EQ(CI.getLangOpts().Modules, false); +} + +TEST(CompilerInvocationTest, CopyOnWriteVisitPaths) { + CowCompilerInvocation A; + A.getMutHeaderSearchOpts().ModuleCachePath = "mcp"; + A.getMutLangOpts().Modules = true; + + CowCompilerInvocation B(A); + + const HeaderSearchOptions *HSOpts = &B.getHeaderSearchOpts(); + const LangOptions *LangOpts = &B.getLangOpts(); + B.visitMutPaths([](std::string &Path) { + if (Path == "mcp") { + Path = "pcm"; + return true; + } + return false; + }); + + // Modifying a path copies and modifies only one instance of the invocation. + EXPECT_NE(HSOpts, &B.getHeaderSearchOpts()); + EXPECT_EQ(B.getHeaderSearchOpts().ModuleCachePath, "pcm"); + // And the other instance remains unmodified. + 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; +} + // Boolean option with a keypath that defaults to true. // The only flag with a negative spelling can set the keypath to false. >From 2608f7f639a1b86c11295cff4806364ed8c921d0 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Tue, 23 Jun 2026 15:53:19 -0700 Subject: [PATCH 3/5] [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 0d1ec780116e5531a7cc3e0ce2f0c7287264da16 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Fri, 26 Jun 2026 09:10:05 -0700 Subject: [PATCH 4/5] 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 bf6672b3a8ca66ed1e44e66f79f2cc913d64cd9b Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Fri, 26 Jun 2026 09:15:44 -0700 Subject: [PATCH 5/5] 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; \ _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
