Skip to content

Conversation

@jansvoboda11
Copy link
Contributor

This PR changes llvm::FileCollector to use the llvm::vfs::FileSystem API for making file paths absolute instead of using llvm::sys::fs::make_absolute() directly. This matches the behavior of the compiler on most other input files.

@llvmbot llvmbot added clang Clang issues not falling into any other category lldb llvm:support labels Sep 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 25, 2025

@llvm/pr-subscribers-debuginfo
@llvm/pr-subscribers-lldb

@llvm/pr-subscribers-clang

Author: Jan Svoboda (jansvoboda11)

Changes

This PR changes llvm::FileCollector to use the llvm::vfs::FileSystem API for making file paths absolute instead of using llvm::sys::fs::make_absolute() directly. This matches the behavior of the compiler on most other input files.


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

5 Files Affected:

  • (modified) clang/include/clang/Frontend/Utils.h (+3-2)
  • (modified) clang/lib/Frontend/CompilerInstance.cpp (+1-1)
  • (modified) lldb/source/Plugins/ExpressionParser/Clang/ModuleDependencyCollector.h (+2-2)
  • (modified) llvm/include/llvm/Support/FileCollector.h (+7-1)
  • (modified) llvm/lib/Support/FileCollector.cpp (+6-5)
diff --git a/clang/include/clang/Frontend/Utils.h b/clang/include/clang/Frontend/Utils.h
index f86c2f5074de0..49fd920d1ec43 100644
--- a/clang/include/clang/Frontend/Utils.h
+++ b/clang/include/clang/Frontend/Utils.h
@@ -143,8 +143,9 @@ class ModuleDependencyCollector : public DependencyCollector {
   std::error_code copyToRoot(StringRef Src, StringRef Dst = {});
 
 public:
-  ModuleDependencyCollector(std::string DestDir)
-      : DestDir(std::move(DestDir)) {}
+  ModuleDependencyCollector(std::string DestDir,
+                            IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS)
+      : DestDir(std::move(DestDir)), Canonicalizer(std::move(VFS)) {}
   ~ModuleDependencyCollector() override { writeFileMap(); }
 
   StringRef getDest() { return DestDir; }
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index d6f3aec981336..c989ad2e5155c 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -503,7 +503,7 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
   // then we're the top level compiler instance and need to create one.
   if (!ModuleDepCollector && !DepOpts.ModuleDependencyOutputDir.empty()) {
     ModuleDepCollector = std::make_shared<ModuleDependencyCollector>(
-        DepOpts.ModuleDependencyOutputDir);
+        DepOpts.ModuleDependencyOutputDir, getVirtualFileSystemPtr());
   }
 
   // If there is a module dep collector, register with other dep collectors
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ModuleDependencyCollector.h b/lldb/source/Plugins/ExpressionParser/Clang/ModuleDependencyCollector.h
index 4fe727460fdb9..dcba0d9c34962 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ModuleDependencyCollector.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ModuleDependencyCollector.h
@@ -19,8 +19,8 @@ class ModuleDependencyCollectorAdaptor
 public:
   ModuleDependencyCollectorAdaptor(
       std::shared_ptr<llvm::FileCollectorBase> file_collector)
-      : clang::ModuleDependencyCollector(""), m_file_collector(file_collector) {
-  }
+      : clang::ModuleDependencyCollector("", llvm::vfs::getRealFileSystem()),
+        m_file_collector(file_collector) {}
 
   void addFile(llvm::StringRef Filename,
                llvm::StringRef FileDst = {}) override {
diff --git a/llvm/include/llvm/Support/FileCollector.h b/llvm/include/llvm/Support/FileCollector.h
index b00bf3174e654..9cc6776b948ba 100644
--- a/llvm/include/llvm/Support/FileCollector.h
+++ b/llvm/include/llvm/Support/FileCollector.h
@@ -81,19 +81,25 @@ class LLVM_ABI FileCollector : public FileCollectorBase {
     /// Canonicalize a pair of virtual and real paths.
     LLVM_ABI PathStorage canonicalize(StringRef SrcPath);
 
+    explicit PathCanonicalizer(IntrusiveRefCntPtr<vfs::FileSystem> VFS)
+        : VFS(std::move(VFS)) {}
+
   private:
     /// Replace with a (mostly) real path, or don't modify. Resolves symlinks
     /// in the directory, using \a CachedDirs to avoid redundant lookups, but
     /// leaves the filename as a possible symlink.
     void updateWithRealPath(SmallVectorImpl<char> &Path);
 
+    IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS;
+
     StringMap<std::string> CachedDirs;
   };
 
   /// \p Root is the directory where collected files are will be stored.
   /// \p OverlayRoot is VFS mapping root.
   /// \p Root directory gets created in copyFiles unless it already exists.
-  FileCollector(std::string Root, std::string OverlayRoot);
+  FileCollector(std::string Root, std::string OverlayRoot,
+                IntrusiveRefCntPtr<vfs::FileSystem> VFS);
 
   /// Write the yaml mapping (for the VFS) to the given file.
   std::error_code writeMapping(StringRef MappingFile);
diff --git a/llvm/lib/Support/FileCollector.cpp b/llvm/lib/Support/FileCollector.cpp
index edb5313d43eec..5dc224a6d427b 100644
--- a/llvm/lib/Support/FileCollector.cpp
+++ b/llvm/lib/Support/FileCollector.cpp
@@ -49,8 +49,9 @@ static bool isCaseSensitivePath(StringRef Path) {
   return true;
 }
 
-FileCollector::FileCollector(std::string Root, std::string OverlayRoot)
-    : Root(Root), OverlayRoot(OverlayRoot) {
+FileCollector::FileCollector(std::string Root, std::string OverlayRoot,
+                             IntrusiveRefCntPtr<vfs::FileSystem> VFS)
+    : Root(Root), OverlayRoot(OverlayRoot), Canonicalizer(std::move(VFS)) {
   assert(sys::path::is_absolute(Root) && "Root not absolute");
   assert(sys::path::is_absolute(OverlayRoot) && "OverlayRoot not absolute");
 }
@@ -88,9 +89,9 @@ void FileCollector::PathCanonicalizer::updateWithRealPath(
 }
 
 /// Make Path absolute.
-static void makeAbsolute(SmallVectorImpl<char> &Path) {
+static void makeAbsolute(vfs::FileSystem &VFS, SmallVectorImpl<char> &Path) {
   // We need an absolute src path to append to the root.
-  sys::fs::make_absolute(Path);
+  VFS.makeAbsolute(Path);
 
   // Canonicalize src to a native path to avoid mixed separator styles.
   sys::path::native(Path);
@@ -105,7 +106,7 @@ FileCollector::PathCanonicalizer::PathStorage
 FileCollector::PathCanonicalizer::canonicalize(StringRef SrcPath) {
   PathStorage Paths;
   Paths.VirtualPath = SrcPath;
-  makeAbsolute(Paths.VirtualPath);
+  makeAbsolute(*VFS, Paths.VirtualPath);
 
   // If a ".." component is present after a symlink component, remove_dots may
   // lead to the wrong real destination path. Let the source be canonicalized

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

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

Makes sense. LGTM

@jansvoboda11 jansvoboda11 merged commit 0e3c316 into llvm:main Sep 26, 2025
9 checks passed
@jansvoboda11 jansvoboda11 deleted the file-collector-vfs branch September 26, 2025 15:17
@llvm-ci
Copy link
Collaborator

llvm-ci commented Sep 26, 2025

LLVM Buildbot has detected a new failure on builder clang-m68k-linux-cross running on suse-gary-m68k-cross while building clang,lldb,llvm at step 5 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/16650

Here is the relevant piece of the build log for the reference
Step 5 (ninja check 1) failure: stage 1 checked (failure)
...
[113/392] Building CXX object tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/TUSchedulerTests.cpp.o
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp: In member function ‘virtual void clang::clangd::{anonymous}::TUSchedulerTests_PublishWithStalePreamble_Test::TestBody()::BlockPreambleThread::onPreambleAST(clang::clangd::PathRef, llvm::StringRef, clang::clangd::CapturedASTCtx, std::shared_ptr<const clang::include_cleaner::PragmaIncludes>)’:
/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:1219:10: warning: suggest explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
 1219 |       if (BuildBefore)
      |          ^
[114/392] Building CXX object tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/tweaks/ExpandDeducedTypeTests.cpp.o
[115/392] Building CXX object tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/tweaks/ExtractVariableTests.cpp.o
[116/392] Building CXX object tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/tweaks/AddUsingTests.cpp.o
[117/392] Building CXX object tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/tweaks/TweakTesting.cpp.o
[118/392] Building CXX object tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/CodeCompleteTests.cpp.o
FAILED: tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/CodeCompleteTests.cpp.o 
/usr/bin/c++ -DGTEST_HAS_RTTI=0 -DLLVM_BUILD_STATIC -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GLIBCXX_USE_CXX11_ABI=1 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/tools/clang/tools/extra/clangd/unittests -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang-tools-extra/clangd/unittests -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang-tools-extra/clangd/../include-cleaner/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/tools/clang/tools/extra/clangd/../clang-tidy -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/tools/clang/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/llvm/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang-tools-extra/clangd -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/stage1/tools/clang/tools/extra/clangd -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/third-party/unittest/googletest/include -I/var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/third-party/unittest/googlemock/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-uninitialized -Wno-nonnull -Wno-class-memaccess -Wno-dangling-reference -Wno-redundant-move -Wno-pessimizing-move -Wno-array-bounds -Wno-stringop-overread -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wno-misleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -O3 -DNDEBUG -std=c++17  -Wno-variadic-macros -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -Wno-suggest-override -MD -MT tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/CodeCompleteTests.cpp.o -MF tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/CodeCompleteTests.cpp.o.d -o tools/clang/tools/extra/clangd/unittests/CMakeFiles/ClangdTests.dir/CodeCompleteTests.cpp.o -c /var/lib/buildbot/workers/suse-gary-m68k-cross/clang-m68k-linux-cross/llvm/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
c++: fatal error: Killed signal terminated program cc1plus
compilation terminated.
[119/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/DeterminismTest.cpp.o
[120/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/ConflictingEvalCallsTest.cpp.o
[121/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/CFGTest.cpp.o
[122/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/APSIntTypeTest.cpp.o
[123/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/SmartPointerAccessorCachingTest.cpp.o
[124/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/DebugSupportTest.cpp.o
[125/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/SimplifyConstraintsTest.cpp.o
[126/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/IntervalPartitionTest.cpp.o
[127/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/CallEventTest.cpp.o
[128/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/TestingSupport.cpp.o
[129/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/BlockEntranceCallbackTest.cpp.o
[130/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/TransferBranchTest.cpp.o
[131/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/ExprEngineVisitTest.cpp.o
[132/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp.o
[133/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/LifetimeSafetyTest.cpp.o
[134/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/MatchSwitchTest.cpp.o
[135/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp.o
[136/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp.o
[137/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp.o
[138/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp.o
[139/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/WatchedLiteralsSolverTest.cpp.o
[140/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/TestingSupportTest.cpp.o
[141/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/RecordOpsTest.cpp.o
[142/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/ExprMutationAnalyzerTest.cpp.o
[143/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/ASTOpsTest.cpp.o
[144/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/LoggerTest.cpp.o
[145/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/CachedConstAccessorsLatticeTest.cpp.o
[146/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp.o
[147/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/SignAnalysisTest.cpp.o
[148/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/BugReportInterestingnessTest.cpp.o
[149/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/StaticAnalyzer/CallDescriptionTest.cpp.o
[150/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp.o
[151/392] Building CXX object tools/clang/unittests/CMakeFiles/AllClangUnitTests.dir/Analysis/FlowSensitive/TransferTest.cpp.o
ninja: build stopped: subcommand failed.

mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
This PR changes `llvm::FileCollector` to use the `llvm::vfs::FileSystem`
API for making file paths absolute instead of using
`llvm::sys::fs::make_absolute()` directly. This matches the behavior of
the compiler on most other input files.
jansvoboda11 added a commit to swiftlang/llvm-project that referenced this pull request Oct 18, 2025
This PR changes `llvm::FileCollector` to use the `llvm::vfs::FileSystem`
API for making file paths absolute instead of using
`llvm::sys::fs::make_absolute()` directly. This matches the behavior of
the compiler on most other input files.
jansvoboda11 added a commit to swiftlang/llvm-project that referenced this pull request Oct 23, 2025
This PR changes `llvm::FileCollector` to use the `llvm::vfs::FileSystem`
API for making file paths absolute instead of using
`llvm::sys::fs::make_absolute()` directly. This matches the behavior of
the compiler on most other input files.
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 debuginfo lldb llvm:support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants