kadircet created this revision.
kadircet added reviewers: ilya-biryukov, sammccall.
Herald added subscribers: cfe-commits, arphaman, mgrang, jkorous, MaskRay,
ioeric.
Herald added a project: clang.
Clangd was reporting implicit symbols, like results of implicit cast
expressions during code navigation, which is not desired. For example:
struct Foo{ Foo(int); };
void bar(Foo);
vod foo() {
int x;
bar(^x);
}
Performing a GoTo on the point specified by ^ would give two results one
pointing to line `int x` and the other for definition of `Foo(int);`
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D58495
Files:
clangd/XRefs.cpp
unittests/clangd/SymbolInfoTests.cpp
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
===================================================================
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -406,6 +406,16 @@
double y = va^r<int>;
)cpp",
+
+ R"cpp(// No implicit constructors
+ class X {
+ X(X&& x) = default;
+ };
+ X [[makeX]]() {}
+ void foo() {
+ auto x = m^akeX();
+ }
+ )cpp",
};
for (const char *Test : Tests) {
Annotations T(Test);
@@ -457,16 +467,12 @@
)cpp");
auto AST = TestTU::withCode(T.code()).build();
// Ordered assertions are deliberate: we expect a predictable order.
- EXPECT_THAT(locateSymbolAt(AST, T.point("1")),
- ElementsAre(Sym("str"), Sym("Foo")));
+ EXPECT_THAT(locateSymbolAt(AST, T.point("1")), ElementsAre(Sym("str")));
EXPECT_THAT(locateSymbolAt(AST, T.point("2")), ElementsAre(Sym("str")));
- EXPECT_THAT(locateSymbolAt(AST, T.point("3")),
- ElementsAre(Sym("f"), Sym("Foo")));
+ EXPECT_THAT(locateSymbolAt(AST, T.point("3")), ElementsAre(Sym("f")));
EXPECT_THAT(locateSymbolAt(AST, T.point("4")), ElementsAre(Sym("g")));
- EXPECT_THAT(locateSymbolAt(AST, T.point("5")),
- ElementsAre(Sym("f"), Sym("Foo")));
- EXPECT_THAT(locateSymbolAt(AST, T.point("6")),
- ElementsAre(Sym("str"), Sym("Foo"), Sym("Foo")));
+ EXPECT_THAT(locateSymbolAt(AST, T.point("5")), ElementsAre(Sym("f")));
+ EXPECT_THAT(locateSymbolAt(AST, T.point("6")), ElementsAre(Sym("str")));
}
TEST(LocateSymbol, RelPathsInCompileCommand) {
Index: unittests/clangd/SymbolInfoTests.cpp
===================================================================
--- unittests/clangd/SymbolInfoTests.cpp
+++ unittests/clangd/SymbolInfoTests.cpp
@@ -180,12 +180,8 @@
func_baz1(f^f);
}
)cpp",
- {
- CreateExpectedSymbolDetails(
- "ff", "func_baz2", "c:TestTU.cpp@218@F@func_baz2#@ff"),
- CreateExpectedSymbolDetails(
- "bar", "bar::", "c:@S@bar@F@bar#&1$@S@foo#"),
- }},
+ {CreateExpectedSymbolDetails(
+ "ff", "func_baz2", "c:TestTU.cpp@218@F@func_baz2#@ff")}},
{
R"cpp( // Type reference - declaration
struct foo;
Index: clangd/XRefs.cpp
===================================================================
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -123,7 +123,7 @@
// explicitly in the code.
// True means the declaration is explicitly referenced at least once; false
// otherwise.
- llvm::DenseMap<const Decl *, bool> Decls;
+ llvm::DenseSet<const Decl *> Decls;
const SourceLocation &SearchedLocation;
const ASTContext &AST;
Preprocessor &PP;
@@ -133,22 +133,14 @@
ASTContext &AST, Preprocessor &PP)
: SearchedLocation(SearchedLocation), AST(AST), PP(PP) {}
- // Get all DeclInfo of the found declarations.
- // The results are sorted by "IsReferencedExplicitly" and declaration
- // location.
- std::vector<DeclInfo> getFoundDecls() const {
- std::vector<DeclInfo> Result;
- for (auto It : Decls) {
- Result.emplace_back();
- Result.back().D = It.first;
- Result.back().IsReferencedExplicitly = It.second;
- }
+ // The results are sorted by declaration location.
+ std::vector<const Decl *> getFoundDecls() const {
+ std::vector<const Decl *> Result;
+ for (const Decl *D : Decls)
+ Result.push_back(D);
- // Sort results. Declarations being referenced explicitly come first.
- llvm::sort(Result, [](const DeclInfo &L, const DeclInfo &R) {
- if (L.IsReferencedExplicitly != R.IsReferencedExplicitly)
- return L.IsReferencedExplicitly > R.IsReferencedExplicitly;
- return L.D->getBeginLoc() < R.D->getBeginLoc();
+ llvm::sort(Result, [](const Decl *L, const Decl *R) {
+ return L->getBeginLoc() < R->getBeginLoc();
});
return Result;
}
@@ -185,16 +177,17 @@
return isa<ImplicitCastExpr>(E);
};
- bool IsExplicit = !IsImplicitExpr(ASTNode.OrigE);
+ if (IsImplicitExpr(ASTNode.OrigE))
+ return true;
// Find and add definition declarations (for GoToDefinition).
// We don't use parameter `D`, as Parameter `D` is the canonical
// declaration, which is the first declaration of a redeclarable
// declaration, and it could be a forward declaration.
if (const auto *Def = getDefinition(D)) {
- Decls[Def] |= IsExplicit;
+ Decls.insert(Def);
} else {
// Couldn't find a definition, fall back to use `D`.
- Decls[D] |= IsExplicit;
+ Decls.insert(D);
}
}
return true;
@@ -232,7 +225,7 @@
};
struct IdentifiedSymbol {
- std::vector<DeclInfo> Decls;
+ std::vector<const Decl *> Decls;
std::vector<MacroDecl> Macros;
};
@@ -329,8 +322,7 @@
llvm::DenseMap<SymbolID, size_t> ResultIndex;
// Emit all symbol locations (declaration or definition) from AST.
- for (const DeclInfo &DI : Symbols.Decls) {
- const Decl *D = DI.D;
+ for (const Decl *D : Symbols.Decls) {
auto Loc = makeLocation(AST, findNameLoc(D), *MainFilePath);
if (!Loc)
continue;
@@ -456,11 +448,7 @@
const SourceManager &SM = AST.getASTContext().getSourceManager();
auto Symbols = getSymbolAtPosition(
AST, getBeginningOfIdentifier(AST, Pos, SM.getMainFileID()));
- std::vector<const Decl *> TargetDecls;
- for (const DeclInfo &DI : Symbols.Decls) {
- TargetDecls.push_back(DI.D);
- }
- auto References = findRefs(TargetDecls, AST);
+ auto References = findRefs(Symbols.Decls, AST);
std::vector<DocumentHighlight> Result;
for (const auto &Ref : References) {
@@ -714,7 +702,7 @@
return getHoverContents(Symbols.Macros[0].Name);
if (!Symbols.Decls.empty())
- return getHoverContents(Symbols.Decls[0].D);
+ return getHoverContents(Symbols.Decls[0]);
auto DeducedType = getDeducedType(AST, SourceLocationBeg);
if (DeducedType && !DeducedType->isNull())
@@ -738,15 +726,9 @@
auto Loc = getBeginningOfIdentifier(AST, Pos, SM.getMainFileID());
auto Symbols = getSymbolAtPosition(AST, Loc);
- std::vector<const Decl *> TargetDecls;
- for (const DeclInfo &DI : Symbols.Decls) {
- if (DI.IsReferencedExplicitly)
- TargetDecls.push_back(DI.D);
- }
-
// We traverse the AST to find references in the main file.
// TODO: should we handle macros, too?
- auto MainFileRefs = findRefs(TargetDecls, AST);
+ auto MainFileRefs = findRefs(Symbols.Decls, AST);
for (const auto &Ref : MainFileRefs) {
Location Result;
Result.range = getTokenRange(AST, Ref.Loc);
@@ -759,7 +741,7 @@
RefsRequest Req;
Req.Limit = Limit;
- for (const Decl *D : TargetDecls) {
+ for (const Decl *D : Symbols.Decls) {
// Not all symbols can be referenced from outside (e.g. function-locals).
// TODO: we could skip TU-scoped symbols here (e.g. static functions) if
// we know this file isn't a header. The details might be tricky.
@@ -790,9 +772,9 @@
std::vector<SymbolDetails> Results;
- for (const auto &Sym : Symbols.Decls) {
+ for (const Decl *D : Symbols.Decls) {
SymbolDetails NewSymbol;
- if (const NamedDecl *ND = dyn_cast<NamedDecl>(Sym.D)) {
+ if (const NamedDecl *ND = dyn_cast<NamedDecl>(D)) {
std::string QName = printQualifiedName(*ND);
std::tie(NewSymbol.containerName, NewSymbol.name) =
splitQualifiedName(QName);
@@ -804,7 +786,7 @@
}
}
llvm::SmallString<32> USR;
- if (!index::generateUSRForDecl(Sym.D, USR)) {
+ if (!index::generateUSRForDecl(D, USR)) {
NewSymbol.USR = USR.str();
NewSymbol.ID = SymbolID(NewSymbol.USR);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
