Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(treesitter): clip end row early #28460

Merged
merged 2 commits into from May 7, 2024
Merged

Conversation

tomtomjhj
Copy link
Sponsor Contributor

Also add some more argument checks.

Ref: #28417 (comment)
cc @justinmk

@github-actions github-actions bot added the refactor changes that are not features or bugfixes label Apr 22, 2024
@tomtomjhj tomtomjhj marked this pull request as draft April 22, 2024 15:04
Comment on lines +552 to +554
win_T *win = handle_get_window(window);
if (!win) {
return luaL_error(lstate, "invalid window");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's a false positive (i.e. "can never happen"), assert() or abort() is fine too

Copy link
Sponsor Contributor Author

@tomtomjhj tomtomjhj Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

find_window_by_handle in the old version uses handle_get_window and does null check internally, and sets err in that case. So ERROR_SET(&err) in the old version implies win is non-null. But adding assert to that is somewhat redundant, so I replaced those stuff with handle_get_window and direct null check.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's redundant either way, because that warning was a false positive.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but I just wanted to reduce the noise from the warning

Also add some more argument checks.
@tomtomjhj tomtomjhj marked this pull request as ready for review April 22, 2024 15:37
@tomtomjhj tomtomjhj changed the title refactor(fold): avoid coverity false-positive fix(treesitter): normalise end row early Apr 22, 2024
@tomtomjhj
Copy link
Sponsor Contributor Author

tomtomjhj commented Apr 22, 2024

With the additional argument checks, I noticed a regression from #28417 and fixed it. cc @lewis6991

Problem:
UINT32_MAX + 1 passed to vim._foldupdate.

Solution:
Clip the end row from treesitter asap to avoid such issues.
@tomtomjhj tomtomjhj changed the title fix(treesitter): normalise end row early fix(treesitter): clip end row early Apr 22, 2024
@lewis6991 lewis6991 merged commit e7f50f4 into neovim:master May 7, 2024
29 checks passed
@tomtomjhj tomtomjhj deleted the fold-check branch May 7, 2024 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor changes that are not features or bugfixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants