kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ioeric.
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.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D55818
Files:
clangd/SourceCode.cpp
clangd/SourceCode.h
clangd/XRefs.cpp
clangd/index/Background.cpp
clangd/index/SymbolCollector.cpp
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -51,37 +51,18 @@
//
// 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;
+ } else {
+ elog("Couldn't get canonical path for: {0}: {1}", Path,
+ CanonPath.takeError());
}
-
+ 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 +192,17 @@
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 +468,7 @@
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
Index: clangd/index/Background.cpp
===================================================================
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -96,26 +96,23 @@
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) {
+ elog("Warning: could not get canonical path for file: {0}",
+ AbsPath.takeError());
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;
};
}
Index: clangd/XRefs.cpp
===================================================================
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -229,7 +229,7 @@
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 @@
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::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 @@
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;
Index: clangd/SourceCode.h
===================================================================
--- clangd/SourceCode.h
+++ clangd/SourceCode.h
@@ -78,17 +78,18 @@
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::Expected<std::string> getCanonicalPath(const FileEntry *F,
+ const SourceManager &SourceMgr);
bool IsRangeConsecutive(const Range &Left, const Range &Right);
} // namespace clangd
Index: clangd/SourceCode.cpp
===================================================================
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -183,34 +183,38 @@
return Edits;
}
-Optional<std::string> getRealPath(const FileEntry *F,
- const SourceManager &SourceMgr) {
+llvm::Expected<std::string> getCanonicalPath(const FileEntry *F,
+ const SourceManager &SourceMgr) {
+ if (!F)
+ return llvm::make_error<StringError>("Null fileentry",
+ llvm::inconvertibleErrorCode());
// Ideally, we get the real path from the FileEntry object.
SmallString<128> FilePath = F->tryGetRealPathName();
- if (!FilePath.empty()) {
+ if (!FilePath.empty())
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();
-
- if (!sys::path::is_absolute(Path)) {
- if (!SourceMgr.getFileManager().makeAbsolutePath(Path)) {
- log("Could not turn relative path to absolute: {0}", Path);
- return None;
- }
+ FilePath = F->getName();
+ vlog("FileEntry for {0} did not contain the real path.", FilePath);
+
+ if (!sys::path::is_absolute(FilePath)) {
+ if (!SourceMgr.getFileManager().makeAbsolutePath(FilePath))
+ return llvm::make_error<StringError>(
+ "Could not turn relative path to absolute: " + FilePath,
+ llvm::inconvertibleErrorCode());
}
- SmallString<128> RealPath;
- if (SourceMgr.getFileManager().getVirtualFileSystem()->getRealPath(
- Path, RealPath)) {
- log("Could not compute real path: {0}", Path);
- return Path.str().str();
+ // Resolve symlinks in the parent directories.
+ if (const DirectoryEntry *Dir = SourceMgr.getFileManager().getDirectory(
+ llvm::sys::path::parent_path(FilePath))) {
+ SmallString<128> RealPath;
+ StringRef DirName = SourceMgr.getFileManager().getCanonicalName(Dir);
+ llvm::sys::path::append(RealPath, DirName,
+ llvm::sys::path::filename(FilePath));
+ return RealPath.str().str();
}
- return RealPath.str().str();
+ return FilePath.str().str();
}
TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
