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 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 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,49 @@ return Edits; } -Optional getRealPath(const FileEntry *F, - const SourceManager &SourceMgr) { +Optional 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, 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 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 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/index/Background.cpp =================================================================== --- clangd/index/Background.cpp +++ clangd/index/Background.cpp @@ -96,26 +96,20 @@ createFileFilter(const llvm::StringMap &FileDigests, llvm::StringMap &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; }; } Index: clangd/index/SymbolCollector.cpp =================================================================== --- clangd/index/SymbolCollector.cpp +++ clangd/index/SymbolCollector.cpp @@ -51,37 +51,17 @@ // // The Path can be a path relative to the build directory, or retrieved from // the SourceManager. -Optional 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 @@ 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 @@ 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