Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp =================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.cpp +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp @@ -467,6 +467,25 @@ llvm_unreachable("unknown code completion context"); } +// Some member calls are blacklisted because they're so rarely useful. +static bool isBlacklistedMember(const NamedDecl &D) { + // Destructor completion is rarely useful, and works inconsistently. + // (s.^ completes ~string, but s.~st^ is an error). + if (D.getKind() == Decl::CXXDestructor) + return true; + // Injected name may be useful for A::foo(), but who writes A::A::foo()? + if (auto *R = dyn_cast_or_null(&D)) + if (R->isInjectedClassName()) + return true; + // Explicit calls to operators are also rare. + auto NameKind = D.getDeclName().getNameKind(); + if (NameKind == DeclarationName::CXXOperatorName || + NameKind == DeclarationName::CXXLiteralOperatorName || + NameKind == DeclarationName::CXXConversionFunctionName) + return true; + return false; +} + // The CompletionRecorder captures Sema code-complete output, including context. // It filters out ignored results (but doesn't apply fuzzy-filtering yet). // It doesn't do scoring or conversion to CompletionItem yet, as we want to @@ -520,9 +539,9 @@ (Result.Availability == CXAvailability_NotAvailable || Result.Availability == CXAvailability_NotAccessible)) continue; - // Destructor completion is rarely useful, and works inconsistently. - // (s.^ completes ~string, but s.~st^ is an error). - if (dyn_cast_or_null(Result.Declaration)) + if (Result.Declaration && + !Context.getBaseType().isNull() // is this a member-access context? + && isBlacklistedMember(*Result.Declaration)) continue; // We choose to never append '::' to completion results in clangd. Result.StartsNestedNameSpecifier = false; Index: clang-tools-extra/trunk/clangd/Quality.h =================================================================== --- clang-tools-extra/trunk/clangd/Quality.h +++ clang-tools-extra/trunk/clangd/Quality.h @@ -44,9 +44,6 @@ /// Attributes of a symbol that affect how much we like it. struct SymbolQualitySignals { - unsigned SemaCCPriority = 0; // 1-80, 1 is best. 0 means absent. - // FIXME: this is actually a mix of symbol - // quality and relevance. Untangle this. bool Deprecated = false; unsigned References = 0; Index: clang-tools-extra/trunk/clangd/Quality.cpp =================================================================== --- clang-tools-extra/trunk/clangd/Quality.cpp +++ clang-tools-extra/trunk/clangd/Quality.cpp @@ -94,7 +94,6 @@ } void SymbolQualitySignals::merge(const CodeCompletionResult &SemaCCResult) { - SemaCCPriority = SemaCCResult.Priority; if (SemaCCResult.Availability == CXAvailability_Deprecated) Deprecated = true; @@ -117,11 +116,6 @@ if (References >= 3) Score *= std::log(References); - if (SemaCCPriority) - // Map onto a 0-2 interval, so we don't reward/penalize non-Sema results. - // Priority 80 is a really bad score. - Score *= 2 - std::min(80, SemaCCPriority) / 40; - if (Deprecated) Score *= 0.1f; @@ -146,8 +140,6 @@ raw_ostream &operator<<(raw_ostream &OS, const SymbolQualitySignals &S) { OS << formatv("=== Symbol quality: {0}\n", S.evaluate()); - if (S.SemaCCPriority) - OS << formatv("\tSemaCCPriority: {0}\n", S.SemaCCPriority); OS << formatv("\tReferences: {0}\n", S.References); OS << formatv("\tDeprecated: {0}\n", S.Deprecated); OS << formatv("\tCategory: {0}\n", static_cast(S.Category)); Index: clang-tools-extra/trunk/test/clangd/completion.test =================================================================== --- clang-tools-extra/trunk/test/clangd/completion.test +++ clang-tools-extra/trunk/test/clangd/completion.test @@ -18,8 +18,8 @@ # CHECK-NEXT: "kind": 5, # CHECK-NEXT: "label": "a", # CHECK-NEXT: "sortText": "{{.*}}a" -# CHECK-NEXT: }, -# CHECK: ] +# CHECK-NEXT: } +# CHECK-NEXT: ] --- # Update the source file and check for completions again. {"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"test:///main.cpp","version":2},"contentChanges":[{"text":"struct S { int b; };\nint main() {\nS().\n}"}]}} @@ -38,7 +38,7 @@ # CHECK-NEXT: "kind": 5, # CHECK-NEXT: "label": "b", # CHECK-NEXT: "sortText": "{{.*}}b" -# CHECK-NEXT: }, -# CHECK: ] +# CHECK-NEXT: } +# CHECK-NEXT: ] --- {"jsonrpc":"2.0","id":4,"method":"shutdown"} Index: clang-tools-extra/trunk/test/clangd/protocol.test =================================================================== --- clang-tools-extra/trunk/test/clangd/protocol.test +++ clang-tools-extra/trunk/test/clangd/protocol.test @@ -34,11 +34,11 @@ # CHECK-NEXT: "result": { # CHECK-NEXT: "isIncomplete": false, # CHECK-NEXT: "items": [ -# CHECK: "filterText": "fake", -# CHECK-NEXT: "insertText": "fake", +# CHECK: "filterText": "a", +# CHECK-NEXT: "insertText": "a", # CHECK-NEXT: "insertTextFormat": 1, -# CHECK-NEXT: "kind": 7, -# CHECK-NEXT: "label": "fake", +# CHECK-NEXT: "kind": 5, +# CHECK-NEXT: "label": "a", # CHECK-NEXT: "sortText": "{{.*}}" # CHECK: ] # CHECK-NEXT: } @@ -63,11 +63,11 @@ # CHECK-NEXT: "result": { # CHECK-NEXT: "isIncomplete": false, # CHECK-NEXT: "items": [ -# CHECK: "filterText": "fake", -# CHECK-NEXT: "insertText": "fake", +# CHECK: "filterText": "a", +# CHECK-NEXT: "insertText": "a", # CHECK-NEXT: "insertTextFormat": 1, -# CHECK-NEXT: "kind": 7, -# CHECK-NEXT: "label": "fake", +# CHECK-NEXT: "kind": 5, +# CHECK-NEXT: "label": "a", # CHECK-NEXT: "sortText": "{{.*}}" # CHECK: ] # CHECK-NEXT: } @@ -92,11 +92,11 @@ # CHECK-NEXT: "result": { # CHECK-NEXT: "isIncomplete": false, # CHECK-NEXT: "items": [ -# CHECK: "filterText": "fake", -# CHECK-NEXT: "insertText": "fake", +# CHECK: "filterText": "a", +# CHECK-NEXT: "insertText": "a", # CHECK-NEXT: "insertTextFormat": 1, -# CHECK-NEXT: "kind": 7, -# CHECK-NEXT: "label": "fake", +# CHECK-NEXT: "kind": 5, +# CHECK-NEXT: "label": "a", # CHECK-NEXT: "sortText": "{{.*}}" # CHECK: ] # CHECK-NEXT: } Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp @@ -207,9 +207,6 @@ EXPECT_THAT(completions(Body + "int main() { S().FR^ }").items, AllOf(Has("FooBar"), Not(Has("FooBaz")), Not(Has("Qux")))); - EXPECT_THAT(completions(Body + "int main() { S().opr^ }").items, - Has("operator=")); - EXPECT_THAT(completions(Body + "int main() { aaa^ }").items, AllOf(Has("Abracadabra"), Has("Alakazam"))); @@ -250,9 +247,10 @@ // Class members. The only items that must be present in after-dot // completion. - EXPECT_THAT( - Results.items, - AllOf(Has(Opts.EnableSnippets ? "method()" : "method"), Has("field"))); + EXPECT_THAT(Results.items, + AllOf(Has(Opts.EnableSnippets ? "method()" : "method"), + Has("field"), Not(Has("ClassWithMembers")), + Not(Has("operator=")), Not(Has("~ClassWithMembers")))); EXPECT_IFF(Opts.IncludeIneligibleResults, Results.items, Has("private_field")); // Global items. @@ -379,6 +377,25 @@ EXPECT_THAT(Results.items, Not(Contains(Labeled("foo() const")))); // private } +TEST(CompletionTest, InjectedTypename) { + // These are suppressed when accessed as a member... + EXPECT_THAT(completions("struct X{}; void foo(){ X().^ }").items, + Not(Has("X"))); + EXPECT_THAT(completions("struct X{ void foo(){ this->^ } };").items, + Not(Has("X"))); + // ...but accessible in other, more useful cases. + EXPECT_THAT(completions("struct X{ void foo(){ ^ } };").items, Has("X")); + EXPECT_THAT(completions("struct Y{}; struct X:Y{ void foo(){ ^ } };").items, + Has("Y")); + EXPECT_THAT( + completions( + "template struct Y{}; struct X:Y{ void foo(){ ^ } };") + .items, + Has("Y")); + // This case is marginal (`using X::X` is useful), we allow it for now. + EXPECT_THAT(completions("struct X{}; void foo(){ X::^ }").items, Has("X")); +} + TEST(CompletionTest, Snippets) { clangd::CodeCompleteOptions Opts; Opts.EnableSnippets = true; Index: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp @@ -39,7 +39,6 @@ SymbolQualitySignals Quality; Quality.merge(findSymbol(Symbols, "x")); EXPECT_FALSE(Quality.Deprecated); - EXPECT_EQ(Quality.SemaCCPriority, SymbolQualitySignals().SemaCCPriority); EXPECT_EQ(Quality.References, SymbolQualitySignals().References); EXPECT_EQ(Quality.Category, SymbolQualitySignals::Variable); @@ -48,14 +47,12 @@ Quality = {}; Quality.merge(F); EXPECT_FALSE(Quality.Deprecated); // FIXME: Include deprecated bit in index. - EXPECT_EQ(Quality.SemaCCPriority, SymbolQualitySignals().SemaCCPriority); EXPECT_EQ(Quality.References, 24u); EXPECT_EQ(Quality.Category, SymbolQualitySignals::Function); Quality = {}; Quality.merge(CodeCompletionResult(&findDecl(AST, "f"), /*Priority=*/42)); EXPECT_TRUE(Quality.Deprecated); - EXPECT_EQ(Quality.SemaCCPriority, 42u); EXPECT_EQ(Quality.References, SymbolQualitySignals().References); EXPECT_EQ(Quality.Category, SymbolQualitySignals::Function); } @@ -121,12 +118,6 @@ EXPECT_GT(WithReferences.evaluate(), Default.evaluate()); EXPECT_GT(ManyReferences.evaluate(), WithReferences.evaluate()); - SymbolQualitySignals LowPriority, HighPriority; - LowPriority.SemaCCPriority = 60; - HighPriority.SemaCCPriority = 20; - EXPECT_GT(HighPriority.evaluate(), Default.evaluate()); - EXPECT_LT(LowPriority.evaluate(), Default.evaluate()); - SymbolQualitySignals Variable, Macro; Variable.Category = SymbolQualitySignals::Variable; Macro.Category = SymbolQualitySignals::Macro;