diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp --- a/clang/lib/Lex/ModuleMap.cpp +++ b/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 diff --git a/clang/test/ClangScanDeps/modules-canononical-module-map-case.c b/clang/test/ClangScanDeps/modules-canononical-module-map-case.c new file mode 100644 --- /dev/null +++ b/clang/test/ClangScanDeps/modules-canononical-module-map-case.c @@ -0,0 +1,71 @@ +// This test checks that VFS-mapped module map path has the correct spelling +// after canonicalization, even if it was first accessed using different case. + +// RUN: rm -rf %t +// RUN: split-file %s %t + +//--- actual/One.h +#import +//--- actual/Two.h +// empty +//--- frameworks/FW.framework/Modules/module.modulemap +framework module FW { + header "One.h" + header "Two.h" +} +//--- tu.m +#import + +//--- 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: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json +// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t + +// CHECK: { +// CHECK-NEXT: "modules": [ +// CHECK-NEXT: { +// CHECK-NEXT: "clang-module-deps": [], +// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap", +// CHECK-NEXT: "command-line": [ +// CHECK: "-x" +// CHECK-NEXT: "objective-c" +// CHECK-NEXT: "[[PREFIX]]/frameworks/FW.framework/Modules/module.modulemap" +// CHECK: ], +// CHECK-NEXT: "context-hash": "{{.*}}", +// CHECK-NEXT: "file-deps": [ +// CHECK: ], +// CHECK-NEXT: "name": "FW" +// CHECK-NEXT: } +// CHECK-NEXT: ] +// CHECK: } diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/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. + llvm::SmallVector 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 &Path) const; }; private: @@ -984,9 +991,10 @@ /// into the contents of \p From if it is a directory. Returns a LookupResult /// giving the matched entry and, if that entry is a FileEntry or /// DirectoryRemapEntry, the path it redirects to in the external file system. - ErrorOr lookupPathImpl(llvm::sys::path::const_iterator Start, - llvm::sys::path::const_iterator End, - Entry *From) const; + ErrorOr + lookupPathImpl(llvm::sys::path::const_iterator Start, + llvm::sys::path::const_iterator End, Entry *From, + llvm::SmallVectorImpl &Entries) const; /// Get the status for a path with the provided \c LookupResult. ErrorOr status(const Twine &CanonicalPath, const Twine &OriginalPath, diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -2239,6 +2239,14 @@ } } +void RedirectingFileSystem::LookupResult::getPath( + llvm::SmallVectorImpl &Result) const { + Result.clear(); + for (Entry *Parent : Parents) + llvm::sys::path::append(Result, Parent->getName()); + llvm::sys::path::append(Result, E->getName()); +} + std::error_code RedirectingFileSystem::makeCanonical(SmallVectorImpl &Path) const { if (std::error_code EC = makeAbsolute(Path)) @@ -2257,11 +2265,14 @@ RedirectingFileSystem::lookupPath(StringRef Path) const { sys::path::const_iterator Start = sys::path::begin(Path); sys::path::const_iterator End = sys::path::end(Path); + llvm::SmallVector Entries; for (const auto &Root : Roots) { ErrorOr 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); } @@ -2269,7 +2280,8 @@ ErrorOr RedirectingFileSystem::lookupPathImpl( sys::path::const_iterator Start, sys::path::const_iterator End, - RedirectingFileSystem::Entry *From) const { + RedirectingFileSystem::Entry *From, + llvm::SmallVectorImpl &Entries) const { assert(!isTraversalComponent(*Start) && !isTraversalComponent(From->getName()) && "Paths should not contain traversal components"); @@ -2298,10 +2310,12 @@ auto *DE = cast(From); for (const std::unique_ptr &DirEntry : llvm::make_range(DE->contents_begin(), DE->contents_end())) { + Entries.push_back(From); ErrorOr 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; + Entries.pop_back(); } return make_error_code(llvm::errc::no_such_file_or_directory); @@ -2543,10 +2557,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; } diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp --- a/llvm/unittests/Support/VirtualFileSystemTest.cpp +++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp @@ -2580,6 +2580,7 @@ Lower->addSymlink("/link"); IntrusiveRefCntPtr FS = getFromYAMLString( "{ 'use-external-names': false,\n" + " 'case-sensitive': false,\n" " 'roots': [\n" "{\n" " 'type': 'directory',\n" @@ -2588,6 +2589,11 @@ " 'type': 'file',\n" " 'name': 'bar',\n" " 'external-contents': '/link'\n" + " },\n" + " {\n" + " 'type': 'directory',\n" + " 'name': 'baz',\n" + " 'contents': []\n" " }\n" " ]\n" "},\n" @@ -2610,9 +2616,9 @@ EXPECT_FALSE(FS->getRealPath("//root/bar", RealPath)); EXPECT_EQ(RealPath.str(), "/symlink"); - // Directories should fall back to the underlying file system is possible. - EXPECT_FALSE(FS->getRealPath("//dir/", RealPath)); - EXPECT_EQ(RealPath.str(), "//dir/"); + // Directories should return the virtual path as written in the definition. + EXPECT_FALSE(FS->getRealPath("//ROOT/baz", RealPath)); + EXPECT_EQ(RealPath.str(), "//root/baz"); // Try a non-existing file. EXPECT_EQ(FS->getRealPath("/non_existing", RealPath),