[PATCH] D70527: [clang] Fix the canonicalization of paths in -fdiagnostics-absolute-paths

[Reid Kleckner via Phabricator via cfe-commits]

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