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

Ka-Ka marked 3 inline comments as done. Ka-Ka added a comment. I have now updated the testcase according to comments by @MaskRay in commit 073cdb239044 Thanks for post-commit review comments. Repository: rG LLVM Github

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

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

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

MaskRay added inline comments. Comment at: clang/test/Frontend/absolute-paths-symlinks.c:15 + +// REQUIRES: shell rnk wrote: > 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. I

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

This revision was automatically updated to reflect the committed changes. Ka-Ka marked 2 inline comments as done. Closed by commit rGe8efac4b1530: [clang] Fix the canonicalization of paths in -fdiagnostics-absolute-paths (authored by Ka-Ka). Changed prior to commit:

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

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

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

Ka-Ka updated this revision to Diff 234268. Ka-Ka marked 2 inline comments as done. Ka-Ka added a comment. Updated patch according to review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70527/new/ https://reviews.llvm.org/D70527 Files:

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

Ka-Ka added a comment. In D70527#1786718 , @rnk wrote: > In D70527#1785552 , @ikudrin wrote: > > > Personally, I would prefer to see the file name and path to be changed as > > little as possible because that

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

rnk added a comment. In D70527#1785552 , @ikudrin wrote: > Personally, I would prefer to see the file name and path to be changed as > little as possible because that would help to recognize the files better. We > cannot use `remove_dots()` on POSIX

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

rnk added a comment. From auditing the call sites, it seems that almost all of them could be simplified by using the new API: http://llvm-cs.pcc.me.uk/tools/clang/lib/Basic/FileManager.cpp/rgetCanonicalName Comment at: clang/include/clang/Basic/FileManager.h:226 /// The

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

ikudrin added a comment. Personally, I would prefer to see the file name and path to be changed as little as possible because that would help to recognize the files better. We cannot use `remove_dots()` on POSIX OSes to simplify paths, because it may return an invalid path; thus we have to use

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

Ka-Ka added a comment. Ping 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

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

Ka-Ka created this revision. Ka-Ka added reviewers: rsmith, hans, rnk, ikudrin. Herald added a project: clang. In the current implementation of clang the canonicalization of paths in diagnostic messages (when using -fdiagnostics-absolute-paths) only works if the symbolic link is in the directory