hokein created this revision.
hokein added a reviewer: kbobyrev.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
DeclarationName for cxx constructor is special, it is not an identifier.
thus the "Spelled" flag are not set for all ctor references, this patch
fixes it.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D74125
Files:
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -700,39 +700,70 @@
EXPECT_THAT(Refs, IsEmpty());
}
-TEST_F(SymbolCollectorTest, SpelledReference) {
- Annotations Header(R"cpp(
- struct Foo;
- #define MACRO Foo
- )cpp");
- Annotations Main(R"cpp(
- struct $spelled[[Foo]] {
- $spelled[[Foo]]();
- ~$spelled[[Foo]]();
+TEST_F(SymbolCollectorTest, SpelledReferences) {
+ struct {
+ llvm::StringRef Header;
+ llvm::StringRef Main;
+ llvm::StringRef TargetSymbolName;
+ } TestCases[] = {
+ {
+ R"cpp(
+ struct Foo;
+ #define MACRO Foo
+ )cpp",
+ R"cpp(
+ struct $spelled[[Foo]] {
+ $spelled[[Foo]]();
+ ~$spelled[[Foo]]();
+ };
+ $spelled[[Foo]] Variable1;
+ $implicit[[MACRO]] Variable2;
+ )cpp",
+ "Foo",
+ },
+ {
+ R"cpp(
+ class Foo {
+ public:
+ Foo() = default;
+ };
+ )cpp",
+ R"cpp(
+ void f() { Foo $implicit[[f]]; f = $spelled[[Foo]]();}
+ )cpp",
+ "Foo::Foo" /// constructor.
+ },
};
- $spelled[[Foo]] Variable1;
- $implicit[[MACRO]] Variable2;
- )cpp");
CollectorOpts.RefFilter = RefKind::All;
CollectorOpts.RefsInHeaders = false;
- runSymbolCollector(Header.code(), Main.code());
- const auto SpelledRanges = Main.ranges("spelled");
- const auto ImplicitRanges = Main.ranges("implicit");
- RefSlab::Builder SpelledSlabBuilder, ImplicitSlabBuilder;
- for (const auto &SymbolAndRefs : Refs) {
- const auto Symbol = SymbolAndRefs.first;
- for (const auto &Ref : SymbolAndRefs.second)
- if ((Ref.Kind & RefKind::Spelled) != RefKind::Unknown)
- SpelledSlabBuilder.insert(Symbol, Ref);
- else
- ImplicitSlabBuilder.insert(Symbol, Ref);
+ for (const auto& T : TestCases) {
+ Annotations Header(T.Header);
+ Annotations Main(T.Main);
+ // Reset the file system.
+ InMemoryFileSystem = new llvm::vfs::InMemoryFileSystem;
+ runSymbolCollector(Header.code(), Main.code());
+
+ const auto SpelledRanges = Main.ranges("spelled");
+ const auto ImplicitRanges = Main.ranges("implicit");
+ RefSlab::Builder SpelledSlabBuilder, ImplicitSlabBuilder;
+ const auto TargetID = findSymbol(Symbols, T.TargetSymbolName).ID;
+ for (const auto &SymbolAndRefs : Refs) {
+ const auto ID = SymbolAndRefs.first;
+ if (!(ID == TargetID))
+ continue;
+ for (const auto &Ref : SymbolAndRefs.second)
+ if ((Ref.Kind & RefKind::Spelled) != RefKind::Unknown)
+ SpelledSlabBuilder.insert(ID, Ref);
+ else
+ ImplicitSlabBuilder.insert(ID, Ref);
+ }
+ const auto SpelledRefs = std::move(SpelledSlabBuilder).build(),
+ ImplicitRefs = std::move(ImplicitSlabBuilder).build();
+ EXPECT_THAT(SpelledRefs,
+ Contains(Pair(TargetID, HaveRanges(SpelledRanges))));
+ EXPECT_THAT(ImplicitRefs,
+ Contains(Pair(TargetID, HaveRanges(ImplicitRanges))));
}
- const auto SpelledRefs = std::move(SpelledSlabBuilder).build(),
- ImplicitRefs = std::move(ImplicitSlabBuilder).build();
- EXPECT_THAT(SpelledRefs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
- HaveRanges(SpelledRanges))));
- EXPECT_THAT(ImplicitRefs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
- HaveRanges(ImplicitRanges))));
}
TEST_F(SymbolCollectorTest, NameReferences) {
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -574,12 +574,18 @@
// FIXME: All MacroRefs are marked as Spelled now, but this should be checked.
for (const auto &IDAndRefs : MacroRefs)
for (const auto &LocAndRole : IDAndRefs.second)
- CollectRef(IDAndRefs.first, LocAndRole);
+ CollectRef(IDAndRefs.first, LocAndRole, /*Spelled=*/true);
// Populate Refs slab from DeclRefs.
llvm::DenseMap<FileID, std::vector<syntax::Token>> FilesToTokensCache;
for (auto &DeclAndRef : DeclRefs) {
if (auto ID = getSymbolID(DeclAndRef.first)) {
for (auto &LocAndRole : DeclAndRef.second) {
+ DeclarationName Name = DeclAndRef.first->getDeclName();
+ auto NameKind = Name.getNameKind();
+ if (NameKind != DeclarationName::Identifier &&
+ NameKind != DeclarationName::CXXConstructorName)
+ continue;
+
const auto FileID = SM.getFileID(LocAndRole.first);
// FIXME: It's better to use TokenBuffer by passing spelled tokens from
// the caller of SymbolCollector.
@@ -591,9 +597,8 @@
// corresponding NamedDecl is. If it is, mark this reference as spelled.
const auto *IdentifierToken =
spelledIdentifierTouching(LocAndRole.first, Tokens);
- DeclarationName Name = DeclAndRef.first->getDeclName();
- bool Spelled = IdentifierToken && Name.isIdentifier() &&
- Name.getAsString() == IdentifierToken->text(SM);
+ bool Spelled =
+ IdentifierToken && Name.getAsString() == IdentifierToken->text(SM);
CollectRef(*ID, LocAndRole, Spelled);
}
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
