ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.

lg



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


================
Comment at: test/CodeCompletion/overrides.cpp:17
+
+// 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) const override{{$}}
----------------
nit: add `// completion ^void`


================
Comment at: test/CodeCompletion/overrides.cpp:22
+//
+// 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{{$}}
----------------
add `// completion at vo^id`


================
Comment at: test/CodeCompletion/overrides.cpp:27
+//
+// 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) const override{{$}}
----------------
add `// completion at void ^`


Repository:
  rC Clang

https://reviews.llvm.org/D52225



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to