This revision was automatically updated to reflect the committed changes.
kadircet marked 2 inline comments as done.
Closed by commit rG3b8fb471cbbd: [clangd][NFCI] Store TUPath inside ParsedAST
(authored by kadircet).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130690/new/
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
