kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.
LGTM
Have one question though, does it improve behavior in vscode? Since label seems
to be the same, it will most definitely improve clangd's ranking but vscode
ignores it anyway.
================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3165
+ }
+ if (!SeenTypedChunk && Chunk.Kind == CodeCompletionString::CK_TypedText)
+ SeenTypedChunk = true;
----------------
why not just put `SeenTypedChunk |= Chunk.Kind ==
CodeCompletionString::CK_TypedText`
================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3172
+ // Add a space after return type.
+ if (Chunk.Kind == CodeCompletionString::CK_ResultType) {
+ assert(!SeenTypedChunk);
----------------
do we expect anything but return type in the `BeforeName` or any case where we
shouldn't put a space between `BeforeName` and `NameAndSignature` ?
why not let the user add a space while concatenating these two?
================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3191
+ printOverrideString(*CCS, BeforeName, NameAndSignature);
+ NameAndSignature += " override";
+
----------------
let's move this into `printOverrideString`
================
Comment at: clang/test/CodeCompletion/overrides.cpp:14
void vfunc(bool param) override;
- void
+ vfo
};
----------------
nit: I suppose it should be `vfu`?(same thing for the comments below starting
with `Runs completion...`
================
Comment at: clang/test/CodeCompletion/overrides.cpp:29
-//
-// Runs completion at void ^.
-// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:8 %s -o - |
FileCheck -check-prefix=CHECK-CC3 %s
----------------
ilya-biryukov wrote:
> I've removed this to avoid dealing with the error return code (`vfo` is
> unresolved, so clang produces error there).
> Happy to bring it back if you feel it's important
it was here to prevent a regression maybe trigger it on the 13th line instead?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62298/new/
https://reviews.llvm.org/D62298
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
