[clangd] Don't show base class versions of members as completions.

[sammccall]

Summary

These are available via qualifiers, but signal to noise level is low.
Keep required quailifier machinery around though, for cross-ns completion.

[ioeric]

Keep required quailifier machinery around though, for cross-ns completion.

Do we have cross-ns completion in sema?

clangd/CodeComplete.cpp

732

Are there hidden results that are neither declarations nor class members and do not require qualifier?

[sammccall]

In D53571#1274082, @ioeric wrote:

Keep required quailifier machinery around though, for cross-ns completion.

Do we have cross-ns completion in sema?

No, I don't think so. I meant RequiredQualifier etc.
Unfortunately I've got no idea whether there are !QualifierIsInformative cases in Sema we still care about, so I don't know whether that logic is safe to remove...

clangd/CodeComplete.cpp

732

I don't know. I thought a namespace-scope thing shadowed by a member was such a case, but it doesn't seem to generate results at all.

But that (hypothetical) example illustrates a point: I do think we would want to show ns::foo() if it was shadowed by MyClass()::foo(). The difference is that they're probably fundamentally different things that share a name, while MySubClass::foo() is usually just a strictly better version of MyBaseClass::foo() as far as a caller is concerned.

[sammccall]

Remove misleading change to test.

[ioeric]

The diff seems to be wrong. Please rebase.

clangd/CodeComplete.cpp

732

Fair enough. Thanks!

[sammccall]

Rebase

clangd/CodeComplete.cpp

void ProcessCodeCompleteResults(class Sema &S, CodeCompletionContext Context,
     }
     // Record the completion context.
     CCSema = &S;
     CCContext = Context;
 
     // Retain the results we might want.
     for (unsigned I = 0; I < NumResults; ++I) {
       auto &Result = InResults[I];
-      // Drop hidden items which cannot be found by lookup after completion.
-      // Exception: some items can be named by using a qualifier.
-      if (Result.Hidden && (!Result.Qualifier || Result.QualifierIsInformative))
+      // Class members that are shadowed by subclasses are usually noise.
+      if (Result.Hidden && Result.Declaration &&
+          Result.Declaration->isCXXClassMember())
         continue;
       if (!Opts.IncludeIneligibleResults &&
           (Result.Availability == CXAvailability_NotAvailable ||
            Result.Availability == CXAvailability_NotAccessible))
         continue;
       if (Result.Declaration &&
           !Context.getBaseType().isNull() // is this a member-access context?
           && isBlacklistedMember(*Result.Declaration))

unittests/clangd/CodeCompleteTests.cpp

auto External = completions(R"cpp(
       }
   )cpp");
   EXPECT_THAT(External.Completions,
               AllOf(Has("pub"), Not(Has("prot")), Not(Has("priv"))));
 }
 
 TEST(CompletionTest, Qualifiers) {
   auto Results = completions(R"cpp(
+    namespace x {
+      int foo();
       class Foo {
         public: int foo() const;
         int bar() const;
       };
       class Bar : public Foo {
         int foo() const;
       };
       void test() { Bar().^ }
+    }
   )cpp");
   EXPECT_THAT(Results.Completions,
-              HasSubsequence(AllOf(Qualifier(""), Named("bar")),
-                             AllOf(Qualifier("Foo::"), Named("foo"))));
+              Contains(AllOf(Qualifier(""), Named("bar"))));
+  // Hidden members are not shown.
+  EXPECT_THAT(Results.Completions,
+              Not(Contains(AllOf(Qualifier("Foo::"), Named("foo")))));
+  // Private members are not shown.
   EXPECT_THAT(Results.Completions,
-              Not(Contains(AllOf(Qualifier(""), Named("foo"))))); // private
+              Not(Contains(AllOf(Qualifier(""), Named("foo")))));
 }
 
 TEST(CompletionTest, InjectedTypename) {
   // These are suppressed when accessed as a member...
   EXPECT_THAT(completions("struct X{}; void foo(){ X().^ }").Completions,
               Not(Has("X")));
   EXPECT_THAT(completions("struct X{ void foo(){ this->^ } };").Completions,
               Not(Has("X")));