Author: ioeric Date: Mon Jun 4 04:31:55 2018 New Revision: 333885 URL: http://llvm.org/viewvc/llvm-project?rev=333885&view=rev Log: [clangd] Avoid indexing decls associated with friend decls.
Summary: These decls are sometime used as the canonical declarations (e.g. for go-to-def), which seems to be bad. - friend decls that are not definitions should be ignored for indexing purposes - this means they should never be selected as canonical decl - if the friend decl is the only decl, then the symbol should not be indexed Reviewers: sammccall Reviewed By: sammccall Subscribers: mgorny, klimek, ilya-biryukov, MaskRay, jkorous, cfe-commits Differential Revision: https://reviews.llvm.org/D47623 Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp clang-tools-extra/trunk/clangd/index/SymbolCollector.h clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=333885&r1=333884&r2=333885&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original) +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Mon Jun 4 04:31:55 2018 @@ -290,6 +290,19 @@ bool SymbolCollector::handleDeclOccurenc index::IndexDataConsumer::ASTNodeInfo ASTNode) { assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set."); assert(CompletionAllocator && CompletionTUInfo); + assert(ASTNode.OrigD); + // If OrigD is an declaration associated with a friend declaration and it's + // not a definition, skip it. Note that OrigD is the occurrence that the + // collector is currently visiting. + if ((ASTNode.OrigD->getFriendObjectKind() != + Decl::FriendObjectKind::FOK_None) && + !(Roles & static_cast<unsigned>(index::SymbolRole::Definition))) + return true; + // A declaration created for a friend declaration should not be used as the + // canonical declaration in the index. Use OrigD instead, unless we've already + // picked a replacement for D + if (D->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None) + D = CanonicalDecls.try_emplace(D, ASTNode.OrigD).first->second; const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(D); if (!ND) return true; Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.h?rev=333885&r1=333884&r2=333885&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolCollector.h (original) +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h Mon Jun 4 04:31:55 2018 @@ -80,6 +80,12 @@ private: Options Opts; // Decls referenced from the current TU, flushed on finish(). llvm::DenseSet<const NamedDecl *> ReferencedDecls; + // Maps canonical declaration provided by clang to canonical declaration for + // an index symbol, if clangd prefers a different declaration than that + // provided by clang. For example, friend declaration might be considered + // canonical by clang but should not be considered canonical in the index + // unless it's a definition. + llvm::DenseMap<const Decl *, const Decl *> CanonicalDecls; }; } // namespace clangd Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=333885&r1=333884&r2=333885&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Mon Jun 4 04:31:55 2018 @@ -812,6 +812,48 @@ TEST_F(SymbolCollectorTest, DoubleCheckP QName("nx::Kind"), QName("nx::Kind_Fine"))); } +TEST_F(SymbolCollectorTest, DoNotIndexSymbolsInFriendDecl) { + Annotations Header(R"( + namespace nx { + class $z[[Z]] {}; + class X { + friend class Y; + friend class Z; + friend void foo(); + friend void $bar[[bar]]() {} + }; + class $y[[Y]] {}; + void $foo[[foo]](); + } + )"); + runSymbolCollector(Header.code(), /*Main=*/""); + + EXPECT_THAT(Symbols, + UnorderedElementsAre( + QName("nx"), QName("nx::X"), + AllOf(QName("nx::Y"), DeclRange(Header.range("y"))), + AllOf(QName("nx::Z"), DeclRange(Header.range("z"))), + AllOf(QName("nx::foo"), DeclRange(Header.range("foo"))), + AllOf(QName("nx::bar"), DeclRange(Header.range("bar"))))); +} + +TEST_F(SymbolCollectorTest, ReferencesInFriendDecl) { + const std::string Header = R"( + class X; + class Y; + )"; + const std::string Main = R"( + class C { + friend ::X; + friend class Y; + }; + )"; + CollectorOpts.CountReferences = true; + runSymbolCollector(Header, Main); + EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), Refs(1)), + AllOf(QName("Y"), Refs(1)))); +} + } // namespace } // namespace clangd } // namespace clang _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
