[clang] [clang-tools-extra] [clangd] Add InsertReplaceEdit for code completion (PR #187623)
via cfe-commits
cfe-commits at lists.llvm.org
Fri May 1 10:29:54 PDT 2026
timon-ul wrote:
Alright @argothiel , I have spent some more time getting familiar with the surrounding code and your changes and I am basically ready to give this PR the green light. Just some last thoughts I wanted to ask you about.
> VS Code always advertises insertReplaceSupport: true
I see, yeah no wonder I had trouble. Now I was able to test these changes. For the basic identifiers it works, but it will not work with function arguments and templates. Is this an intended limitation? Here an example of what I mean
```
void testing(int i);
t^ing(int i);
```
completes to
```
void testing(int i);
testing(int i)(int i);
```
basically it is able to detect the ending `ing` I already had there but it fails to realize I already have `(int i)`. Even if this is not intended behaviour for now I can accept the current PR and maybe this is then a future enhancement, but at least I personally do not see why we would not want to detect this and I think it is actually pretty useful.
Now regarding the current code itself:
> We could duplicate the lexing logic in clangd
First off, I also do not want any code duplication, that sounds like hell to maintain, I was more wondering if we really needed to directly call `Lexer::LexIdentifierContinue` or if a generic call to the lexer would have been sufficient. I still think this might work, but we would need to handle the suffix starting with unicode, which I think will probably be ugly. So in this regard I am fine with how it is right now.
What I am still wondering though is if you have considered adding this functionality to the initial lexing. The one that creates the `CodeCompletionRange`. This feels at least to me like the natural point given the lexer has to here already go through the whole file and lex it and technically now the end of the identifier is part of the range. I personally am unsure if this is a good idea, but I wanted to still raise this point.
Actually just now while writing this review I stumbled upon [this line](https://searchfox.org/llvm/source/clang-tools-extra/clangd/CodeComplete.cpp#1674) that actually already gets the next `Token` which we can get an end location from (as you see we are right before we call `runWithSema`). Glancing at it I am not sure it can handle a suffix starting with a unicode character, but at least I feel like we could combine both (and maybe not run the lexer twice then).
https://github.com/llvm/llvm-project/pull/187623
More information about the cfe-commits
mailing list