[clangd] Suggest code-completions for overriding base class virtual methods.

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

clang-tools-extra/clangd/CodeComplete.cpp

@@ -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 {
@@ -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 {
@@ -352,6 +404,8 @@ struct CodeCompletionBuilder {
         Completion.Documentation = getDocComment(ASTCtx, *C.SemaResult,
                                                  /*CommentsFromHeader=*/false);
     }
+    if (C.IsOverride)
+      S.OverrideSuffix = true;
   }
 
   CodeCompletion build() {
@@ -359,6 +413,12 @@ struct CodeCompletionBuilder {
     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);
   }
 
@@ -367,6 +427,7 @@ private:
     std::string SnippetSuffix;
     std::string Signature;
     std::string ReturnType;
+    bool OverrideSuffix;
   };
 
   // If all BundledEntrys have the same value for a property, return it.
@@ -379,6 +440,14 @@ private:
     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;
@@ -406,6 +475,12 @@ private:
     return "(…)";
   }
 
+  bool summarizeOverride() const {
+    if (auto *OverrideSuffix = onlyValue<&BundledEntry::OverrideSuffix>())
+      return *OverrideSuffix;
+    return false;
+  }
+
   ASTContext &ASTCtx;
   CodeCompletion Completion;
   SmallVector<BundledEntry, 1> Bundled;
@@ -1181,10 +1256,14 @@ private:
     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;
@@ -1192,6 +1271,7 @@ private:
     }
     Output.HasMore = Incomplete;
     Output.Context = Recorder->CCContext.getKind();
+
     return Output;
   }
 
@@ -1218,20 +1298,24 @@ private:
     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());
@@ -1258,6 +1342,13 @@ private:
     // 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))

clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp

@@ -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