benlangmuir created this revision.
benlangmuir added reviewers: bnbarham, jansvoboda11.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
As progress towards having FileEntryRef contain the requested name of the file,
this commit narrows the "remap" hack to only apply to paths that were remapped
to an external contents path by a VFS. That was always the original intent of
this code, and the fact it was making relative paths absolute was an unintended
side effect.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D130935
Files:
clang/lib/Basic/FileManager.cpp
clang/unittests/Basic/FileManagerTest.cpp
Index: clang/unittests/Basic/FileManagerTest.cpp
===================================================================
--- clang/unittests/Basic/FileManagerTest.cpp
+++ clang/unittests/Basic/FileManagerTest.cpp
@@ -44,16 +44,16 @@
}
}
- if (!StatPath)
- StatPath = Path;
-
auto fileType = IsFile ?
llvm::sys::fs::file_type::regular_file :
llvm::sys::fs::file_type::directory_file;
- llvm::vfs::Status Status(StatPath, llvm::sys::fs::UniqueID(1, INode),
+ llvm::vfs::Status Status(StatPath ? StatPath : Path,
+ llvm::sys::fs::UniqueID(1, INode),
/*MTime*/{}, /*User*/0, /*Group*/0,
/*Size*/0, fileType,
llvm::sys::fs::perms::all_all);
+ if (StatPath)
+ Status.ExposesExternalVFSPath = true;
StatCalls[Path] = Status;
}
Index: clang/lib/Basic/FileManager.cpp
===================================================================
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -274,8 +274,8 @@
if (!UFE)
UFE = new (FilesAlloc.Allocate()) FileEntry();
- if (Status.getName() == Filename) {
- // The name matches. Set the FileEntry.
+ if (Status.getName() == Filename || !Status.ExposesExternalVFSPath) {
+ // Use the requested name. Set the FileEntry.
NamedFileEnt->second = FileEntryRef::MapValue(*UFE, DirInfo);
} else {
// Name mismatch. We need a redirect. First grab the actual entry we want
@@ -292,19 +292,7 @@
// filesystems behave and confuses parts of clang expect to see the
// name-as-accessed on the \a FileEntryRef.
//
- // Further, it isn't *just* external names, but will also give back
absolute
- // paths when a relative path was requested - the check is comparing the
- // name from the status, which is passed an absolute path resolved from the
- // current working directory. `clang-apply-replacements` appears to depend
- // on this behaviour, though it's adjusting the working directory, which is
- // definitely not supported. Once that's fixed this hack should be able to
- // be narrowed to only when there's an externally mapped name given back.
- //
// A potential plan to remove this is as follows -
- // - Add API to determine if the name has been rewritten by the VFS.
- // - Fix `clang-apply-replacements` to pass down the absolute path rather
- // than changing the CWD. Narrow this hack down to just externally
- // mapped paths.
// - Expose the requested filename. One possibility would be to allow
// redirection-FileEntryRefs to be returned, rather than returning
// the pointed-at-FileEntryRef, and customizing `getName()` to look
Index: clang/unittests/Basic/FileManagerTest.cpp
===================================================================
--- clang/unittests/Basic/FileManagerTest.cpp
+++ clang/unittests/Basic/FileManagerTest.cpp
@@ -44,16 +44,16 @@
}
}
- if (!StatPath)
- StatPath = Path;
-
auto fileType = IsFile ?
llvm::sys::fs::file_type::regular_file :
llvm::sys::fs::file_type::directory_file;
- llvm::vfs::Status Status(StatPath, llvm::sys::fs::UniqueID(1, INode),
+ llvm::vfs::Status Status(StatPath ? StatPath : Path,
+ llvm::sys::fs::UniqueID(1, INode),
/*MTime*/{}, /*User*/0, /*Group*/0,
/*Size*/0, fileType,
llvm::sys::fs::perms::all_all);
+ if (StatPath)
+ Status.ExposesExternalVFSPath = true;
StatCalls[Path] = Status;
}
Index: clang/lib/Basic/FileManager.cpp
===================================================================
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -274,8 +274,8 @@
if (!UFE)
UFE = new (FilesAlloc.Allocate()) FileEntry();
- if (Status.getName() == Filename) {
- // The name matches. Set the FileEntry.
+ if (Status.getName() == Filename || !Status.ExposesExternalVFSPath) {
+ // Use the requested name. Set the FileEntry.
NamedFileEnt->second = FileEntryRef::MapValue(*UFE, DirInfo);
} else {
// Name mismatch. We need a redirect. First grab the actual entry we want
@@ -292,19 +292,7 @@
// filesystems behave and confuses parts of clang expect to see the
// name-as-accessed on the \a FileEntryRef.
//
- // Further, it isn't *just* external names, but will also give back absolute
- // paths when a relative path was requested - the check is comparing the
- // name from the status, which is passed an absolute path resolved from the
- // current working directory. `clang-apply-replacements` appears to depend
- // on this behaviour, though it's adjusting the working directory, which is
- // definitely not supported. Once that's fixed this hack should be able to
- // be narrowed to only when there's an externally mapped name given back.
- //
// A potential plan to remove this is as follows -
- // - Add API to determine if the name has been rewritten by the VFS.
- // - Fix `clang-apply-replacements` to pass down the absolute path rather
- // than changing the CWD. Narrow this hack down to just externally
- // mapped paths.
// - Expose the requested filename. One possibility would be to allow
// redirection-FileEntryRefs to be returned, rather than returning
// the pointed-at-FileEntryRef, and customizing `getName()` to look
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
