Skip to content

Fix completion crash when item.range starts using {inserting, replacing}#954

Merged
HighCommander4 merged 2 commits into
clangd:masterfrom
argothiel:insertReplaceEditSupport
May 7, 2026
Merged

Fix completion crash when item.range starts using {inserting, replacing}#954
HighCommander4 merged 2 commits into
clangd:masterfrom
argothiel:insertReplaceEditSupport

Conversation

@argothiel

Copy link
Copy Markdown
Contributor

The LSP spec (since 3.16) allows CompletionItem.range to be either a plain Range or an InsertReplaceRange ({inserting, replacing}). The old code cast item.range directly to vscode.Range; once clangd starts sending the newer form, this will crash (llvm/llvm-project#187623).

Narrow the type before extracting the start position, and guard against a missing range entirely.

The LSP spec (since 3.16) allows CompletionItem.range to be either a
plain Range or an InsertReplaceRange ({inserting, replacing}). The old
code cast item.range directly to vscode.Range; once clangd starts sending
the newer form, this will crash (llvm/llvm-project#187623).

Narrow the type before extracting the start position, and guard against
a missing range entirely.
@HighCommander4

Copy link
Copy Markdown
Contributor

cc @timon-ul

(I'm intending this as a review request, but I can't actually add you to the "Reviewers" field because you're not a member of the clangd org.)

@argothiel

Copy link
Copy Markdown
Contributor Author

@JVApen Thanks for the review! Could you merge? I don't have permissions.

@JVApen

JVApen commented May 6, 2026

Copy link
Copy Markdown
Contributor

I wanted to wait until others had the chance to review, in case I missed something. This looks very trivial, though I've learned others have a different way of looking at patches.

Given the explicit request for @timon-ul, I'd like to wait merging until he responded.

@argothiel

Copy link
Copy Markdown
Contributor Author

Makes sense, thank you! 🙂

@JVApen

JVApen commented May 6, 2026

Copy link
Copy Markdown
Contributor

It might make sense to bump the version number here for a new pre-release version and if that's not causing issues, to trigger a real release in a next PR. See https://github.com/clangd/vscode-clangd/pull/876/changes for an example. I would recommend doing this in a separate PR. For X.Y.Z, every odd Y is a pre-release, following the guidelines of vs code

@HighCommander4

Copy link
Copy Markdown
Contributor

(Agree this looks pretty trivial. I just wanted @timon-ul to okay it since it's related to llvm/llvm-project#187623 which he has been reviewing.)

@timon-ul

timon-ul commented May 6, 2026

Copy link
Copy Markdown

Just want to say it looks fine, but wanted to be a good reviewer and test the changes, but I am struggeling a bit with setting that up...

@timon-ul

timon-ul commented May 6, 2026

Copy link
Copy Markdown

Looks good to me, got it to run, works, code also looks good.

@HighCommander4 HighCommander4 merged commit dcf4f4b into clangd:master May 7, 2026
1 check passed
@argothiel argothiel deleted the insertReplaceEditSupport branch May 9, 2026 14:57
@argothiel

Copy link
Copy Markdown
Contributor Author

It might make sense to bump the version number here for a new pre-release version and if that's not causing issues, to trigger a real release in a next PR. See https://github.com/clangd/vscode-clangd/pull/876/changes for an example. I would recommend doing this in a separate PR. For X.Y.Z, every odd Y is a pre-release, following the guidelines of vs code

PR created: #956

JVApen pushed a commit that referenced this pull request May 9, 2026
Include support for InsertReplaceEdit in code completion middleware (#954).
@argothiel argothiel mentioned this pull request May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants