Skip to content

Conversation

@jansvoboda11
Copy link
Contributor

@jansvoboda11 jansvoboda11 commented Nov 10, 2025

This PR extracts visitation of paths stored in CompilerInvocation into a member function. We already have a second copy of this downstream, and I'm in the need of adding a third one.

@jansvoboda11 jansvoboda11 changed the title [clang] Extract CompilerInvocationBase::visitPaths() [clang] Extract CompilerInvocation::visitPaths() Nov 12, 2025
@jansvoboda11 jansvoboda11 marked this pull request as ready for review November 12, 2025 19:50
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Nov 12, 2025
@llvmbot
Copy link
Member

llvmbot commented Nov 12, 2025

@llvm/pr-subscribers-clang

Author: Jan Svoboda (jansvoboda11)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/167420.diff

4 Files Affected:

  • (modified) clang/include/clang/Frontend/CompilerInvocation.h (+13)
  • (modified) clang/include/clang/Frontend/FrontendOptions.h (+2)
  • (modified) clang/lib/Frontend/CompilerInvocation.cpp (+80)
  • (modified) clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp (+7-76)
diff --git a/clang/include/clang/Frontend/CompilerInvocation.h b/clang/include/clang/Frontend/CompilerInvocation.h
index e147d2ba6087e..51787d914e1ec 100644
--- a/clang/include/clang/Frontend/CompilerInvocation.h
+++ b/clang/include/clang/Frontend/CompilerInvocation.h
@@ -147,6 +147,13 @@ class CompilerInvocationBase {
   }
   /// @}
 
+  /// 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 &)>;
@@ -181,6 +188,12 @@ 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,
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 f584a2a5824b2..a95796924311b 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -5280,6 +5280,86 @@ std::string CompilerInvocation::getModuleHash() const {
   return toString(llvm::APInt(64, Hash), 36, /*Signed=*/false);
 }
 
+void CompilerInvocationBase::visitPathsImpl(
+    llvm::function_ref<bool(std::string &)> Predicate) {
+#define RETURN_IF(PATH)                                                        \
+  do {                                                                         \
+    if (Predicate(PATH))                                                       \
+      return;                                                                  \
+  } while (0)
+
+#define RETURN_IF_MANY(PATHS)                                                  \
+  do {                                                                         \
+    if (llvm::any_of(PATHS, Predicate))                                        \
+      return;                                                                  \
+  } while (0)
+
+  auto &HeaderSearchOpts = *this->HSOpts;
+  // Header search paths.
+  RETURN_IF(HeaderSearchOpts.Sysroot);
+  for (auto &Entry : HeaderSearchOpts.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);
+
+  // Preprocessor options.
+  auto &PPOpts = *this->PPOpts;
+  RETURN_IF_MANY(PPOpts.MacroIncludes);
+  RETURN_IF_MANY(PPOpts.Includes);
+  RETURN_IF(PPOpts.ImplicitPCHInclude);
+
+  // Frontend options.
+  auto &FrontendOpts = *this->FrontendOpts;
+  for (auto &Input : FrontendOpts.Inputs) {
+    if (Input.isBuffer())
+      continue;
+
+    RETURN_IF(Input.File);
+  }
+  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.
+  auto &FileSystemOpts = *this->FSOpts;
+  RETURN_IF(FileSystemOpts.WorkingDir);
+
+  // Codegen options.
+  auto &CodeGenOpts = *this->CodeGenOpts;
+  RETURN_IF(CodeGenOpts.DebugCompilationDir);
+  RETURN_IF(CodeGenOpts.CoverageCompilationDir);
+
+  // Sanitizer options.
+  RETURN_IF_MANY(LangOpts->NoSanitizeFiles);
+
+  // Coverage mappings.
+  RETURN_IF(CodeGenOpts.ProfileInstrumentUsePath);
+  RETURN_IF(CodeGenOpts.SampleProfileFile);
+  RETURN_IF(CodeGenOpts.ProfileRemappingFile);
+
+  // Dependency output options.
+  for (auto &ExtraDep : DependencyOutputOpts->ExtraDeps)
+    RETURN_IF(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 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..0022597348a82 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -471,82 +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.
-#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;
+  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,

Copy link
Contributor

@qiongsiwu qiongsiwu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM with two questions.

@jansvoboda11 jansvoboda11 merged commit 71763a5 into llvm:main Nov 12, 2025
13 checks passed
@jansvoboda11 jansvoboda11 deleted the ci-path-visitation branch November 12, 2025 22:20
git-crd pushed a commit to git-crd/crd-llvm-project that referenced this pull request Nov 13, 2025
This PR extracts visitation of paths stored in `CompilerInvocation` into
a member function. We already have a second copy of this downstream, and
I'm in the need of adding a third one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants