Skip to content

Commit 9d28fcc

Browse files
jansvoboda11mahesh-attarde
authored andcommitted
[llvm][clang] Use the VFS in FileCollector (llvm#160788)
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.
1 parent e80e49f commit 9d28fcc

File tree

7 files changed

+36
-20
lines changed

7 files changed

+36
-20
lines changed

clang/include/clang/Frontend/Utils.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,9 @@ class ModuleDependencyCollector : public DependencyCollector {
143143
std::error_code copyToRoot(StringRef Src, StringRef Dst = {});
144144

145145
public:
146-
ModuleDependencyCollector(std::string DestDir)
147-
: DestDir(std::move(DestDir)) {}
146+
ModuleDependencyCollector(std::string DestDir,
147+
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS)
148+
: DestDir(std::move(DestDir)), Canonicalizer(std::move(VFS)) {}
148149
~ModuleDependencyCollector() override { writeFileMap(); }
149150

150151
StringRef getDest() { return DestDir; }

clang/lib/Frontend/CompilerInstance.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
503503
// then we're the top level compiler instance and need to create one.
504504
if (!ModuleDepCollector && !DepOpts.ModuleDependencyOutputDir.empty()) {
505505
ModuleDepCollector = std::make_shared<ModuleDependencyCollector>(
506-
DepOpts.ModuleDependencyOutputDir);
506+
DepOpts.ModuleDependencyOutputDir, getVirtualFileSystemPtr());
507507
}
508508

509509
// If there is a module dep collector, register with other dep collectors

lldb/source/Plugins/ExpressionParser/Clang/ModuleDependencyCollector.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ class ModuleDependencyCollectorAdaptor
1919
public:
2020
ModuleDependencyCollectorAdaptor(
2121
std::shared_ptr<llvm::FileCollectorBase> file_collector)
22-
: clang::ModuleDependencyCollector(""), m_file_collector(file_collector) {
23-
}
22+
: clang::ModuleDependencyCollector("", llvm::vfs::getRealFileSystem()),
23+
m_file_collector(file_collector) {}
2424

2525
void addFile(llvm::StringRef Filename,
2626
llvm::StringRef FileDst = {}) override {

llvm/include/llvm/Support/FileCollector.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,19 +81,25 @@ class LLVM_ABI FileCollector : public FileCollectorBase {
8181
/// Canonicalize a pair of virtual and real paths.
8282
LLVM_ABI PathStorage canonicalize(StringRef SrcPath);
8383

84+
explicit PathCanonicalizer(IntrusiveRefCntPtr<vfs::FileSystem> VFS)
85+
: VFS(std::move(VFS)) {}
86+
8487
private:
8588
/// Replace with a (mostly) real path, or don't modify. Resolves symlinks
8689
/// in the directory, using \a CachedDirs to avoid redundant lookups, but
8790
/// leaves the filename as a possible symlink.
8891
void updateWithRealPath(SmallVectorImpl<char> &Path);
8992

93+
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS;
94+
9095
StringMap<std::string> CachedDirs;
9196
};
9297

9398
/// \p Root is the directory where collected files are will be stored.
9499
/// \p OverlayRoot is VFS mapping root.
95100
/// \p Root directory gets created in copyFiles unless it already exists.
96-
FileCollector(std::string Root, std::string OverlayRoot);
101+
FileCollector(std::string Root, std::string OverlayRoot,
102+
IntrusiveRefCntPtr<vfs::FileSystem> VFS);
97103

98104
/// Write the yaml mapping (for the VFS) to the given file.
99105
std::error_code writeMapping(StringRef MappingFile);

llvm/lib/Support/FileCollector.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,9 @@ static bool isCaseSensitivePath(StringRef Path) {
4949
return true;
5050
}
5151

52-
FileCollector::FileCollector(std::string Root, std::string OverlayRoot)
53-
: Root(Root), OverlayRoot(OverlayRoot) {
52+
FileCollector::FileCollector(std::string Root, std::string OverlayRoot,
53+
IntrusiveRefCntPtr<vfs::FileSystem> VFS)
54+
: Root(Root), OverlayRoot(OverlayRoot), Canonicalizer(std::move(VFS)) {
5455
assert(sys::path::is_absolute(Root) && "Root not absolute");
5556
assert(sys::path::is_absolute(OverlayRoot) && "OverlayRoot not absolute");
5657
}
@@ -88,9 +89,9 @@ void FileCollector::PathCanonicalizer::updateWithRealPath(
8889
}
8990

9091
/// Make Path absolute.
91-
static void makeAbsolute(SmallVectorImpl<char> &Path) {
92+
static void makeAbsolute(vfs::FileSystem &VFS, SmallVectorImpl<char> &Path) {
9293
// We need an absolute src path to append to the root.
93-
sys::fs::make_absolute(Path);
94+
VFS.makeAbsolute(Path);
9495

9596
// Canonicalize src to a native path to avoid mixed separator styles.
9697
sys::path::native(Path);
@@ -105,7 +106,7 @@ FileCollector::PathCanonicalizer::PathStorage
105106
FileCollector::PathCanonicalizer::canonicalize(StringRef SrcPath) {
106107
PathStorage Paths;
107108
Paths.VirtualPath = SrcPath;
108-
makeAbsolute(Paths.VirtualPath);
109+
makeAbsolute(*VFS, Paths.VirtualPath);
109110

110111
// If a ".." component is present after a symlink component, remove_dots may
111112
// lead to the wrong real destination path. Let the source be canonicalized

llvm/tools/dsymutil/Reproducer.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,10 @@ ReproducerGenerate::ReproducerGenerate(std::error_code &EC, int Argc,
3737
char **Argv, bool GenerateOnExit)
3838
: Root(createReproducerDir(EC)), GenerateOnExit(GenerateOnExit) {
3939
llvm::append_range(Args, ArrayRef(Argv, Argc));
40+
auto RealFS = vfs::getRealFileSystem();
4041
if (!Root.empty())
41-
FC = std::make_shared<FileCollector>(Root, Root);
42-
VFS = FileCollector::createCollectorVFS(vfs::getRealFileSystem(), FC);
42+
FC = std::make_shared<FileCollector>(Root, Root, RealFS);
43+
VFS = FileCollector::createCollectorVFS(std::move(RealFS), FC);
4344
}
4445

4546
ReproducerGenerate::~ReproducerGenerate() {

llvm/unittests/Support/FileCollectorTest.cpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ class TestingFileCollector : public FileCollector {
4343
TEST(FileCollectorTest, addFile) {
4444
TempDir root("add_file_root", /*Unique*/ true);
4545
std::string root_fs(root.path());
46-
TestingFileCollector FileCollector(root_fs, root_fs);
46+
TestingFileCollector FileCollector(root_fs, root_fs,
47+
vfs::getRealFileSystem());
4748

4849
FileCollector.addFile("/path/to/a");
4950
FileCollector.addFile("/path/to/b");
@@ -77,7 +78,8 @@ TEST(FileCollectorTest, addDirectory) {
7778
TempFile c(ccc.str());
7879

7980
std::string root_fs(file_root.path());
80-
TestingFileCollector FileCollector(root_fs, root_fs);
81+
TestingFileCollector FileCollector(root_fs, root_fs,
82+
vfs::getRealFileSystem());
8183

8284
FileCollector.addDirectory(file_root.path());
8385

@@ -105,7 +107,8 @@ TEST(FileCollectorTest, copyFiles) {
105107
// Create file collector and add files.
106108
TempDir root("copy_files_root", /*Unique*/ true);
107109
std::string root_fs(root.path());
108-
TestingFileCollector FileCollector(root_fs, root_fs);
110+
TestingFileCollector FileCollector(root_fs, root_fs,
111+
vfs::getRealFileSystem());
109112
FileCollector.addFile(a.path());
110113
FileCollector.addFile(b.path());
111114
FileCollector.addFile(c.path());
@@ -133,7 +136,8 @@ TEST(FileCollectorTest, recordAndConstructDirectory) {
133136
// Create file collector and add files.
134137
TempDir root("copy_files_root", /*Unique*/ true);
135138
std::string root_fs(root.path());
136-
TestingFileCollector FileCollector(root_fs, root_fs);
139+
TestingFileCollector FileCollector(root_fs, root_fs,
140+
vfs::getRealFileSystem());
137141
FileCollector.addFile(a.path());
138142

139143
// The empty directory isn't seen until we add it.
@@ -169,7 +173,8 @@ TEST(FileCollectorTest, recordVFSAccesses) {
169173
// Create file collector and add files.
170174
TempDir root("copy_files_root", /*Unique*/ true);
171175
std::string root_fs(root.path());
172-
auto Collector = std::make_shared<TestingFileCollector>(root_fs, root_fs);
176+
auto Collector = std::make_shared<TestingFileCollector>(
177+
root_fs, root_fs, vfs::getRealFileSystem());
173178
auto VFS =
174179
FileCollector::createCollectorVFS(vfs::getRealFileSystem(), Collector);
175180
VFS->status(a.path());
@@ -216,7 +221,8 @@ TEST(FileCollectorTest, Symlinks) {
216221
// Root where files are copied to.
217222
TempDir reproducer_root("reproducer_root", /*Unique*/ true);
218223
std::string root_fs(reproducer_root.path());
219-
TestingFileCollector FileCollector(root_fs, root_fs);
224+
TestingFileCollector FileCollector(root_fs, root_fs,
225+
vfs::getRealFileSystem());
220226

221227
// Add all the files to the collector.
222228
FileCollector.addFile(a.path());
@@ -264,7 +270,8 @@ TEST(FileCollectorTest, recordVFSSymlinkAccesses) {
264270
// Create file collector and add files.
265271
TempDir root("copy_files_root", true);
266272
std::string root_fs(root.path());
267-
auto Collector = std::make_shared<TestingFileCollector>(root_fs, root_fs);
273+
auto Collector = std::make_shared<TestingFileCollector>(
274+
root_fs, root_fs, vfs::getRealFileSystem());
268275
auto VFS =
269276
FileCollector::createCollectorVFS(vfs::getRealFileSystem(), Collector);
270277
SmallString<256> Output;

0 commit comments

Comments
 (0)