jansvoboda11 updated this revision to Diff 536224.
jansvoboda11 added a comment.
Return canonical virtual path from `RedirectingFileSystem`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135849/new/
https://reviews.llvm.org/D135849
Files:
clang/lib/Lex/ModuleMap.cpp
clang/test/ClangScanDeps/modules-canon-mm-case-sensitive.c
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp
Index: llvm/lib/Support/VirtualFileSystem.cpp
===================================================================
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -2239,6 +2239,13 @@
}
}
+void RedirectingFileSystem::LookupResult::getPath(
+ llvm::SmallVectorImpl<char> &Result) const {
+ for (Entry *Parent : Parents)
+ llvm::sys::path::append(Result, Parent->getName());
+ llvm::sys::path::append(Result, E->getName());
+}
+
std::error_code
RedirectingFileSystem::makeCanonical(SmallVectorImpl<char> &Path) const {
if (std::error_code EC = makeAbsolute(Path))
@@ -2257,19 +2264,26 @@
RedirectingFileSystem::lookupPath(StringRef Path) const {
sys::path::const_iterator Start = sys::path::begin(Path);
sys::path::const_iterator End = sys::path::end(Path);
+ std::vector<Entry *> Entries;
+ Entries.reserve(std::distance(Start, End));
+
for (const auto &Root : Roots) {
+ Entries.clear();
ErrorOr<RedirectingFileSystem::LookupResult> Result =
- lookupPathImpl(Start, End, Root.get());
- if (Result || Result.getError() != llvm::errc::no_such_file_or_directory)
+ lookupPathImpl(Start, End, Root.get(), Entries);
+ if (Result || Result.getError() != llvm::errc::no_such_file_or_directory) {
+ Result->Parents = std::move(Entries);
return Result;
+ }
}
return make_error_code(llvm::errc::no_such_file_or_directory);
}
ErrorOr<RedirectingFileSystem::LookupResult>
-RedirectingFileSystem::lookupPathImpl(
- sys::path::const_iterator Start, sys::path::const_iterator End,
- RedirectingFileSystem::Entry *From) const {
+RedirectingFileSystem::lookupPathImpl(sys::path::const_iterator Start,
+ sys::path::const_iterator End,
+ RedirectingFileSystem::Entry *From,
+ std::vector<Entry *> &Entries) const {
assert(!isTraversalComponent(*Start) &&
!isTraversalComponent(From->getName()) &&
"Paths should not contain traversal components");
@@ -2295,11 +2309,14 @@
if (isa<RedirectingFileSystem::DirectoryRemapEntry>(From))
return LookupResult(From, Start, End);
+ unsigned EntriesSize = Entries.size();
auto *DE = cast<RedirectingFileSystem::DirectoryEntry>(From);
for (const std::unique_ptr<RedirectingFileSystem::Entry> &DirEntry :
llvm::make_range(DE->contents_begin(), DE->contents_end())) {
+ Entries.resize(EntriesSize);
+ Entries.push_back(From);
ErrorOr<RedirectingFileSystem::LookupResult> Result =
- lookupPathImpl(Start, End, DirEntry.get());
+ lookupPathImpl(Start, End, DirEntry.get(), Entries);
if (Result || Result.getError() != llvm::errc::no_such_file_or_directory)
return Result;
}
@@ -2543,10 +2560,12 @@
return P;
}
- // If we found a DirectoryEntry, still fallthrough to the original path if
- // allowed, because directories don't have a single external contents path.
- if (Redirection == RedirectKind::Fallthrough)
- return ExternalFS->getRealPath(CanonicalPath, Output);
+ // We found a DirectoryEntry, which does not have a single external contents
+ // path. Use the canonical virtual path.
+ if (Redirection == RedirectKind::Fallthrough) {
+ Result->getPath(Output);
+ return {};
+ }
return llvm::errc::invalid_argument;
}
Index: llvm/include/llvm/Support/VirtualFileSystem.h
===================================================================
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -872,6 +872,9 @@
/// Represents the result of a path lookup into the RedirectingFileSystem.
struct LookupResult {
+ /// Chain of parent directory entries for \c E.
+ std::vector<Entry *> Parents;
+
/// The entry the looked-up path corresponds to.
Entry *E;
@@ -895,6 +898,10 @@
return FE->getExternalContentsPath();
return std::nullopt;
}
+
+ /// Get the (canonical) path of the found entry. This uses the as-written
+ /// path components from the VFS specification.
+ void getPath(llvm::SmallVectorImpl<char> &Path) const;
};
private:
@@ -986,7 +993,8 @@
/// DirectoryRemapEntry, the path it redirects to in the external file system.
ErrorOr<LookupResult> lookupPathImpl(llvm::sys::path::const_iterator Start,
llvm::sys::path::const_iterator End,
- Entry *From) const;
+ Entry *From,
+ std::vector<Entry *> &Entries) const;
/// Get the status for a path with the provided \c LookupResult.
ErrorOr<Status> status(const Twine &CanonicalPath, const Twine &OriginalPath,
Index: clang/test/ClangScanDeps/modules-canon-mm-case-sensitive.c
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/modules-canon-mm-case-sensitive.c
@@ -0,0 +1,49 @@
+// RUN: sudo rm -rf %t
+// RUN: split-file %s %t
+
+//--- actual/One.h
+#import <FW/Two.h>
+//--- actual/Two.h
+// empty
+//--- frameworks/FW.framework/Modules/module.modulemap
+framework module FW {
+ header "One.h"
+ header "Two.h"
+}
+//--- tu.m
+#import <fw/One.h>
+
+//--- overlay.json.in
+{
+ "version": 0,
+ "case-sensitive": "false",
+ "roots": [
+ {
+ "contents": [
+ {
+ "external-contents": "DIR/actual/One.h",
+ "name": "One.h",
+ "type": "file"
+ },
+ {
+ "external-contents": "DIR/actual/Two.h",
+ "name": "Two.h",
+ "type": "file"
+ }
+ ],
+ "name": "DIR/frameworks/FW.framework/Headers",
+ "type": "directory"
+ }
+ ]
+}
+
+//--- cdb.json.in
+[{
+ "directory": "DIR",
+ "file": "DIR/tu.m",
+ "command": "clang -fmodules -fmodules-cache-path=DIR/cache -ivfsoverlay DIR/overlay.json -F DIR/frameworks -c DIR/tu.m -o DIR/tu.o"
+}]
+
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json
+// RUN: sed -e "s|DIR|%/t|g" %t/overlay.json.in > %t/overlay.json
+// RUN: sudo /usr/sbin/taskpolicy -x clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json
Index: clang/lib/Lex/ModuleMap.cpp
===================================================================
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1325,18 +1325,8 @@
// Canonicalize the directory.
StringRef CanonicalDir = FM.getCanonicalName(*DirEntry);
- if (CanonicalDir != Dir) {
- auto CanonicalDirEntry = FM.getDirectory(CanonicalDir);
- // Only use the canonicalized path if it resolves to the same entry as the
- // original. This is not true if there's a VFS overlay on top of a FS where
- // the directory is a symlink. The overlay would not remap the target path
- // of the symlink to the same directory entry in that case.
- if (CanonicalDirEntry && *CanonicalDirEntry == *DirEntry) {
- bool Done = llvm::sys::path::replace_path_prefix(Path, Dir, CanonicalDir);
- (void)Done;
- assert(Done && "Path should always start with Dir");
- }
- }
+ if (CanonicalDir != Dir)
+ llvm::sys::path::replace_path_prefix(Path, Dir, CanonicalDir);
// In theory, the filename component should also be canonicalized if it
// on a case-insensitive filesystem. However, the extra canonicalization is
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
