Skip to content

Commit

Permalink
[clang] Fix the canonicalization of paths in -fdiagnostics-absolute-p…
Browse files Browse the repository at this point in the history
…aths

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 part of the filename, not if
the file itself is a symbolic link to another file.

This patch adds support to canonicalize the complete path including the
file.

Reviewers: rsmith, hans, rnk, ikudrin

Reviewed By: rnk

Differential Revision: https://reviews.llvm.org/D70527
  • Loading branch information
karka228 committed Dec 20, 2019
1 parent 92211bf commit e8efac4
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 19 deletions.
11 changes: 9 additions & 2 deletions clang/include/clang/Basic/FileManager.h
Expand Up @@ -222,8 +222,8 @@ class FileManager : public RefCountedBase<FileManager> {
llvm::BumpPtrAllocator>
SeenFileEntries;

/// The canonical names of directories.
llvm::DenseMap<const DirectoryEntry *, llvm::StringRef> CanonicalDirNames;
/// The canonical names of files and directories .
llvm::DenseMap<const void *, llvm::StringRef> CanonicalNames;

/// Storage for canonical names that we have computed.
llvm::BumpPtrAllocator CanonicalNameStorage;
Expand Down Expand Up @@ -421,6 +421,13 @@ class FileManager : public RefCountedBase<FileManager> {
/// required, which is (almost) never.
StringRef getCanonicalName(const DirectoryEntry *Dir);

/// Retrieve the canonical name for a given file.
///
/// This is a very expensive operation, despite its results being cached,
/// and should only be used when the physical layout of the file system is
/// required, which is (almost) never.
StringRef getCanonicalName(const FileEntry *File);

void PrintStats() const;
};

Expand Down
25 changes: 20 additions & 5 deletions clang/lib/Basic/FileManager.cpp
Expand Up @@ -548,10 +548,9 @@ void FileManager::GetUniqueIDMapping(
}

StringRef FileManager::getCanonicalName(const DirectoryEntry *Dir) {
// FIXME: use llvm::sys::fs::canonical() when it gets implemented
llvm::DenseMap<const DirectoryEntry *, llvm::StringRef>::iterator Known
= CanonicalDirNames.find(Dir);
if (Known != CanonicalDirNames.end())
llvm::DenseMap<const void *, llvm::StringRef>::iterator Known
= CanonicalNames.find(Dir);
if (Known != CanonicalNames.end())
return Known->second;

StringRef CanonicalName(Dir->getName());
Expand All @@ -560,7 +559,23 @@ StringRef FileManager::getCanonicalName(const DirectoryEntry *Dir) {
if (!FS->getRealPath(Dir->getName(), CanonicalNameBuf))
CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);

CanonicalDirNames.insert({Dir, CanonicalName});
CanonicalNames.insert({Dir, CanonicalName});
return CanonicalName;
}

StringRef FileManager::getCanonicalName(const FileEntry *File) {
llvm::DenseMap<const void *, llvm::StringRef>::iterator Known
= CanonicalNames.find(File);
if (Known != CanonicalNames.end())
return Known->second;

StringRef CanonicalName(File->getName());

SmallString<4096> CanonicalNameBuf;
if (!FS->getRealPath(File->getName(), CanonicalNameBuf))
CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);

CanonicalNames.insert({File, CanonicalName});
return CanonicalName;
}

Expand Down
23 changes: 11 additions & 12 deletions clang/lib/Frontend/TextDiagnostic.cpp
Expand Up @@ -761,11 +761,12 @@ void TextDiagnostic::printDiagnosticMessage(raw_ostream &OS,
}

void TextDiagnostic::emitFilename(StringRef Filename, const SourceManager &SM) {
SmallVector<char, 128> AbsoluteFilename;
#ifdef _WIN32
SmallString<4096> TmpFilename;
#endif
if (DiagOpts->AbsolutePath) {
auto Dir = SM.getFileManager().getDirectory(
llvm::sys::path::parent_path(Filename));
if (Dir) {
auto File = SM.getFileManager().getFile(Filename);
if (File) {
// We want to print a simplified absolute path, i. e. without "dots".
//
// The hardest part here are the paths like "<part1>/<link>/../<part2>".
Expand All @@ -781,16 +782,14 @@ void TextDiagnostic::emitFilename(StringRef Filename, const SourceManager &SM) {
// on Windows we can just use llvm::sys::path::remove_dots(), because,
// on that system, both aforementioned paths point to the same place.
#ifdef _WIN32
SmallString<4096> DirName = (*Dir)->getName();
llvm::sys::fs::make_absolute(DirName);
llvm::sys::path::native(DirName);
llvm::sys::path::remove_dots(DirName, /* remove_dot_dot */ true);
TmpFilename = (*File)->getName();
llvm::sys::fs::make_absolute(TmpFilename);
llvm::sys::path::native(TmpFilename);
llvm::sys::path::remove_dots(TmpFilename, /* remove_dot_dot */ true);
Filename = StringRef(TmpFilename.data(), TmpFilename.size());
#else
StringRef DirName = SM.getFileManager().getCanonicalName(*Dir);
Filename = SM.getFileManager().getCanonicalName(*File);
#endif
llvm::sys::path::append(AbsoluteFilename, DirName,
llvm::sys::path::filename(Filename));
Filename = StringRef(AbsoluteFilename.data(), AbsoluteFilename.size());
}
}

Expand Down
15 changes: 15 additions & 0 deletions clang/test/Frontend/absolute-paths-symlinks.c
@@ -0,0 +1,15 @@
// RUN: rm -rf %t
// RUN: mkdir %t
// RUN: cd %t
// RUN: cp "%s" "test.c"
// RUN: ln -sf "test.c" "link.c"
// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-absolute-paths "link.c" 2>&1|FileCheck %s

// Verify that -fdiagnostics-absolute-paths resolve symbolic links in
// diagnostics messages.

// CHECK: test.c
// CHECK-SAME: error: unknown type name
This do not compile

// REQUIRES: shell

0 comments on commit e8efac4

Please sign in to comment.