[PATCH] D23816: Add support for -fdiagnostics-abs-path: printing absolute paths in diagnostics

[Richard Smith via cfe-commits]

On Thu, Aug 25, 2016 at 5:22 PM, Hans Wennborg <hans at chromium.org> wrote:

> hans added inline comments.
>
> ================
> Comment at: lib/Frontend/TextDiagnostic.cpp:770-777
> @@ +769,10 @@
> +  if (DiagOpts->AbsolutePath) {
> +    const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
> +        llvm::sys::path::parent_path(Filename));
> +    if (Dir) {
> +      StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
> +      llvm::sys::path::append(AbsoluteFilename, DirName,
> +                              llvm::sys::path::filename(Filename));
> +      Filename = StringRef(AbsoluteFilename.data(),
> AbsoluteFilename.size());
> +    }
> +  }
> ----------------
> rsmith wrote:
> > Why split off the filename and rejoin it here? Is this to better handle
> cases where the directory exists but the file doesn't, or an attempt to
> avoid resolving file symlinks? A comment in the code explaining this would
> be useful.
> If I understand correctly, getCanonicalName only works with a
> DirectoryEntry, which is supposed to be a directory, not a full path to a
> file. So I didn't see any other way than getting the canonical name for the
> dir, and then joining it with the filename.


Huh, so it does. That's an odd choice. Anyway, let's go with this, unless
you feel like refactoring it to work on an arbitrary path.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160825/b47d3081/attachment.html>