Index: clangd/CodeComplete.h =================================================================== --- clangd/CodeComplete.h +++ clangd/CodeComplete.h @@ -131,6 +131,9 @@ /// Holds the range of the token we are going to replace with this completion. Range CompletionTokenRange; + /// Whether this completion is for overriding a virtual method. + bool IsOverride = false; + // Scores are used to rank completion items. struct Scores { // The score that items are ranked by. Index: clangd/CodeComplete.cpp =================================================================== --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -188,6 +188,56 @@ return HeaderFile{std::move(*Resolved), /*Verbatim=*/false}; } +// First traverses all method definitions inside current class/struct/union +// definition. Than traverses base classes to find virtual methods that haven't +// been overriden within current context. +// FIXME(kadircet): Currently we cannot see declarations below completion point. +// It is because Sema gets run only upto completion point. Need to find a +// solution to run it for the whole class/struct/union definition. +static std::vector +getNonOverridenMethodCompletionResults(const DeclContext *DC, Sema *S) { + const auto *CR = llvm::dyn_cast(DC); + // If not inside a class/struct/union return empty. + if (!CR) + return {}; + // First store overrides within current class. + // These are stored by name to make querying fast in the later step. + llvm::StringMap> Overrides; + for (auto *Method : CR->methods()) { + if (!Method->isVirtual()) + continue; + const std::string Name = Method->getNameAsString(); + Overrides[Name].push_back(Method); + } + + std::vector Results; + for (const auto &Base : CR->bases()) { + const auto *BR = Base.getType().getTypePtr()->getAsCXXRecordDecl(); + if (!BR) + continue; + for (auto *Method : BR->methods()) { + if (!Method->isVirtual()) + continue; + const auto it = Overrides.find(Method->getName()); + bool IsOverriden = false; + if (it != Overrides.end()) { + for (auto *MD : it->second) { + // If the method in current body is not an overload of this virtual + // function, that it overrides this one. + if (!S->IsOverload(MD, Method, false)) { + IsOverriden = true; + break; + } + } + } + if (!IsOverriden) + Results.emplace_back(Method, 0); + } + } + + return Results; +} + /// A code completion result, in clang-native form. /// It may be promoted to a CompletionItem if it's among the top-ranked results. struct CompletionCandidate { @@ -196,6 +246,9 @@ const CodeCompletionResult *SemaResult = nullptr; const Symbol *IndexResult = nullptr; + // States whether this item is an override suggestion. + bool IsOverride = false; + // Returns a token identifying the overload set this is part of. // 0 indicates it's not part of any overload set. size_t overloadSet() const { @@ -351,6 +404,8 @@ Completion.Documentation = getDocComment(ASTCtx, *C.SemaResult, /*CommentsFromHeader=*/false); } + if (C.IsOverride) + S.OverrideSuffix = "override"; } CodeCompletion build() { @@ -358,6 +413,7 @@ Completion.Signature = summarizeSignature(); Completion.SnippetSuffix = summarizeSnippet(); Completion.BundleSize = Bundled.size(); + Completion.IsOverride = summarizeOverride(); return std::move(Completion); } @@ -366,6 +422,7 @@ std::string SnippetSuffix; std::string Signature; std::string ReturnType; + std::string OverrideSuffix; }; // If all BundledEntrys have the same value for a property, return it. @@ -398,6 +455,12 @@ return "(…)"; } + bool summarizeOverride() const { + if (auto *OverrideSuffix = onlyValue<&BundledEntry::OverrideSuffix>()) + return !OverrideSuffix->empty(); + return false; + } + ASTContext &ASTCtx; CodeCompletion Completion; SmallVector Bundled; @@ -1172,10 +1235,14 @@ auto IndexResults = (Opts.Index && allowIndex(Recorder->CCContext)) ? queryIndex() : SymbolSlab(); - // Merge Sema and Index results, score them, and pick the winners. - auto Top = mergeResults(Recorder->Results, IndexResults); - // Convert the results to final form, assembling the expensive strings. + // Merge Sema, Index and Override results, score them, and pick the + // winners. + const auto Overrides = getNonOverridenMethodCompletionResults( + Recorder->CCSema->CurContext, Recorder->CCSema); + auto Top = mergeResults(Recorder->Results, IndexResults, Overrides); CodeCompleteResult Output; + + // Convert the results to final form, assembling the expensive strings. for (auto &C : Top) { Output.Completions.push_back(toCodeCompletion(C.first)); Output.Completions.back().Score = C.second; @@ -1183,6 +1250,7 @@ } Output.HasMore = Incomplete; Output.Context = Recorder->CCContext.getKind(); + return Output; } @@ -1209,20 +1277,24 @@ return std::move(ResultsBuilder).build(); } - // Merges Sema and Index results where possible, to form CompletionCandidates. - // Groups overloads if desired, to form CompletionCandidate::Bundles. - // The bundles are scored and top results are returned, best to worst. + // Merges Sema, Index and Override results where possible, to form + // CompletionCandidates. Groups overloads if desired, to form + // CompletionCandidate::Bundles. The bundles are scored and top results are + // returned, best to worst. std::vector - mergeResults(const std::vector &SemaResults, - const SymbolSlab &IndexResults) { + mergeResults(const std::vector &SemaResults, + const SymbolSlab &IndexResults, + const std::vector &OverrideResults) { trace::Span Tracer("Merge and score results"); std::vector Bundles; llvm::DenseMap BundleLookup; auto AddToBundles = [&](const CodeCompletionResult *SemaResult, - const Symbol *IndexResult) { + const Symbol *IndexResult, + bool IsOverride = false) { CompletionCandidate C; C.SemaResult = SemaResult; C.IndexResult = IndexResult; + C.IsOverride = IsOverride; C.Name = IndexResult ? IndexResult->Name : Recorder->getName(*SemaResult); if (auto OverloadSet = Opts.BundleOverloads ? C.overloadSet() : 0) { auto Ret = BundleLookup.try_emplace(OverloadSet, Bundles.size()); @@ -1249,6 +1321,13 @@ // Emit all Sema results, merging them with Index results if possible. for (auto &SemaResult : Recorder->Results) AddToBundles(&SemaResult, CorrespondingIndexResult(SemaResult)); + // Handle OverrideResults the same way we deal with SemaResults. Since these + // results use the same structs as a SemaResult it is safe to do that, but + // we need to make sure we dont' duplicate things in future if Sema starts + // to provide them as well. + for (auto &OverrideResult : OverrideResults) + AddToBundles(&OverrideResult, CorrespondingIndexResult(OverrideResult), + true); // Now emit any Index-only results. for (const auto &IndexResult : IndexResults) { if (UsedIndexResults.count(&IndexResult)) @@ -1387,7 +1466,8 @@ LSP.label = (HeaderInsertion ? Opts.IncludeIndicator.Insert : Opts.IncludeIndicator.NoInsert) + (Opts.ShowOrigins ? "[" + llvm::to_string(Origin) + "]" : "") + - RequiredQualifier + Name + Signature; + (IsOverride ? ReturnType + " " : "") + RequiredQualifier + Name + + Signature + (IsOverride ? " override" : ""); LSP.kind = Kind; LSP.detail = BundleSize > 1 ? llvm::formatv("[{0} overloads]", BundleSize) @@ -1397,7 +1477,10 @@ LSP.documentation = Documentation; LSP.sortText = sortText(Score.Total, Name); LSP.filterText = Name; - LSP.textEdit = {CompletionTokenRange, RequiredQualifier + Name}; + LSP.textEdit = {CompletionTokenRange, + IsOverride ? llvm::formatv("{0} {1}{2} override", ReturnType, + Name, Signature) + : RequiredQualifier + Name}; // Merge continious additionalTextEdits into main edit. The main motivation // behind this is to help LSP clients, it seems most of them are confused when // they are provided with additionalTextEdits that are consecutive to main Index: unittests/clangd/CodeCompleteTests.cpp =================================================================== --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -61,6 +61,7 @@ MATCHER(InsertInclude, "") { return bool(arg.HeaderInsertion); } MATCHER_P(SnippetSuffix, Text, "") { return arg.SnippetSuffix == Text; } MATCHER_P(Origin, OriginSet, "") { return arg.Origin == OriginSet; } +MATCHER(IsOverride, "") { return bool(arg.IsOverride); } // Shorthand for Contains(Named(Name)). Matcher &> Has(std::string Name) { @@ -1700,6 +1701,58 @@ } } +TEST(CompletionTest, SuggestOverrides) { + constexpr const char *const Text(R"cpp( + class A { + public: + virtual void vfunc(bool param); + virtual void vfunc(bool param, int p); + void func(bool param); + }; + class B : public A { + virtual void ttt(bool param); + void vfunc(bool param, int p) override; + }; + class C : public B { + public: + void vfunc(bool param) override; + ^ + }; + )cpp"); + const auto Results = completions(Text); + EXPECT_THAT(Results.Completions, + AllOf(Contains(AllOf(Named("vfunc"), IsOverride(), + Labeled("vfunc(bool param, int p)"))), + Contains(AllOf(Named("ttt"), IsOverride(), + Labeled("ttt(bool param)"))), + Not(Contains(AllOf(Named("vfunc"), IsOverride(), + Labeled("vfunc(bool param)")))))); +} + +TEST(CompletionTest, RenderOverride) { + CodeCompletion C; + C.Name = "x"; + C.Signature = "(bool) const"; + C.SnippetSuffix = "(${0:bool})"; + C.ReturnType = "int"; + C.Documentation = "This is x()."; + C.Kind = CompletionItemKind::Method; + C.Score.Total = 1.0; + C.Origin = SymbolOrigin::AST; + C.IsOverride = true; + + CodeCompleteOptions Opts; + Opts.IncludeIndicator.NoInsert = ""; + auto R = C.render(Opts); + EXPECT_EQ(R.label, "int x(bool) const override"); + EXPECT_EQ(R.insertText, "int x(bool) const override"); + EXPECT_EQ(R.insertTextFormat, InsertTextFormat::PlainText); + EXPECT_EQ(R.filterText, "x"); + EXPECT_EQ(R.detail, "int"); + EXPECT_EQ(R.documentation, "This is x()."); + EXPECT_THAT(R.additionalTextEdits, IsEmpty()); +} + } // namespace } // namespace clangd } // namespace clang