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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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; \ _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
