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<const std::vector<CodeCompletion> &> 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<CodeCompletionResult>
+getVirtualNonOverridenMethods(const DeclContext *DC, Sema *S) {
+ const auto *CR = llvm::dyn_cast<CXXRecordDecl>(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<std::vector<FunctionDecl *>> Overrides;
+ for (auto *Method : CR->methods()) {
+ if (!Method->isVirtual())
+ continue;
+ const std::string Name = Method->getNameAsString();
+ Overrides[Name].push_back(Method);
+ }
+
+ std::vector<CodeCompletionResult> 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 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 {
@@ -351,21 +404,25 @@
Completion.Documentation = getDocComment(ASTCtx, *C.SemaResult,
/*CommentsFromHeader=*/false);
}
+ if (C.IsOverride)
+ S.OverrideSuffix = "override";
}
CodeCompletion build() {
Completion.ReturnType = summarizeReturnType();
Completion.Signature = summarizeSignature();
Completion.SnippetSuffix = summarizeSnippet();
Completion.BundleSize = Bundled.size();
+ Completion.IsOverride = summarizeOverride();
return std::move(Completion);
}
private:
struct BundledEntry {
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 (onlyValue<&BundledEntry::OverrideSuffix>())
+ return true;
+ return false;
+ }
+
ASTContext &ASTCtx;
CodeCompletion Completion;
SmallVector<BundledEntry, 1> Bundled;
@@ -1173,16 +1236,20 @@
? 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.
+ const auto Overrides = getVirtualNonOverridenMethods(
+ 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;
Output.Completions.back().CompletionTokenRange = TextEditRange;
}
Output.HasMore = Incomplete;
Output.Context = Recorder->CCContext.getKind();
+
return Output;
}
@@ -1213,16 +1280,19 @@
// Groups overloads if desired, to form CompletionCandidate::Bundles.
// The bundles are scored and top results are returned, best to worst.
std::vector<ScoredBundle>
- mergeResults(const std::vector<CodeCompletionResult> &SemaResults,
- const SymbolSlab &IndexResults) {
+ mergeResults(const std::vector<CodeCompletionResult> &SemaResults,
+ const SymbolSlab &IndexResults,
+ const std::vector<CodeCompletionResult> &OverrideResults) {
trace::Span Tracer("Merge and score results");
std::vector<CompletionCandidate::Bundle> Bundles;
llvm::DenseMap<size_t, size_t> 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 +1319,9 @@
// Emit all Sema results, merging them with Index results if possible.
for (auto &SemaResult : Recorder->Results)
AddToBundles(&SemaResult, CorrespondingIndexResult(SemaResult));
+ 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 +1460,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 +1471,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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
