[clang] Implement Override Suggestions in Sema.

[kadircet]

Summary

In clangd we had a new type of completion suggestions for cpp
class/struct/unions that will show override signatures for virtual methods in
base classes. This patch implements it in sema because it is hard to deduce more
info about completion token outside of Sema and handle itchy cases.

See the patch D50898 for more info on the functionality.

In addition to above patch this one also converts the suggestion into a
CK_Pattern with whole insertion text as the name of the suggestion and factors
out CodeCompletionString generation for declerations so that it can be re-used
by others.

[ioeric]

Could you move the corresponding tests from clangd to sema?

lib/Sema/SemaCodeComplete.cpp

3137

Could you put this definition before createCodeCompletionStringForDecl? I think it should make it easier to see unchanged lines.

[kadircet]

• Change order of fucntions for better diff.
• Add tests.

[kadircet]

ping

[ioeric]

lib/Sema/SemaCodeComplete.cpp

1603

Sema is available via Results.getSema()

1639

Could you add comments explaining what the following code does? The original code in clangd only has Results.emplace_back(Method, 0);, and it's non-trivial what the new code here is for.

1644

nit: /*Parameter-name=*/false

2940

nit: I think the contract would be clearer if this returns CodeCompletionString *. The empty returns below are a bit hard to follow.

test/CodeCompletion/overrides.cpp

21

nit: add {{$}} to the end of patterns.

21

nit: I think it's more common practice to put CHECKs for one completion in one group and after the completion command. I think it would be easier to read.

Please also add comment on what each case is testing.

[kadircet]

• Address comments.

lib/Sema/SemaCodeComplete.cpp

1639

Yeah you are right added some comments, previously this was handled on CodeCompletionBuilder in clangd which converted the completion item into a override declaration to be inserted.

[ioeric]

lg

lib/Sema/SemaCodeComplete.cpp

1639

Thanks! Could you elaborate on why you need to explicitly handle CCR and CCS, i.e. why was Results.emplace_back(Method, 0); not enough? I guess it's to avoid inserting return type when it's already typed, but it's not obvious from the code.

test/CodeCompletion/overrides.cpp

18

nit: add // completion ^void

23

add // completion at vo^id

28

add // completion at void ^

[kadircet]

• Fix a bug and address comments.

[kadircet]

lib/Sema/SemaCodeComplete.cpp

1639

Actually it is to generate a new CodeCompletionString with a single TypedText chunk that will include all of the declaration. Since you need to type whole declaration, which is also the part that has been used to filter completion candidates based on completion token.

It also helped me notice a bug relating optional params, I wasn't putting them into the typedtext chunk whereas I should've, fixed that one also.

[kadircet]

• Add a fixme on limitation.

include/clang/Sema/CodeCompleteConsumer.h

public:
   /// Creates a new code-completion string for the macro result. Similar to the
   /// above overloads, except this only requires preprocessor information.
   /// The result kind must be `RK_Macro`.
   CodeCompletionString *
   CreateCodeCompletionStringForMacro(Preprocessor &PP,
                                      CodeCompletionAllocator &Allocator,
                                      CodeCompletionTUInfo &CCTUInfo);
 
+  CodeCompletionString *createCodeCompletionStringForDecl(
+      Preprocessor &PP, ASTContext &Ctx, CodeCompletionBuilder &Result,
+      bool IncludeBriefComments, const CodeCompletionContext &CCContext,
+      PrintingPolicy &Policy);
+
+  CodeCompletionString *createCodeCompletionStringForOverride(
+      Preprocessor &PP, ASTContext &Ctx, CodeCompletionBuilder &Result,
+      bool IncludeBriefComments, const CodeCompletionContext &CCContext,
+      PrintingPolicy &Policy);
+
   /// Retrieve the name that should be used to order a result.
   ///
   /// If the name needs to be constructed as a string, that string will be
   /// saved into Saved and the returned StringRef will refer to it.
   StringRef getOrderedName(std::string &Saved) const;
 
 private:
   void computeCursorKindAndAvailability(bool Accessible = true);

lib/Sema/SemaCodeComplete.cpp

static void AddStaticAssertResult(CodeCompletionBuilder &Builder,
   Builder.AddChunk(CodeCompletionString::CK_LeftParen);
   Builder.AddPlaceholderChunk("expression");
   Builder.AddChunk(CodeCompletionString::CK_Comma);
   Builder.AddPlaceholderChunk("message");
   Builder.AddChunk(CodeCompletionString::CK_RightParen);
   Results.AddResult(CodeCompletionResult(Builder.TakeString()));
 }
 
+namespace {
+void printOverrideString(llvm::raw_ostream &OS, CodeCompletionString *CCS) {
+  for (const auto &C : *CCS) {
+    if (C.Kind == CodeCompletionString::CK_Optional)
+      printOverrideString(OS, C.Optional);
+    else
+      OS << C.Text;
+    // Add a space after return type.
+    if (C.Kind == CodeCompletionString::CK_ResultType)
+      OS << ' ';
+  }
+}
+} // namespace
+
+static void AddOverrideResults(ResultBuilder &Results,
+                               const CodeCompletionContext &CCContext,
+                               CodeCompletionBuilder &Builder) {
+  Sema &S = Results.getSema();
+  const auto *CR = llvm::dyn_cast<CXXRecordDecl>(S.CurContext);
+  // 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() || !Method->getIdentifier())
+      continue;
+    Overrides[Method->getName()].push_back(Method);
+  }
+
+  for (const auto &Base : CR->bases()) {
+    const auto *BR = Base.getType().getTypePtr()->getAsCXXRecordDecl();
+    if (!BR)
+      continue;
+    for (auto *Method : BR->methods()) {
+      if (!Method->isVirtual() || !Method->getIdentifier())
+        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, then it overrides this one.
+          if (!S.IsOverload(MD, Method, false)) {
+            IsOverriden = true;
+            break;
+          }
+        }
+      }
+      if (!IsOverriden) {
+        // Generates a new CodeCompletionResult by taking this function and
+        // converting it into an override declaration with only one chunk in the
+        // final CodeCompletionString as a TypedTextChunk.
+        std::string OverrideSignature;
+        llvm::raw_string_ostream OS(OverrideSignature);
+        CodeCompletionResult CCR(Method, 0);
+        PrintingPolicy Policy =
+            getCompletionPrintingPolicy(S.getASTContext(), S.getPreprocessor());
+        auto *CCS = CCR.createCodeCompletionStringForOverride(
+            S.getPreprocessor(), S.getASTContext(), Builder,
+            /*IncludeBriefComments=*/false, CCContext, Policy);
+        Results.AddResult(CodeCompletionResult(CCS, Method, CCP_CodePattern));
+      }
+    }
+  }
+}
+
 /// Add language constructs that show up for "ordinary" names.
 static void AddOrdinaryNameResults(Sema::ParserCompletionContext CCC,
                                    Scope *S,
                                    Sema &SemaRef,
                                    ResultBuilder &Results) {
   CodeCompletionAllocator &Allocator = Results.getAllocator();
   CodeCompletionBuilder Builder(Allocator, Results.getCodeCompletionTUInfo());
 
if (SemaRef.getLangOpts().CPlusPlus) {
           Builder.AddChunk(CodeCompletionString::CK_Colon);
         Results.AddResult(Result(Builder.TakeString()));
 
         // private:
         Builder.AddTypedTextChunk("private");
         if (IsNotInheritanceScope && Results.includeCodePatterns())
           Builder.AddChunk(CodeCompletionString::CK_Colon);
         Results.AddResult(Result(Builder.TakeString()));
+
+        // FIXME: This adds override results only if we are at the first word of
+        // the declaration/definition. Also call this from other sides to have
+        // more use-cases.
+        AddOverrideResults(Results, CodeCompletionContext::CCC_ClassStructUnion,
+                           Builder);
       }
     }
     LLVM_FALLTHROUGH;
 
   case Sema::PCC_Template:
   case Sema::PCC_MemberTemplate:
     if (SemaRef.getLangOpts().CPlusPlus && Results.includeCodePatterns()) {
       // template < parameters >
if (Kind == RK_Pattern) {
     return Pattern;
   }
 
   if (Kind == RK_Keyword) {
     Result.AddTypedTextChunk(Keyword);
     return Result.TakeString();
   }
   assert(Kind == RK_Declaration && "Missed a result kind?");
+  return createCodeCompletionStringForDecl(PP, Ctx, Result, IncludeBriefComments,
+                                    CCContext, Policy);
+}
+
+CodeCompletionString *
+CodeCompletionResult::createCodeCompletionStringForOverride(
+    Preprocessor &PP, ASTContext &Ctx, CodeCompletionBuilder &Result,
+    bool IncludeBriefComments, const CodeCompletionContext &CCContext,
+    PrintingPolicy &Policy) {
+  std::string OverrideSignature;
+  llvm::raw_string_ostream OS(OverrideSignature);
+  auto *CCS = createCodeCompletionStringForDecl(PP, Ctx, Result,
+                                                /*IncludeBriefComments=*/false,
+                                                CCContext, Policy);
+  printOverrideString(OS, CCS);
+  OS << " override";
+  Result.AddTypedTextChunk(Result.getAllocator().CopyString(OS.str()));
+  return Result.TakeString();
+}
+
+CodeCompletionString *CodeCompletionResult::createCodeCompletionStringForDecl(
+    Preprocessor &PP, ASTContext &Ctx, CodeCompletionBuilder &Result,
+    bool IncludeBriefComments, const CodeCompletionContext &CCContext,
+    PrintingPolicy &Policy) {
   const NamedDecl *ND = Declaration;
   Result.addParentContext(ND->getDeclContext());
 
   if (IncludeBriefComments) {
     // Add documentation comment, if it exists.
     if (const RawComment *RC = getCompletionComment(Ctx, Declaration)) {
       Result.addBriefComment(RC->getBriefText(Ctx));
     }
   }
 
   if (StartsNestedNameSpecifier) {
     Result.AddTypedTextChunk(
                       Result.getAllocator().CopyString(ND->getNameAsString()));
     Result.AddTextChunk("::");
AddQualifierToCompletionString(Result, Qualifier, QualifierIsInformative,
                                    Ctx, Policy);
     Result.AddTypedTextChunk(
                 Result.getAllocator().CopyString(Template->getNameAsString()));
     Result.AddChunk(CodeCompletionString::CK_LeftAngle);
     AddTemplateParameterChunks(Ctx, Policy, Template, Result);
     Result.AddChunk(CodeCompletionString::CK_RightAngle);
     return Result.TakeString();
   }
-
   if (const ObjCMethodDecl *Method = dyn_cast<ObjCMethodDecl>(ND)) {
     Selector Sel = Method->getSelector();
     if (Sel.isUnarySelector()) {
       Result.AddTypedTextChunk(Result.getAllocator().CopyString(
                                   Sel.getNameForSlot(0)));
       return Result.TakeString();
     }
 
if (const ObjCMethodDecl *Method = dyn_cast<ObjCMethodDecl>(ND)) {
     return Result.TakeString();
   }
 
   if (Qualifier)
     AddQualifierToCompletionString(Result, Qualifier, QualifierIsInformative,
                                    Ctx, Policy);
 
   Result.AddTypedTextChunk(
-                       Result.getAllocator().CopyString(ND->getNameAsString()));
   return Result.TakeString();
 }
 
 const RawComment *clang::getCompletionComment(const ASTContext &Ctx,
                                               const NamedDecl *ND) {
   if (!ND)
     return nullptr;
   if (auto *RC = Ctx.getRawCommentForAnyRedecl(ND))
     return RC;
 
   // Try to find comment from a property for ObjC methods.
   const ObjCMethodDecl *M = dyn_cast<ObjCMethodDecl>(ND);
   if (!M)
     return nullptr;
   const ObjCPropertyDecl *PDecl = M->findPropertyDecl();
   if (!PDecl)
     return nullptr;
 

test/CodeCompletion/overrides.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 int ttt(bool param, int x = 3) const;
+void vfunc(bool param, int p) override;
+};
+class C : public B {
+ public:
+  void vfunc(bool param) override;
+  void
+};
+
+// Runs completion at ^void.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:3 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+// CHECK-CC1: COMPLETION: Pattern : int ttt(bool param, int x = 3) const override{{$}}
+// CHECK-CC1: COMPLETION: Pattern : void vfunc(bool param, int p) override{{$}}
+// CHECK-CC1-NOT: COMPLETION: Pattern : void vfunc(bool param) override{{$}}
+//
+// Runs completion at vo^id.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:5 %s -o - | FileCheck -check-prefix=CHECK-CC2 %s
+// CHECK-CC2: COMPLETION: Pattern : void vfunc(bool param, int p) override{{$}}
+// CHECK-CC2-NOT: COMPLETION: Pattern : int ttt(bool param, int x = 3) const override{{$}}
+// CHECK-CC2-NOT: COMPLETION: Pattern : void vfunc(bool param) override{{$}}
+//
+// Runs completion at void ^.
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:8 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// CHECK-CC3-NOT: COMPLETION: Pattern : int ttt(bool param, int x = 3) const override{{$}}
+// CHECK-CC3-NOT: COMPLETION: Pattern : void vfunc(bool param, int p) override{{$}}
+// CHECK-CC3-NOT: COMPLETION: Pattern : void vfunc(bool param) override{{$}}