From 366a8d443fedbcec2aa3808e3df8d3b9ca58b047 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Mon, 10 Nov 2025 14:02:13 -0800 Subject: [PATCH 1/4] [clang] Extract `CompilerInvocation::anyPath()` --- .../clang/Frontend/CompilerInvocation.h | 5 ++ clang/lib/Frontend/CompilerInvocation.cpp | 78 ++++++++++++++++++ .../DependencyScanning/ModuleDepCollector.cpp | 79 +------------------ 3 files changed, 86 insertions(+), 76 deletions(-) diff --git a/clang/include/clang/Frontend/CompilerInvocation.h b/clang/include/clang/Frontend/CompilerInvocation.h index e147d2ba6087e..7ca8c210615ab 100644 --- a/clang/include/clang/Frontend/CompilerInvocation.h +++ b/clang/include/clang/Frontend/CompilerInvocation.h @@ -147,6 +147,11 @@ class CompilerInvocationBase { } /// @} + /// Visitation. + /// @{ + bool anyPath(llvm::function_ref Predicate) const; + /// @} + /// Command line generation. /// @{ using StringAllocator = llvm::function_ref; diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index f584a2a5824b2..e710e2e87b032 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -5280,6 +5280,84 @@ std::string CompilerInvocation::getModuleHash() const { return toString(llvm::APInt(64, Hash), 36, /*Signed=*/false); } +bool CompilerInvocationBase::anyPath( + llvm::function_ref Predicate) const { +#define PROPAGATE_TRUE_IF(PATH) \ + do { \ + if (Predicate(PATH)) \ + return true; \ + } while (0) + +#define PROPAGATE_TRUE_IF_MANY(PATHS) \ + do { \ + if (llvm::any_of(PATHS, Predicate)) \ + return true; \ + } while (0) + + // Header search paths. + const auto &HeaderSearchOpts = getHeaderSearchOpts(); + PROPAGATE_TRUE_IF(HeaderSearchOpts.Sysroot); + for (auto &Entry : HeaderSearchOpts.UserEntries) + if (Entry.IgnoreSysRoot) + PROPAGATE_TRUE_IF(Entry.Path); + PROPAGATE_TRUE_IF(HeaderSearchOpts.ResourceDir); + PROPAGATE_TRUE_IF(HeaderSearchOpts.ModuleCachePath); + PROPAGATE_TRUE_IF(HeaderSearchOpts.ModuleUserBuildPath); + for (auto I = HeaderSearchOpts.PrebuiltModuleFiles.begin(), + E = HeaderSearchOpts.PrebuiltModuleFiles.end(); + I != E;) { + auto Current = I++; + PROPAGATE_TRUE_IF(Current->second); + } + PROPAGATE_TRUE_IF_MANY(HeaderSearchOpts.PrebuiltModulePaths); + PROPAGATE_TRUE_IF_MANY(HeaderSearchOpts.VFSOverlayFiles); + + // Preprocessor options. + const auto &PPOpts = getPreprocessorOpts(); + PROPAGATE_TRUE_IF_MANY(PPOpts.MacroIncludes); + PROPAGATE_TRUE_IF_MANY(PPOpts.Includes); + PROPAGATE_TRUE_IF(PPOpts.ImplicitPCHInclude); + + // Frontend options. + const auto &FrontendOpts = getFrontendOpts(); + for (const FrontendInputFile &Input : FrontendOpts.Inputs) { + if (Input.isBuffer()) + continue; // FIXME: Can this happen when parsing command-line? + + PROPAGATE_TRUE_IF(Input.getFile()); + } + PROPAGATE_TRUE_IF(FrontendOpts.CodeCompletionAt.FileName); + PROPAGATE_TRUE_IF_MANY(FrontendOpts.ModuleMapFiles); + PROPAGATE_TRUE_IF_MANY(FrontendOpts.ModuleFiles); + PROPAGATE_TRUE_IF_MANY(FrontendOpts.ModulesEmbedFiles); + PROPAGATE_TRUE_IF_MANY(FrontendOpts.ASTMergeFiles); + PROPAGATE_TRUE_IF(FrontendOpts.OverrideRecordLayoutsFile); + PROPAGATE_TRUE_IF(FrontendOpts.StatsFile); + + // Filesystem options. + const auto &FileSystemOpts = getFileSystemOpts(); + PROPAGATE_TRUE_IF(FileSystemOpts.WorkingDir); + + // Codegen options. + const auto &CodeGenOpts = getCodeGenOpts(); + PROPAGATE_TRUE_IF(CodeGenOpts.DebugCompilationDir); + PROPAGATE_TRUE_IF(CodeGenOpts.CoverageCompilationDir); + + // Sanitizer options. + PROPAGATE_TRUE_IF_MANY(getLangOpts().NoSanitizeFiles); + + // Coverage mappings. + PROPAGATE_TRUE_IF(CodeGenOpts.ProfileInstrumentUsePath); + PROPAGATE_TRUE_IF(CodeGenOpts.SampleProfileFile); + PROPAGATE_TRUE_IF(CodeGenOpts.ProfileRemappingFile); + + // Dependency output options. + for (auto &ExtraDep : getDependencyOutputOpts().ExtraDeps) + PROPAGATE_TRUE_IF(ExtraDep.first); + + return false; +} + void CompilerInvocationBase::generateCC1CommandLine( ArgumentConsumer Consumer) const { llvm::Triple T(getTargetOpts().Triple); diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index e07a208748b77..abf18770a01dc 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -471,82 +471,9 @@ static bool isSafeToIgnoreCWD(const CowCompilerInvocation &CI) { // Check if the command line input uses relative paths. // It is not safe to ignore the current working directory if any of the // command line inputs use relative paths. -#define IF_RELATIVE_RETURN_FALSE(PATH) \ - do { \ - if (!PATH.empty() && !llvm::sys::path::is_absolute(PATH)) \ - return false; \ - } while (0) - -#define IF_ANY_RELATIVE_RETURN_FALSE(PATHS) \ - do { \ - if (llvm::any_of(PATHS, [](const auto &P) { \ - return !P.empty() && !llvm::sys::path::is_absolute(P); \ - })) \ - return false; \ - } while (0) - - // Header search paths. - const auto &HeaderSearchOpts = CI.getHeaderSearchOpts(); - IF_RELATIVE_RETURN_FALSE(HeaderSearchOpts.Sysroot); - for (auto &Entry : HeaderSearchOpts.UserEntries) - if (Entry.IgnoreSysRoot) - IF_RELATIVE_RETURN_FALSE(Entry.Path); - IF_RELATIVE_RETURN_FALSE(HeaderSearchOpts.ResourceDir); - IF_RELATIVE_RETURN_FALSE(HeaderSearchOpts.ModuleCachePath); - IF_RELATIVE_RETURN_FALSE(HeaderSearchOpts.ModuleUserBuildPath); - for (auto I = HeaderSearchOpts.PrebuiltModuleFiles.begin(), - E = HeaderSearchOpts.PrebuiltModuleFiles.end(); - I != E;) { - auto Current = I++; - IF_RELATIVE_RETURN_FALSE(Current->second); - } - IF_ANY_RELATIVE_RETURN_FALSE(HeaderSearchOpts.PrebuiltModulePaths); - IF_ANY_RELATIVE_RETURN_FALSE(HeaderSearchOpts.VFSOverlayFiles); - - // Preprocessor options. - const auto &PPOpts = CI.getPreprocessorOpts(); - IF_ANY_RELATIVE_RETURN_FALSE(PPOpts.MacroIncludes); - IF_ANY_RELATIVE_RETURN_FALSE(PPOpts.Includes); - IF_RELATIVE_RETURN_FALSE(PPOpts.ImplicitPCHInclude); - - // Frontend options. - const auto &FrontendOpts = CI.getFrontendOpts(); - for (const FrontendInputFile &Input : FrontendOpts.Inputs) { - if (Input.isBuffer()) - continue; // FIXME: Can this happen when parsing command-line? - - IF_RELATIVE_RETURN_FALSE(Input.getFile()); - } - IF_RELATIVE_RETURN_FALSE(FrontendOpts.CodeCompletionAt.FileName); - IF_ANY_RELATIVE_RETURN_FALSE(FrontendOpts.ModuleMapFiles); - IF_ANY_RELATIVE_RETURN_FALSE(FrontendOpts.ModuleFiles); - IF_ANY_RELATIVE_RETURN_FALSE(FrontendOpts.ModulesEmbedFiles); - IF_ANY_RELATIVE_RETURN_FALSE(FrontendOpts.ASTMergeFiles); - IF_RELATIVE_RETURN_FALSE(FrontendOpts.OverrideRecordLayoutsFile); - IF_RELATIVE_RETURN_FALSE(FrontendOpts.StatsFile); - - // Filesystem options. - const auto &FileSystemOpts = CI.getFileSystemOpts(); - IF_RELATIVE_RETURN_FALSE(FileSystemOpts.WorkingDir); - - // Codegen options. - const auto &CodeGenOpts = CI.getCodeGenOpts(); - IF_RELATIVE_RETURN_FALSE(CodeGenOpts.DebugCompilationDir); - IF_RELATIVE_RETURN_FALSE(CodeGenOpts.CoverageCompilationDir); - - // Sanitizer options. - IF_ANY_RELATIVE_RETURN_FALSE(CI.getLangOpts().NoSanitizeFiles); - - // Coverage mappings. - IF_RELATIVE_RETURN_FALSE(CodeGenOpts.ProfileInstrumentUsePath); - IF_RELATIVE_RETURN_FALSE(CodeGenOpts.SampleProfileFile); - IF_RELATIVE_RETURN_FALSE(CodeGenOpts.ProfileRemappingFile); - - // Dependency output options. - for (auto &ExtraDep : CI.getDependencyOutputOpts().ExtraDeps) - IF_RELATIVE_RETURN_FALSE(ExtraDep.first); - - return true; + return !CI.anyPath([](StringRef Path) { + return !Path.empty() && !llvm::sys::path::is_absolute(Path); + }); } static std::string getModuleContextHash(const ModuleDeps &MD, From 2dc0d1a3ff2274abf8296d3185b605624e06ddb5 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Mon, 10 Nov 2025 14:19:34 -0800 Subject: [PATCH 2/4] Refactor weird loop --- clang/lib/Frontend/CompilerInvocation.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index e710e2e87b032..6c39d9a38b66e 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -5303,12 +5303,8 @@ bool CompilerInvocationBase::anyPath( PROPAGATE_TRUE_IF(HeaderSearchOpts.ResourceDir); PROPAGATE_TRUE_IF(HeaderSearchOpts.ModuleCachePath); PROPAGATE_TRUE_IF(HeaderSearchOpts.ModuleUserBuildPath); - for (auto I = HeaderSearchOpts.PrebuiltModuleFiles.begin(), - E = HeaderSearchOpts.PrebuiltModuleFiles.end(); - I != E;) { - auto Current = I++; - PROPAGATE_TRUE_IF(Current->second); - } + for (const auto &[Name, File] : HeaderSearchOpts.PrebuiltModuleFiles) + PROPAGATE_TRUE_IF(File); PROPAGATE_TRUE_IF_MANY(HeaderSearchOpts.PrebuiltModulePaths); PROPAGATE_TRUE_IF_MANY(HeaderSearchOpts.VFSOverlayFiles); From 2cdc5ea2c163e18c04a0da8d7814571fb9110613 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Mon, 10 Nov 2025 15:23:15 -0800 Subject: [PATCH 3/4] Extract more generic visitor implementation --- .../clang/Frontend/CompilerInvocation.h | 9 +- .../include/clang/Frontend/FrontendOptions.h | 2 + clang/lib/Frontend/CompilerInvocation.cpp | 92 ++++++++++--------- .../DependencyScanning/ModuleDepCollector.cpp | 8 +- 4 files changed, 65 insertions(+), 46 deletions(-) diff --git a/clang/include/clang/Frontend/CompilerInvocation.h b/clang/include/clang/Frontend/CompilerInvocation.h index 7ca8c210615ab..9f447b4cd630f 100644 --- a/clang/include/clang/Frontend/CompilerInvocation.h +++ b/clang/include/clang/Frontend/CompilerInvocation.h @@ -149,7 +149,9 @@ class CompilerInvocationBase { /// Visitation. /// @{ - bool anyPath(llvm::function_ref Predicate) const; + /// 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 Callback) const; /// @} /// Command line generation. @@ -187,6 +189,11 @@ class CompilerInvocationBase { std::vector getCC1CommandLine() const; private: + /// 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 Predicate); + /// Generate command line options from DiagnosticOptions. static void GenerateDiagnosticArgs(const DiagnosticOptions &Opts, ArgumentConsumer Consumer, diff --git a/clang/include/clang/Frontend/FrontendOptions.h b/clang/include/clang/Frontend/FrontendOptions.h index c919a53ae089e..ba7da56cb9fce 100644 --- a/clang/include/clang/Frontend/FrontendOptions.h +++ b/clang/include/clang/Frontend/FrontendOptions.h @@ -241,6 +241,8 @@ class FrontendInputFile { /// Whether we're dealing with a 'system' input (vs. a 'user' input). bool IsSystem = false; + friend class CompilerInvocationBase; + public: FrontendInputFile() = default; FrontendInputFile(StringRef File, InputKind Kind, bool IsSystem = false) diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 6c39d9a38b66e..a95796924311b 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -5280,78 +5280,84 @@ std::string CompilerInvocation::getModuleHash() const { return toString(llvm::APInt(64, Hash), 36, /*Signed=*/false); } -bool CompilerInvocationBase::anyPath( - llvm::function_ref Predicate) const { -#define PROPAGATE_TRUE_IF(PATH) \ +void CompilerInvocationBase::visitPathsImpl( + llvm::function_ref Predicate) { +#define RETURN_IF(PATH) \ do { \ if (Predicate(PATH)) \ - return true; \ + return; \ } while (0) -#define PROPAGATE_TRUE_IF_MANY(PATHS) \ +#define RETURN_IF_MANY(PATHS) \ do { \ if (llvm::any_of(PATHS, Predicate)) \ - return true; \ + return; \ } while (0) + auto &HeaderSearchOpts = *this->HSOpts; // Header search paths. - const auto &HeaderSearchOpts = getHeaderSearchOpts(); - PROPAGATE_TRUE_IF(HeaderSearchOpts.Sysroot); + RETURN_IF(HeaderSearchOpts.Sysroot); for (auto &Entry : HeaderSearchOpts.UserEntries) if (Entry.IgnoreSysRoot) - PROPAGATE_TRUE_IF(Entry.Path); - PROPAGATE_TRUE_IF(HeaderSearchOpts.ResourceDir); - PROPAGATE_TRUE_IF(HeaderSearchOpts.ModuleCachePath); - PROPAGATE_TRUE_IF(HeaderSearchOpts.ModuleUserBuildPath); - for (const auto &[Name, File] : HeaderSearchOpts.PrebuiltModuleFiles) - PROPAGATE_TRUE_IF(File); - PROPAGATE_TRUE_IF_MANY(HeaderSearchOpts.PrebuiltModulePaths); - PROPAGATE_TRUE_IF_MANY(HeaderSearchOpts.VFSOverlayFiles); + 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); // Preprocessor options. - const auto &PPOpts = getPreprocessorOpts(); - PROPAGATE_TRUE_IF_MANY(PPOpts.MacroIncludes); - PROPAGATE_TRUE_IF_MANY(PPOpts.Includes); - PROPAGATE_TRUE_IF(PPOpts.ImplicitPCHInclude); + auto &PPOpts = *this->PPOpts; + RETURN_IF_MANY(PPOpts.MacroIncludes); + RETURN_IF_MANY(PPOpts.Includes); + RETURN_IF(PPOpts.ImplicitPCHInclude); // Frontend options. - const auto &FrontendOpts = getFrontendOpts(); - for (const FrontendInputFile &Input : FrontendOpts.Inputs) { + auto &FrontendOpts = *this->FrontendOpts; + for (auto &Input : FrontendOpts.Inputs) { if (Input.isBuffer()) - continue; // FIXME: Can this happen when parsing command-line? + continue; - PROPAGATE_TRUE_IF(Input.getFile()); + RETURN_IF(Input.File); } - PROPAGATE_TRUE_IF(FrontendOpts.CodeCompletionAt.FileName); - PROPAGATE_TRUE_IF_MANY(FrontendOpts.ModuleMapFiles); - PROPAGATE_TRUE_IF_MANY(FrontendOpts.ModuleFiles); - PROPAGATE_TRUE_IF_MANY(FrontendOpts.ModulesEmbedFiles); - PROPAGATE_TRUE_IF_MANY(FrontendOpts.ASTMergeFiles); - PROPAGATE_TRUE_IF(FrontendOpts.OverrideRecordLayoutsFile); - PROPAGATE_TRUE_IF(FrontendOpts.StatsFile); + 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); // Filesystem options. - const auto &FileSystemOpts = getFileSystemOpts(); - PROPAGATE_TRUE_IF(FileSystemOpts.WorkingDir); + auto &FileSystemOpts = *this->FSOpts; + RETURN_IF(FileSystemOpts.WorkingDir); // Codegen options. - const auto &CodeGenOpts = getCodeGenOpts(); - PROPAGATE_TRUE_IF(CodeGenOpts.DebugCompilationDir); - PROPAGATE_TRUE_IF(CodeGenOpts.CoverageCompilationDir); + auto &CodeGenOpts = *this->CodeGenOpts; + RETURN_IF(CodeGenOpts.DebugCompilationDir); + RETURN_IF(CodeGenOpts.CoverageCompilationDir); // Sanitizer options. - PROPAGATE_TRUE_IF_MANY(getLangOpts().NoSanitizeFiles); + RETURN_IF_MANY(LangOpts->NoSanitizeFiles); // Coverage mappings. - PROPAGATE_TRUE_IF(CodeGenOpts.ProfileInstrumentUsePath); - PROPAGATE_TRUE_IF(CodeGenOpts.SampleProfileFile); - PROPAGATE_TRUE_IF(CodeGenOpts.ProfileRemappingFile); + RETURN_IF(CodeGenOpts.ProfileInstrumentUsePath); + RETURN_IF(CodeGenOpts.SampleProfileFile); + RETURN_IF(CodeGenOpts.ProfileRemappingFile); // Dependency output options. - for (auto &ExtraDep : getDependencyOutputOpts().ExtraDeps) - PROPAGATE_TRUE_IF(ExtraDep.first); + for (auto &ExtraDep : DependencyOutputOpts->ExtraDeps) + RETURN_IF(ExtraDep.first); +} - return false; +void CompilerInvocationBase::visitPaths( + llvm::function_ref 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(this)->visitPathsImpl( + [&Callback](std::string &Path) { return Callback(StringRef(Path)); }); } void CompilerInvocationBase::generateCC1CommandLine( diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index abf18770a01dc..0022597348a82 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -471,9 +471,13 @@ static bool isSafeToIgnoreCWD(const CowCompilerInvocation &CI) { // Check if the command line input uses relative paths. // It is not safe to ignore the current working directory if any of the // command line inputs use relative paths. - return !CI.anyPath([](StringRef Path) { - return !Path.empty() && !llvm::sys::path::is_absolute(Path); + bool AnyRelative = false; + CI.visitPaths([&](StringRef Path) { + assert(!AnyRelative && "Continuing path visitation despite returning true"); + AnyRelative |= !Path.empty() && !llvm::sys::path::is_absolute(Path); + return AnyRelative; }); + return !AnyRelative; } static std::string getModuleContextHash(const ModuleDeps &MD, From 5aef684f57163b656f0d41103efb19c37c09b937 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Wed, 12 Nov 2025 11:48:25 -0800 Subject: [PATCH 4/4] Private -> protected --- clang/include/clang/Frontend/CompilerInvocation.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Frontend/CompilerInvocation.h b/clang/include/clang/Frontend/CompilerInvocation.h index 9f447b4cd630f..51787d914e1ec 100644 --- a/clang/include/clang/Frontend/CompilerInvocation.h +++ b/clang/include/clang/Frontend/CompilerInvocation.h @@ -188,12 +188,13 @@ class CompilerInvocationBase { /// This is a (less-efficient) wrapper over generateCC1CommandLine(). std::vector getCC1CommandLine() const; -private: +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 Predicate); +private: /// Generate command line options from DiagnosticOptions. static void GenerateDiagnosticArgs(const DiagnosticOptions &Opts, ArgumentConsumer Consumer,