kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added subscribers: usaxena95, arphaman.
Herald added a project: All.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang-tools-extra.
Lots of features built on top of ASTs require getting back to the path
of the TU and they used lossy conversion from file ids using sourcemanager.
This patch preserves the file path passed by the caller inside ParsedAST for
later use.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D130690
Files:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/ParsedAST.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/XRefs.h
clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -408,12 +408,7 @@
Expected<Effect> apply(const Selection &Sel) override {
const SourceManager &SM = Sel.AST->getSourceManager();
- auto MainFileName =
- getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
- if (!MainFileName)
- return error("Couldn't get absolute path for main file.");
-
- auto CCFile = getSourceFile(*MainFileName, Sel);
+ auto CCFile = getSourceFile(Sel.AST->tuPath(), Sel);
if (!CCFile)
return error("Couldn't find a suitable implementation file.");
Index: clang-tools-extra/clangd/XRefs.h
===================================================================
--- clang-tools-extra/clangd/XRefs.h
+++ clang-tools-extra/clangd/XRefs.h
@@ -20,6 +20,7 @@
#include "support/Path.h"
#include "clang/AST/ASTTypeTraits.h"
#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/raw_ostream.h"
#include <vector>
@@ -59,10 +60,11 @@
// AST-based resolution does not work, such as comments, strings, and PP
// disabled regions.
// (This is for internal use by locateSymbolAt, and is exposed for testing).
-std::vector<LocatedSymbol>
-locateSymbolTextually(const SpelledWord &Word, ParsedAST &AST,
- const SymbolIndex *Index, const std::string &MainFilePath,
- ASTNodeKind NodeKind);
+std::vector<LocatedSymbol> locateSymbolTextually(const SpelledWord &Word,
+ ParsedAST &AST,
+ const SymbolIndex *Index,
+ llvm::StringRef MainFilePath,
+ ASTNodeKind NodeKind);
// Try to find a proximate occurrence of `Word` as an identifier, which can be
// used to resolve it.
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -484,12 +484,7 @@
std::vector<LocatedSymbol> locateSymbolForType(const ParsedAST &AST,
const QualType &Type) {
const auto &SM = AST.getSourceManager();
- auto MainFilePath =
- getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
- if (!MainFilePath) {
- elog("Failed to get a path for the main file, so no symbol.");
- return {};
- }
+ auto MainFilePath = AST.tuPath();
// FIXME: this sends unique_ptr<Foo> to unique_ptr<T>.
// Likely it would be better to send it to Foo (heuristically) or to both.
@@ -505,7 +500,7 @@
for (const NamedDecl *D : Decls) {
D = getPreferredDecl(D);
- auto Loc = makeLocation(ASTContext, nameLocation(*D, SM), *MainFilePath);
+ auto Loc = makeLocation(ASTContext, nameLocation(*D, SM), MainFilePath);
if (!Loc)
continue;
@@ -515,7 +510,7 @@
Results.back().ID = getSymbolID(D);
if (const NamedDecl *Def = getDefinition(D))
Results.back().Definition =
- makeLocation(ASTContext, nameLocation(*Def, SM), *MainFilePath);
+ makeLocation(ASTContext, nameLocation(*Def, SM), MainFilePath);
}
return Results;
@@ -546,10 +541,11 @@
} // namespace
-std::vector<LocatedSymbol>
-locateSymbolTextually(const SpelledWord &Word, ParsedAST &AST,
- const SymbolIndex *Index, const std::string &MainFilePath,
- ASTNodeKind NodeKind) {
+std::vector<LocatedSymbol> locateSymbolTextually(const SpelledWord &Word,
+ ParsedAST &AST,
+ const SymbolIndex *Index,
+ llvm::StringRef MainFilePath,
+ ASTNodeKind NodeKind) {
// Don't use heuristics if this is a real identifier, or not an
// identifier.
// Exception: dependent names, because those may have useful textual
@@ -567,7 +563,7 @@
// Look up the selected word in the index.
FuzzyFindRequest Req;
Req.Query = Word.Text.str();
- Req.ProximityPaths = {MainFilePath};
+ Req.ProximityPaths = {MainFilePath.str()};
// Find the namespaces to query by lexing the file.
Req.Scopes =
visibleNamespaces(sourcePrefix(Word.Location, SM), AST.getLangOpts());
@@ -749,14 +745,9 @@
std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
const SymbolIndex *Index) {
const auto &SM = AST.getSourceManager();
- auto MainFilePath =
- getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
- if (!MainFilePath) {
- elog("Failed to get a path for the main file, so no references");
- return {};
- }
+ auto MainFilePath = AST.tuPath();
- if (auto File = locateFileReferent(Pos, AST, *MainFilePath))
+ if (auto File = locateFileReferent(Pos, AST, MainFilePath))
return {std::move(*File)};
auto CurLoc = sourceLocationInMainFile(SM, Pos);
@@ -771,7 +762,7 @@
syntax::spelledTokensTouching(*CurLoc, AST.getTokens());
for (const syntax::Token &Tok : TokensTouchingCursor) {
if (Tok.kind() == tok::identifier) {
- if (auto Macro = locateMacroReferent(Tok, AST, *MainFilePath))
+ if (auto Macro = locateMacroReferent(Tok, AST, MainFilePath))
// Don't look at the AST or index if we have a macro result.
// (We'd just return declarations referenced from the macro's
// expansion.)
@@ -794,7 +785,7 @@
ASTNodeKind NodeKind;
auto ASTResults = locateASTReferent(*CurLoc, TouchedIdentifier, AST,
- *MainFilePath, Index, NodeKind);
+ MainFilePath, Index, NodeKind);
if (!ASTResults.empty())
return ASTResults;
@@ -805,13 +796,13 @@
// Is the same word nearby a real identifier that might refer to something?
if (const syntax::Token *NearbyIdent =
findNearbyIdentifier(*Word, AST.getTokens())) {
- if (auto Macro = locateMacroReferent(*NearbyIdent, AST, *MainFilePath)) {
+ if (auto Macro = locateMacroReferent(*NearbyIdent, AST, MainFilePath)) {
log("Found macro definition heuristically using nearby identifier {0}",
Word->Text);
return {*std::move(Macro)};
}
ASTResults = locateASTReferent(NearbyIdent->location(), NearbyIdent, AST,
- *MainFilePath, Index, NodeKind);
+ MainFilePath, Index, NodeKind);
if (!ASTResults.empty()) {
log("Found definition heuristically using nearby identifier {0}",
NearbyIdent->text(SM));
@@ -822,7 +813,7 @@
}
// No nearby word, or it didn't refer to anything either. Try the index.
auto TextualResults =
- locateSymbolTextually(*Word, AST, Index, *MainFilePath, NodeKind);
+ locateSymbolTextually(*Word, AST, Index, MainFilePath, NodeKind);
if (!TextualResults.empty())
return TextualResults;
}
@@ -832,12 +823,6 @@
std::vector<DocumentLink> getDocumentLinks(ParsedAST &AST) {
const auto &SM = AST.getSourceManager();
- auto MainFilePath =
- getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
- if (!MainFilePath) {
- elog("Failed to get a path for the main file, so no links");
- return {};
- }
std::vector<DocumentLink> Result;
for (auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
@@ -857,7 +842,7 @@
Result.push_back(
DocumentLink({halfOpenToRange(SM, FileRange),
- URIForFile::canonicalize(Inc.Resolved, *MainFilePath)}));
+ URIForFile::canonicalize(Inc.Resolved, AST.tuPath())}));
}
return Result;
@@ -1275,12 +1260,6 @@
if (!Index)
return {};
const SourceManager &SM = AST.getSourceManager();
- auto MainFilePath =
- getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
- if (!MainFilePath) {
- elog("Failed to get a path for the main file, so no implementations.");
- return {};
- }
auto CurLoc = sourceLocationInMainFile(SM, Pos);
if (!CurLoc) {
elog("Failed to convert position to source location: {0}",
@@ -1302,7 +1281,7 @@
QueryKind = RelationKind::BaseOf;
}
}
- return findImplementors(std::move(IDs), QueryKind, Index, *MainFilePath);
+ return findImplementors(std::move(IDs), QueryKind, Index, AST.tuPath());
}
namespace {
@@ -1324,13 +1303,8 @@
const SymbolIndex *Index) {
ReferencesResult Results;
const SourceManager &SM = AST.getSourceManager();
- 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;
- }
- auto URIMainFile = URIForFile::canonicalize(*MainFilePath, *MainFilePath);
+ auto MainFilePath = AST.tuPath();
+ auto URIMainFile = URIForFile::canonicalize(MainFilePath, MainFilePath);
auto CurLoc = sourceLocationInMainFile(SM, Pos);
if (!CurLoc) {
llvm::consumeError(CurLoc.takeError());
@@ -1434,8 +1408,8 @@
return;
}
const auto LSPLocDecl =
- toLSPLocation(Object.CanonicalDeclaration, *MainFilePath);
- const auto LSPLocDef = toLSPLocation(Object.Definition, *MainFilePath);
+ toLSPLocation(Object.CanonicalDeclaration, MainFilePath);
+ const auto LSPLocDef = toLSPLocation(Object.Definition, MainFilePath);
if (LSPLocDecl && LSPLocDecl != LSPLocDef) {
ReferencesResult::Reference Result;
Result.Loc = std::move(*LSPLocDecl);
@@ -1472,10 +1446,10 @@
}
}
Results.HasMore |= Index->refs(Req, [&](const Ref &R) {
- auto LSPLoc = toLSPLocation(R.Location, *MainFilePath);
+ auto LSPLoc = toLSPLocation(R.Location, MainFilePath);
// Avoid indexed results for the main file - the AST is authoritative.
if (!LSPLoc ||
- (!AllowMainFileSymbols && LSPLoc->uri.file() == *MainFilePath))
+ (!AllowMainFileSymbols && LSPLoc->uri.file() == MainFilePath))
return;
ReferencesResult::Reference Result;
Result.Loc = std::move(*LSPLoc);
@@ -1574,7 +1548,8 @@
}
template <typename HierarchyItem>
-static llvm::Optional<HierarchyItem> declToHierarchyItem(const NamedDecl &ND) {
+static llvm::Optional<HierarchyItem>
+declToHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) {
ASTContext &Ctx = ND.getASTContext();
auto &SM = Ctx.getSourceManager();
SourceLocation NameLoc = nameLocation(ND, Ctx.getSourceManager());
@@ -1586,8 +1561,7 @@
return llvm::None;
auto FilePath =
getCanonicalPath(SM.getFileEntryForID(SM.getFileID(NameLoc)), SM);
- auto TUPath = getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
- if (!FilePath || !TUPath)
+ if (!FilePath)
return llvm::None; // Not useful without a uri.
Position NameBegin = sourceLocToPosition(SM, NameLoc);
@@ -1611,7 +1585,7 @@
HI.range = HI.selectionRange;
}
- HI.uri = URIForFile::canonicalize(*FilePath, *TUPath);
+ HI.uri = URIForFile::canonicalize(*FilePath, TUPath);
// Compute the SymbolID and store it in the 'data' field.
// This allows typeHierarchy/resolve to be used to
@@ -1624,16 +1598,16 @@
}
static llvm::Optional<TypeHierarchyItem>
-declToTypeHierarchyItem(const NamedDecl &ND) {
- auto Result = declToHierarchyItem<TypeHierarchyItem>(ND);
+declToTypeHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) {
+ auto Result = declToHierarchyItem<TypeHierarchyItem>(ND, TUPath);
if (Result)
Result->deprecated = ND.isDeprecated();
return Result;
}
static llvm::Optional<CallHierarchyItem>
-declToCallHierarchyItem(const NamedDecl &ND) {
- auto Result = declToHierarchyItem<CallHierarchyItem>(ND);
+declToCallHierarchyItem(const NamedDecl &ND, llvm::StringRef TUPath) {
+ auto Result = declToHierarchyItem<CallHierarchyItem>(ND, TUPath);
if (Result && ND.isDeprecated())
Result->tags.push_back(SymbolTag::Deprecated);
return Result;
@@ -1699,7 +1673,7 @@
using RecursionProtectionSet = llvm::SmallSet<const CXXRecordDecl *, 4>;
-static void fillSuperTypes(const CXXRecordDecl &CXXRD, ASTContext &ASTCtx,
+static void fillSuperTypes(const CXXRecordDecl &CXXRD, llvm::StringRef TUPath,
std::vector<TypeHierarchyItem> &SuperTypes,
RecursionProtectionSet &RPSet) {
// typeParents() will replace dependent template specializations
@@ -1716,9 +1690,9 @@
for (const CXXRecordDecl *ParentDecl : typeParents(&CXXRD)) {
if (Optional<TypeHierarchyItem> ParentSym =
- declToTypeHierarchyItem(*ParentDecl)) {
+ declToTypeHierarchyItem(*ParentDecl, TUPath)) {
ParentSym->parents.emplace();
- fillSuperTypes(*ParentDecl, ASTCtx, *ParentSym->parents, RPSet);
+ fillSuperTypes(*ParentDecl, TUPath, *ParentSym->parents, RPSet);
SuperTypes.emplace_back(std::move(*ParentSym));
}
}
@@ -2041,7 +2015,8 @@
CXXRD = CTSD->getTemplateInstantiationPattern();
}
- Optional<TypeHierarchyItem> Result = declToTypeHierarchyItem(*CXXRD);
+ Optional<TypeHierarchyItem> Result =
+ declToTypeHierarchyItem(*CXXRD, AST.tuPath());
if (!Result)
return Result;
@@ -2049,7 +2024,7 @@
Result->parents.emplace();
RecursionProtectionSet RPSet;
- fillSuperTypes(*CXXRD, AST.getASTContext(), *Result->parents, RPSet);
+ fillSuperTypes(*CXXRD, AST.tuPath(), *Result->parents, RPSet);
}
if (WantChildren && ResolveLevels > 0) {
@@ -2099,7 +2074,7 @@
cast<DeclContext>(Decl)->isFunctionOrMethod()) &&
Decl->getKind() != Decl::Kind::FunctionTemplate)
continue;
- if (auto CHI = declToCallHierarchyItem(*Decl))
+ if (auto CHI = declToCallHierarchyItem(*Decl, AST.tuPath()))
Result.emplace_back(std::move(*CHI));
}
return Result;
Index: clang-tools-extra/clangd/ParsedAST.h
===================================================================
--- clang-tools-extra/clangd/ParsedAST.h
+++ clang-tools-extra/clangd/ParsedAST.h
@@ -26,6 +26,7 @@
#include "Headers.h"
#include "Preamble.h"
#include "index/CanonicalIncludes.h"
+#include "support/Path.h"
#include "clang/Frontend/FrontendAction.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/Syntax/Tokens.h"
@@ -108,6 +109,9 @@
/// Returns the version of the ParseInputs this AST was built from.
llvm::StringRef version() const { return Version; }
+ /// Returns the path passed by the caller when building this AST.
+ PathRef tuPath() const { return TUPath; }
+
/// Returns the version of the ParseInputs used to build Preamble part of this
/// AST. Might be None if no Preamble is used.
llvm::Optional<llvm::StringRef> preambleVersion() const;
@@ -117,7 +121,7 @@
}
private:
- ParsedAST(llvm::StringRef Version,
+ ParsedAST(PathRef TUPath, llvm::StringRef Version,
std::shared_ptr<const PreambleData> Preamble,
std::unique_ptr<CompilerInstance> Clang,
std::unique_ptr<FrontendAction> Action, syntax::TokenBuffer Tokens,
@@ -126,6 +130,7 @@
llvm::Optional<std::vector<Diag>> Diags, IncludeStructure Includes,
CanonicalIncludes CanonIncludes);
+ Path TUPath;
std::string Version;
// In-memory preambles must outlive the AST, it is important that this member
// goes before Clang and Action.
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -665,10 +665,11 @@
Diags->insert(Diags->end(), D.begin(), D.end());
}
}
- ParsedAST Result(Inputs.Version, std::move(Preamble), std::move(Clang),
- std::move(Action), std::move(Tokens), std::move(Macros),
- std::move(Marks), std::move(ParsedDecls), std::move(Diags),
- std::move(Includes), std::move(CanonIncludes));
+ ParsedAST Result(Filename, Inputs.Version, std::move(Preamble),
+ std::move(Clang), std::move(Action), std::move(Tokens),
+ std::move(Macros), std::move(Marks), std::move(ParsedDecls),
+ std::move(Diags), std::move(Includes),
+ std::move(CanonIncludes));
if (Result.Diags) {
auto UnusedHeadersDiags =
issueUnusedIncludesDiagnostics(Result, Inputs.Contents);
@@ -759,7 +760,7 @@
return CanonIncludes;
}
-ParsedAST::ParsedAST(llvm::StringRef Version,
+ParsedAST::ParsedAST(PathRef TUPath, llvm::StringRef Version,
std::shared_ptr<const PreambleData> Preamble,
std::unique_ptr<CompilerInstance> Clang,
std::unique_ptr<FrontendAction> Action,
@@ -768,10 +769,10 @@
std::vector<Decl *> LocalTopLevelDecls,
llvm::Optional<std::vector<Diag>> Diags,
IncludeStructure Includes, CanonicalIncludes CanonIncludes)
- : Version(Version), Preamble(std::move(Preamble)), Clang(std::move(Clang)),
- Action(std::move(Action)), Tokens(std::move(Tokens)),
- Macros(std::move(Macros)), Marks(std::move(Marks)),
- Diags(std::move(Diags)),
+ : TUPath(TUPath), Version(Version), Preamble(std::move(Preamble)),
+ Clang(std::move(Clang)), Action(std::move(Action)),
+ Tokens(std::move(Tokens)), Macros(std::move(Macros)),
+ Marks(std::move(Marks)), Diags(std::move(Diags)),
LocalTopLevelDecls(std::move(LocalTopLevelDecls)),
Includes(std::move(Includes)), CanonIncludes(std::move(CanonIncludes)) {
Resolver = std::make_unique<HeuristicResolver>(getASTContext());
Index: clang-tools-extra/clangd/Hover.cpp
===================================================================
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -975,19 +975,16 @@
return llvm::None;
// Show full header file path if cursor is on include directive.
- if (const auto MainFilePath =
- getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM)) {
- for (const auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
- if (Inc.Resolved.empty() || Inc.HashLine != Pos.line)
- continue;
- HoverInfo HI;
- HI.Name = std::string(llvm::sys::path::filename(Inc.Resolved));
- // FIXME: We don't have a fitting value for Kind.
- HI.Definition =
- URIForFile::canonicalize(Inc.Resolved, *MainFilePath).file().str();
- HI.DefinitionLanguage = "";
- return HI;
- }
+ for (const auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
+ if (Inc.Resolved.empty() || Inc.HashLine != Pos.line)
+ continue;
+ HoverInfo HI;
+ HI.Name = std::string(llvm::sys::path::filename(Inc.Resolved));
+ // FIXME: We don't have a fitting value for Kind.
+ HI.Definition =
+ URIForFile::canonicalize(Inc.Resolved, AST.tuPath()).file().str();
+ HI.DefinitionLanguage = "";
+ return HI;
}
// To be used as a backup for highlighting the selected token, we use back as
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
