rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land.
lgtm ================ Comment at: clang/lib/Basic/FileManager.cpp:551 StringRef FileManager::getCanonicalName(const DirectoryEntry *Dir) { // FIXME: use llvm::sys::fs::canonical() when it gets implemented + llvm::DenseMap<const void *, llvm::StringRef>::iterator Known ---------------- These FIXMEs look stale. We are already using VFS getRealPath which ultimately probably boils down to whatever the original author thought sys::fs::canonical would be. ================ Comment at: clang/lib/Basic/FileManager.cpp:568 +StringRef FileManager::getCanonicalName(const FileEntry *File) { + // FIXME: use llvm::sys::fs::canonical() when it gets implemented + llvm::DenseMap<const void *, llvm::StringRef>::iterator Known ---------------- I don't think we need to carry this FIXME here. ================ Comment at: clang/lib/Basic/FileManager.cpp:570 + llvm::DenseMap<const void *, llvm::StringRef>::iterator Known + = CanonicalNames.find(File); + if (Known != CanonicalNames.end()) ---------------- uber nit: use the .insert get-or-create pattern ================ Comment at: clang/test/Frontend/absolute-paths-symlinks.c:15 + +// REQUIRES: shell ---------------- I wish we had a more precise feature to indicate symlink support, but this is what we do elsewhere, so there's nothing to do here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70527/new/ https://reviews.llvm.org/D70527 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits