Skip to content

Commit

Permalink
[clangd] Suggest code-completions for overriding base class virtual m…
Browse files Browse the repository at this point in the history
…ethods.

Summary:
Whenever a code-completion is triggered within a class/struct/union looks at
base classes and figures out non-overriden virtual functions. Than suggests
completions for those.

Reviewers: ilya-biryukov, hokein, ioeric

Reviewed By: hokein

Subscribers: MaskRay, jkorous, arphaman, cfe-commits

Differential Revision: https://reviews.llvm.org/D50898

llvm-svn: 340530
  • Loading branch information
kadircet committed Aug 23, 2018
1 parent 23cbf09 commit f8b85a3
Show file tree
Hide file tree
Showing 2 changed files with 125 additions and 9 deletions.
109 changes: 100 additions & 9 deletions clang-tools-extra/clangd/CodeComplete.cpp
Expand Up @@ -188,6 +188,55 @@ static llvm::Expected<HeaderFile> toHeaderFile(StringRef Header,
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>
getNonOverridenMethodCompletionResults(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;
Overrides[Method->getName()].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 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 {
Expand All @@ -196,6 +245,9 @@ struct CompletionCandidate {
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 {
Expand Down Expand Up @@ -352,13 +404,21 @@ struct CodeCompletionBuilder {
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);
}

Expand All @@ -367,6 +427,7 @@ struct CodeCompletionBuilder {
std::string SnippetSuffix;
std::string Signature;
std::string ReturnType;
bool OverrideSuffix;
};

// If all BundledEntrys have the same value for a property, return it.
Expand All @@ -379,6 +440,14 @@ struct CodeCompletionBuilder {
return &(B->*Member);
}

template <bool BundledEntry::*Member> const bool *onlyValue() const {
auto B = Bundled.begin(), E = Bundled.end();
for (auto I = B + 1; I != E; ++I)
if (I->*Member != B->*Member)
return nullptr;
return &(B->*Member);
}

std::string summarizeReturnType() const {
if (auto *RT = onlyValue<&BundledEntry::ReturnType>())
return *RT;
Expand Down Expand Up @@ -406,6 +475,12 @@ struct CodeCompletionBuilder {
return "(…)";
}

bool summarizeOverride() const {
if (auto *OverrideSuffix = onlyValue<&BundledEntry::OverrideSuffix>())
return *OverrideSuffix;
return false;
}

ASTContext &ASTCtx;
CodeCompletion Completion;
SmallVector<BundledEntry, 1> Bundled;
Expand Down Expand Up @@ -1181,17 +1256,22 @@ class CodeCompleteFlow {
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;
Output.Completions.back().CompletionTokenRange = TextEditRange;
}
Output.HasMore = Incomplete;
Output.Context = Recorder->CCContext.getKind();

return Output;
}

Expand All @@ -1218,20 +1298,24 @@ class CodeCompleteFlow {
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<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());
Expand All @@ -1258,6 +1342,13 @@ class CodeCompleteFlow {
// 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))
Expand Down
25 changes: 25 additions & 0 deletions clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
Expand Up @@ -1680,6 +1680,31 @@ TEST(CompletionTest, CompletionFunctionArgsDisabled) {
}
}

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

0 comments on commit f8b85a3

Please sign in to comment.