kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.
thanks, lgtm!
================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:474
+ //
+ // fu^(42) -> function(42);
+ if (Snippet->front() == '<') {
----------------
RHS should be `function<int>(42)` right?
================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:480
+ unsigned Balance, Index;
+ for (Balance = 1, Index = 1; Balance && (Index != Snippet->size());
+ ++Index) {
----------------
nit:
```
int Balance = 0;
size_t I = 0;
do {
if(Snippet[I] == '>') --Balance;
else if(Snippet[I] == '<') ++Balance;
++I;
} while(Balance > 0);
return Snippet->substr(0, I);
```
This should handle both the case snippet starts with `<` and not. reducing the
nesting a little and making the flow more uniform (i.e. getting rid of the
second return statement).
Up to you though, if you keep the existing version please move definition of
`Balance` into for-init statement, use `size_t` instead of `unsigned` and array
subscripts instead of `at(Index)`.
================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:496
+ //
+ // fu^<int>(42) -> function<int>(42);
+ if (NextTokenKind == tok::less && Snippet->front() == '<')
----------------
i think it is better to omit `(42)` in this example.
================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:497
+ // fu^<int>(42) -> function<int>(42);
+ if (NextTokenKind == tok::less && Snippet->front() == '<')
+ return "";
----------------
You've marked the previous nit as done, but this case still seems to be coming
after the complicated case, just in case it slipped :D
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89870/new/
https://reviews.llvm.org/D89870
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
