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

[Karl-Johan Karlsson via Phabricator via cfe-commits]

Ka-Ka added a comment.

In D70527#1786718 <https://reviews.llvm.org/D70527#1786718>, @rnk wrote:

> In D70527#1785552 <https://reviews.llvm.org/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 OSes to simplify paths, because it may return an invalid path; thus we have to use `getRealPath()`. If I understand it right, there is no similar problem with the file name itself.
> >
> > So, which issues this patch is going to solve?
>
>
> It seems clear to me, the filename could be an absolute symlink to a real file somewhere far removed from the realpath of the parent directory. It seems reasonable that -fdiagnostics-absolute-paths would look through symlinks in this case.


This fix is important when building with Bazel (google build system) as Bazel set up a temporary sandbox (with a series of symbolic links) to be be able to control the dependencies and keep a valid build cache. Currently when using the clang option -fdiagnostics-absolute-paths it will only resolve the sybolic names in the dir-path of the filename which end up with warning- and error-messages to files inside the sandbox instead of the correct source inside the repo you are working in.

In D70527#1786698 <https://reviews.llvm.org/D70527#1786698>, @rnk wrote:

> 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


I agree, that they can be simplified, but I guess it is easier to do those changes in separate patches.



================
Comment at: clang/include/clang/Basic/FileManager.h:226
   /// The canonical names of directories.
   llvm::DenseMap<const DirectoryEntry *, llvm::StringRef> CanonicalDirNames;
 
----------------
rnk wrote:
> You could make the key `void*` and save a DenseMap here. Nobody ever iterates CanonicalDirNames to look at the keys.
Sure, I will update the patch ...



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70527