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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
