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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
