[clang-tools-extra] r349618 - [clangd] Unify path canonicalizations in the codebase
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 19 02:46:21 PST 2018
Author: kadircet
Date: Wed Dec 19 02:46:21 2018
New Revision: 349618
URL: http://llvm.org/viewvc/llvm-project?rev=349618&view=rev
Log:
[clangd] Unify path canonicalizations in the codebase
Summary:
There were a few different places where we canonicalized paths, each
one had its own flavor. This patch tries to unify them all under one place.
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D55818
Modified:
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/SourceCode.h
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=349618&r1=349617&r2=349618&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/SourceCode.cpp (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp Wed Dec 19 02:46:21 2018
@@ -183,34 +183,49 @@ std::vector<TextEdit> replacementsToEdit
return Edits;
}
-Optional<std::string> getRealPath(const FileEntry *F,
- const SourceManager &SourceMgr) {
+Optional<std::string> getCanonicalPath(const FileEntry *F,
+ const SourceManager &SourceMgr) {
+ if (!F)
+ return None;
// Ideally, we get the real path from the FileEntry object.
SmallString<128> FilePath = F->tryGetRealPathName();
- if (!FilePath.empty()) {
+ if (!FilePath.empty() && sys::path::is_absolute(FilePath))
return FilePath.str().str();
- }
// Otherwise, we try to compute ourselves.
- vlog("FileEntry for {0} did not contain the real path.", F->getName());
-
- SmallString<128> Path = F->getName();
+ FilePath = F->getName();
+ vlog("FileEntry for {0} did not contain the real path.", FilePath);
- if (!sys::path::is_absolute(Path)) {
- if (!SourceMgr.getFileManager().makeAbsolutePath(Path)) {
- log("Could not turn relative path to absolute: {0}", Path);
+ if (!sys::path::is_absolute(FilePath)) {
+ if (auto EC =
+ SourceMgr.getFileManager().getVirtualFileSystem()->makeAbsolute(
+ FilePath)) {
+ elog("Could not turn relative path '{0}' to absolute: {1}", FilePath,
+ EC.message());
return None;
}
}
- SmallString<128> RealPath;
- if (SourceMgr.getFileManager().getVirtualFileSystem()->getRealPath(
- Path, RealPath)) {
- log("Could not compute real path: {0}", Path);
- return Path.str().str();
+ // Handle the symbolic link path case where the current working directory
+ // (getCurrentWorkingDirectory) is a symlink./ We always want to the real
+ // file path (instead of the symlink path) for the C++ symbols.
+ //
+ // Consider the following example:
+ //
+ // src dir: /project/src/foo.h
+ // current working directory (symlink): /tmp/build -> /project/src/
+ //
+ // The file path of Symbol is "/project/src/foo.h" instead of
+ // "/tmp/build/foo.h"
+ if (const DirectoryEntry *Dir = SourceMgr.getFileManager().getDirectory(
+ sys::path::parent_path(FilePath))) {
+ SmallString<128> RealPath;
+ StringRef DirName = SourceMgr.getFileManager().getCanonicalName(Dir);
+ sys::path::append(RealPath, DirName, sys::path::filename(FilePath));
+ return RealPath.str().str();
}
- return RealPath.str().str();
+ return FilePath.str().str();
}
TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
Modified: clang-tools-extra/trunk/clangd/SourceCode.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.h?rev=349618&r1=349617&r2=349618&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/SourceCode.h (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.h Wed Dec 19 02:46:21 2018
@@ -78,17 +78,18 @@ std::vector<TextEdit> replacementsToEdit
TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
const LangOptions &L);
-/// Get the real/canonical path of \p F. This means:
+/// Get the canonical path of \p F. This means:
///
/// - Absolute path
/// - Symlinks resolved
/// - No "." or ".." component
/// - No duplicate or trailing directory separator
///
-/// This function should be used when sending paths to clients, so that paths
-/// are normalized as much as possible.
-llvm::Optional<std::string> getRealPath(const FileEntry *F,
- const SourceManager &SourceMgr);
+/// This function should be used when paths needs to be used outside the
+/// component that generate it, so that paths are normalized as much as
+/// possible.
+llvm::Optional<std::string> getCanonicalPath(const FileEntry *F,
+ const SourceManager &SourceMgr);
bool IsRangeConsecutive(const Range &Left, const Range &Right);
} // namespace clangd
Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=349618&r1=349617&r2=349618&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Wed Dec 19 02:46:21 2018
@@ -229,7 +229,7 @@ Optional<Location> makeLocation(ParsedAS
const FileEntry *F = SourceMgr.getFileEntryForID(SourceMgr.getFileID(TokLoc));
if (!F)
return None;
- auto FilePath = getRealPath(F, SourceMgr);
+ auto FilePath = getCanonicalPath(F, SourceMgr);
if (!FilePath) {
log("failed to get path!");
return None;
@@ -245,7 +245,8 @@ Optional<Location> makeLocation(ParsedAS
std::vector<Location> findDefinitions(ParsedAST &AST, Position Pos,
const SymbolIndex *Index) {
const auto &SM = AST.getASTContext().getSourceManager();
- auto MainFilePath = getRealPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
+ auto MainFilePath =
+ getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
if (!MainFilePath) {
elog("Failed to get a path for the main file, so no references");
return {};
@@ -337,7 +338,7 @@ std::vector<Location> findDefinitions(Pa
std::string TUPath;
const FileEntry *FE =
SM.getFileEntryForID(SM.getMainFileID());
- if (auto Path = getRealPath(FE, SM))
+ if (auto Path = getCanonicalPath(FE, SM))
TUPath = *Path;
// Query the index and populate the empty slot.
Index->lookup(QueryRequest, [&TUPath, &ResultCandidates,
@@ -708,7 +709,8 @@ std::vector<Location> findReferences(Par
const SymbolIndex *Index) {
std::vector<Location> Results;
const SourceManager &SM = AST.getASTContext().getSourceManager();
- auto MainFilePath = getRealPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
+ auto MainFilePath =
+ getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
if (!MainFilePath) {
elog("Failed to get a path for the main file, so no references");
return Results;
Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=349618&r1=349617&r2=349618&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Wed Dec 19 02:46:21 2018
@@ -98,26 +98,20 @@ decltype(SymbolCollector::Options::FileF
createFileFilter(const llvm::StringMap<FileDigest> &FileDigests,
llvm::StringMap<FileDigest> &FilesToUpdate) {
return [&FileDigests, &FilesToUpdate](const SourceManager &SM, FileID FID) {
- StringRef Path;
- if (const auto *F = SM.getFileEntryForID(FID))
- Path = F->getName();
- if (Path.empty())
+ const auto *F = SM.getFileEntryForID(FID);
+ if (!F)
return false; // Skip invalid files.
- SmallString<128> AbsPath(Path);
- if (std::error_code EC =
- SM.getFileManager().getVirtualFileSystem()->makeAbsolute(AbsPath)) {
- elog("Warning: could not make absolute file: {0}", EC.message());
+ auto AbsPath = getCanonicalPath(F, SM);
+ if (!AbsPath)
return false; // Skip files without absolute path.
- }
- sys::path::remove_dots(AbsPath, /*remove_dot_dot=*/true);
auto Digest = digestFile(SM, FID);
if (!Digest)
return false;
- auto D = FileDigests.find(AbsPath);
+ auto D = FileDigests.find(*AbsPath);
if (D != FileDigests.end() && D->second == Digest)
return false; // Skip files that haven't changed.
- FilesToUpdate[AbsPath] = *Digest;
+ FilesToUpdate[*AbsPath] = *Digest;
return true;
};
}
Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=349618&r1=349617&r2=349618&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Wed Dec 19 02:46:21 2018
@@ -51,37 +51,17 @@ const NamedDecl &getTemplateOrThis(const
//
// The Path can be a path relative to the build directory, or retrieved from
// the SourceManager.
-Optional<std::string> toURI(const SourceManager &SM, StringRef Path,
- const SymbolCollector::Options &Opts) {
- SmallString<128> AbsolutePath(Path);
- if (std::error_code EC =
- SM.getFileManager().getVirtualFileSystem()->makeAbsolute(
- AbsolutePath))
- log("Warning: could not make absolute file: {0}", EC.message());
- if (sys::path::is_absolute(AbsolutePath)) {
- // Handle the symbolic link path case where the current working directory
- // (getCurrentWorkingDirectory) is a symlink./ We always want to the real
- // file path (instead of the symlink path) for the C++ symbols.
- //
- // Consider the following example:
- //
- // src dir: /project/src/foo.h
- // current working directory (symlink): /tmp/build -> /project/src/
- //
- // The file path of Symbol is "/project/src/foo.h" instead of
- // "/tmp/build/foo.h"
- if (const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
- sys::path::parent_path(AbsolutePath.str()))) {
- StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
- SmallString<128> AbsoluteFilename;
- sys::path::append(AbsoluteFilename, DirName,
- sys::path::filename(AbsolutePath.str()));
- AbsolutePath = AbsoluteFilename;
- }
- } else if (!Opts.FallbackDir.empty()) {
- sys::fs::make_absolute(Opts.FallbackDir, AbsolutePath);
+std::string toURI(const SourceManager &SM, llvm::StringRef Path,
+ const SymbolCollector::Options &Opts) {
+ llvm::SmallString<128> AbsolutePath(Path);
+ if (auto CanonPath =
+ getCanonicalPath(SM.getFileManager().getFile(Path), SM)) {
+ AbsolutePath = *CanonPath;
}
-
+ // We don't perform is_absolute check in an else branch because makeAbsolute
+ // might return a relative path on some InMemoryFileSystems.
+ if (!sys::path::is_absolute(AbsolutePath) && !Opts.FallbackDir.empty())
+ sys::fs::make_absolute(Opts.FallbackDir, AbsolutePath);
sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/true);
return URI::create(AbsolutePath).toString();
}
@@ -211,17 +191,17 @@ Optional<SymbolLocation> getTokenLocatio
const SymbolCollector::Options &Opts,
const clang::LangOptions &LangOpts,
std::string &FileURIStorage) {
- auto U = toURI(SM, SM.getFilename(TokLoc), Opts);
- if (!U)
+ auto Path = SM.getFilename(TokLoc);
+ if (Path.empty())
return None;
- FileURIStorage = std::move(*U);
+ FileURIStorage = toURI(SM, Path, Opts);
SymbolLocation Result;
Result.FileURI = FileURIStorage.c_str();
auto Range = getTokenRange(TokLoc, SM, LangOpts);
Result.Start = Range.first;
Result.End = Range.second;
- return std::move(Result);
+ return Result;
}
// Checks whether \p ND is a definition of a TagDecl (class/struct/enum/union)
@@ -487,11 +467,7 @@ void SymbolCollector::finish() {
if (Found == URICache.end()) {
if (auto *FileEntry = SM.getFileEntryForID(FID)) {
auto FileURI = toURI(SM, FileEntry->getName(), Opts);
- if (!FileURI) {
- log("Failed to create URI for file: {0}\n", FileEntry);
- FileURI = ""; // reset to empty as we also want to cache this case.
- }
- Found = URICache.insert({FID, *FileURI}).first;
+ Found = URICache.insert({FID, FileURI}).first;
} else {
// Ignore cases where we can not find a corresponding file entry
// for the loc, thoses are not interesting, e.g. symbols formed
More information about the cfe-commits
mailing list