[clang-tools-extra] r333885 - [clangd] Avoid indexing decls associated with friend decls.

[Eric Liu via cfe-commits]

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