[PATCH] D52225: [clang] Implement Override Suggestions in Sema.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: lib/Sema/SemaCodeComplete.cpp:1603
+                               const CodeCompletionContext &CCContext,
+                               CodeCompletionBuilder &Builder, Sema &S) {
+  const auto *CR = llvm::dyn_cast<CXXRecordDecl>(S.CurContext);
----------------
Sema is available via `Results.getSema()`


================
Comment at: lib/Sema/SemaCodeComplete.cpp:1639
+        llvm::raw_string_ostream OS(OverrideSignature);
+        CodeCompletionResult CCR(Method, 0);
+        PrintingPolicy Policy =
----------------
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.


================
Comment at: lib/Sema/SemaCodeComplete.cpp:1644
+            S.getPreprocessor(), S.getASTContext(), Builder,
+            false, CCContext, Policy);
+        for(const auto& C : *Builder.TakeString()) {
----------------
nit: `/*Parameter-name=*/false`


================
Comment at: lib/Sema/SemaCodeComplete.cpp:2905
+
+void CodeCompletionResult::createCodeCompletionStringForDecl(
+    Preprocessor &PP, ASTContext &Ctx,
----------------
nit: I think the contract would be clearer if this returns `CodeCompletionString *`. The empty `return`s below are a bit hard to follow.


================
Comment at: test/CodeCompletion/overrides.cpp:20
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:8 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// CHECK-CC1: COMPLETION: Pattern : int ttt(bool param) const override
+// CHECK-CC2-NOT: COMPLETION: Pattern : int ttt(bool param) const override
----------------
nit: add `{{$}}` to the end of patterns.


================
Comment at: test/CodeCompletion/overrides.cpp:20
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:8 %s -o - | FileCheck -check-prefix=CHECK-CC3 %s
+// CHECK-CC1: COMPLETION: Pattern : int ttt(bool param) const override
+// CHECK-CC2-NOT: COMPLETION: Pattern : int ttt(bool param) const override
----------------
ioeric wrote:
> nit: add `{{$}}` to the end of patterns.
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.


Repository:
  rC Clang

https://reviews.llvm.org/D52225