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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.


================
Comment at: clang/lib/Basic/FileManager.cpp:562
 
-  CanonicalDirNames.insert({Dir, CanonicalName});
+  CanonicalNames.insert({Dir, CanonicalName});
+  return CanonicalName;
----------------
There is no need to change now, but I think try_emplace may be slightly more idiomatic as it saves a copy-list-initialization.


================
Comment at: clang/lib/Basic/FileManager.cpp:572
+
+  StringRef CanonicalName(File->getName());
+
----------------
There is no need to change now. `StringRef ... = ` may be more common.


================
Comment at: clang/test/Frontend/absolute-paths-symlinks.c:4
+// RUN: cd %t
+// RUN: cp "%s" "test.c"
+// RUN: ln -sf "test.c" "link.c"
----------------
No need to change now. Quotes are not needed.


================
Comment at: clang/test/Frontend/absolute-paths-symlinks.c:6
+// RUN: ln -sf "test.c" "link.c"
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-absolute-paths "link.c" 2>&1|FileCheck %s
+
----------------
No need to change now. Spaces around `|`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70527/new/

https://reviews.llvm.org/D70527