[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE340530: [clangd] Suggest code-completions for overriding base class virtual methods. (authored by kadircet, committed by ). Changed prior to commit: https://reviews.llvm.org/D50898?vs=162157&id=162158#toc Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50898 Files: clangd/CodeComplete.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -1680,6 +1680,31 @@ } } +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) const; + 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(Labeled("void vfunc(bool param, int p) override")), +Contains(Labeled("void ttt(bool param) const override")), +Not(Contains(Labeled("void vfunc(bool param) override"); +} + } // namespace } // namespace clangd } // namespace clang Index: clangd/CodeComplete.cpp === --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -188,14 +188,66 @@ 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; +Overrides[Method->getName()].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 { llvm::StringRef Name; // Used for filtering and sorting. // We may have a result from Sema, from the index, or both. 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 { @@ -352,21 +404,30 @@ Completion.Documentation = getDocComment(ASTCtx, *C.SemaResult, /*CommentsFromHeader=*/false); } +if (C.IsOverride) + S.OverrideSuffix = true; } CodeCompletion build() { Completion.ReturnType = summarizeReturnType(); Completion.Signature = summarizeSignature(); Completion.SnippetSuffix = summarizeSnippet(); Completion.BundleSize = Bundled.size(); +if (summarizeOverride()) { + Completion.Name = Completion.ReturnType + ' ' + +std::move(Completion.Name) + +std::move(Completion.Signature) + " override"; + Completion.Signature.clear(); +} return std::move(Completion); } private: struct BundledEntry { std::string SnippetSuffix; std::string Signature; std
[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.
hokein accepted this revision. hokein added a comment. still LG. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.
kadircet updated this revision to Diff 162157. kadircet added a comment. - Resolve discussions. - Move override generation logic from render to item generation so that internal clients can use it as well, also use a boolean for checking override status of a bundle to be more efficient. - Change unittests accordingly, get rid of unused IsOverride field. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50898 Files: clangd/CodeComplete.cpp unittests/clangd/CodeCompleteTests.cpp Index: unittests/clangd/CodeCompleteTests.cpp === --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -1680,6 +1680,31 @@ } } +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) const; + 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(Labeled("void vfunc(bool param, int p) override")), +Contains(Labeled("void ttt(bool param) const override")), +Not(Contains(Labeled("void vfunc(bool param) override"); +} + } // namespace } // namespace clangd } // namespace clang Index: clangd/CodeComplete.cpp === --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -188,14 +188,66 @@ 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; +Overrides[Method->getName()].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 { llvm::StringRef Name; // Used for filtering and sorting. // We may have a result from Sema, from the index, or both. 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 { @@ -352,21 +404,30 @@ Completion.Documentation = getDocComment(ASTCtx, *C.SemaResult, /*CommentsFromHeader=*/false); } +if (C.IsOverride) + S.OverrideSuffix = true; } CodeCompletion build() { Completion.ReturnType = summarizeReturnType(); Completion.Signature = summarizeSignature(); Completion.SnippetSuffix = summarizeSnippet(); Completion.BundleSize = Bundled.size(); +if (summarizeOverride()) { + Completion.Name = Completion.ReturnType + ' ' + +std::move(Completion.Name) + +std::move(Completion.Signature) + " override"; + Completion.Signature.clear(); +} return std::move(Completion); } private: struct BundledEntry { std::str
[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.
kadircet updated this revision to Diff 162151. kadircet added a comment. - Resolve discussions. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50898 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h unittests/clangd/CodeCompleteTests.cpp 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 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,14 +188,66 @@ 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; +Overrides[Method->getName()].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; + } +} +
[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. Thanks, LGTM. Comment at: clangd/CodeComplete.cpp:210 +const std::string Name = Method->getNameAsString(); +Overrides[Name].push_back(Method); + } nit: here as well, use `Overrides[Method->getName()].push_back...` Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.
kadircet updated this revision to Diff 161963. kadircet marked 6 inline comments as done. kadircet added a comment. - Resolve discussions. - Fix a bug inside summarizeOverride. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50898 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h unittests/clangd/CodeCompleteTests.cpp 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 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,14 +188,67 @@ 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. +
[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.
hokein added a comment. Looks good mostly, a few nits. We should make sure all related comments are updated accordingly Comment at: clangd/CodeComplete.cpp:198 +static std::vector +getVirtualNonOverridenMethods(const DeclContext *DC, Sema *S) { + const auto *CR = llvm::dyn_cast(DC); Since we are returning `CodeCompletionResult`, maybe naming it like `getNonOverridenCompleteionResults` is clearer? Comment at: clangd/CodeComplete.cpp:222 + const std::string Name = Method->getNameAsString(); + const auto it = Overrides.find(Name); + bool IsOverriden = false; nit: Can't we use `Overrides.find(Method->getName())` and the other places as well? Comment at: clangd/CodeComplete.cpp:224 + bool IsOverriden = false; + if (it != Overrides.end()) +for (auto *MD : it->second) { nit: use `{}` around the body of `if` -- the one-line for statement is across line, adding `{}` around it will improve the readability. Comment at: clangd/CodeComplete.cpp:1238 : SymbolSlab(); // Merge Sema and Index results, score them, and pick the winners. +const auto Overrides = getVirtualNonOverridenMethods( nit: we need to update the comment accordingly. Comment at: clangd/CodeComplete.cpp:1281 // Groups overloads if desired, to form CompletionCandidate::Bundles. // The bundles are scored and top results are returned, best to worst. std::vector here as well. Comment at: clangd/CodeComplete.cpp:1322 AddToBundles(&SemaResult, CorrespondingIndexResult(SemaResult)); +for (auto &OverrideResult : OverrideResults) + AddToBundles(&OverrideResult, CorrespondingIndexResult(OverrideResult), IIUC, we are treating the override results the same `SemaResult`, it is safe as long as Sema doesn't provide these overrides in its code completion results, otherwise we will have duplicated completion items? I think we probably need a proper comment explaining what's going on here. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.
kadircet added a comment. After today's offline discussion I suppose we are not going to implement it within Sema. And also I think getVirtualNonOverridenMethods is a pretty generic function that has no ties to clangd, so it can be easily moved into Sema. Should we still move it into a separate file or leave it there? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.
ioeric added a comment. In https://reviews.llvm.org/D50898#1205756, @hokein wrote: > I think it is reasonable to make Sema support suggesting override methods, > instead of implementing it in clangd side? drive-by: +1 to this. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.
kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clangd/CodeComplete.cpp:1233 +// struct/class/union definition. +const auto Overrides = getVirtualNonOverridenMethods( +Recorder->CCSema->CurContext, Recorder->CCSema); hokein wrote: > It seems that we treat it as a special case, the code path here runs out of > the `ranking` path. Put override suggestions to ranking system. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.
kadircet updated this revision to Diff 161683. kadircet marked 3 inline comments as done. kadircet added a comment. - Put overrides through scoring and resolve nits. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50898 Files: clangd/CodeComplete.cpp clangd/CodeComplete.h unittests/clangd/CodeCompleteTests.cpp 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) { @@ -1648,6 +1649,58 @@ SigDoc("Doc from sema"; } +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 Index: clangd/CodeComplete.h === --- clangd/CodeComplete.h +++ clangd/CodeComplete.h @@ -127,6 +127,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,14 +188,67 @@ 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 +getVirtualNonOverridenMethods(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 std::string Name = Method->getNameAsString(); + const auto it = Overrides.find(Name); + bool IsOverriden = false; + if (it != Overrides.end()) +for (auto *MD : it->second) { + // If the method in current body is not an overload o
[PATCH] D50898: [clangd] Suggest code-completions for overriding base class virtual methods.
hokein added a comment. I think it is reasonable to make Sema support suggesting override methods, instead of implementing it in clangd side? Comment at: clangd/CodeComplete.cpp:206 + llvm::StringMap> Overrides; + for (auto *Method : dyn_cast(DC)->methods()) { +if (!Method->isVirtual()) nit: CR->methods(). Comment at: clangd/CodeComplete.cpp:210 +const std::string Name = Method->getNameAsString(); +const auto it = Overrides.find(Name); +if (it == Overrides.end()) nit: we can simplify the code like `Overrides[Name].push_back(Method)`. Comment at: clangd/CodeComplete.cpp:219 + for (const auto &Base : CR->bases()) { +const auto *BR = Base.getType().getTypePtr()->getAsCXXRecordDecl(); +for (auto *Method : BR->methods()) { I think we should check whether `BR == nullptr` here. Comment at: clangd/CodeComplete.cpp:1233 +// struct/class/union definition. +const auto Overrides = getVirtualNonOverridenMethods( +Recorder->CCSema->CurContext, Recorder->CCSema); It seems that we treat it as a special case, the code path here runs out of the `ranking` path. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits