dundargoc

unread,

Mar 22, 2022, 2:55:41 PM3/22/22

to vim/vim, Subscribed

You can view, comment on, or merge this pull request online at:

https://github.com/vim/vim/pull/9993

Commit Summary

11ca6bb remove conditions that are always true or false

File Changes

(13 files)

Patch Links:

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

codecov[bot]

unread,

Mar 22, 2022, 3:09:08 PM3/22/22

to vim/vim, Subscribed

Codecov Report

Merging #9993 (11ca6bb) into master (35dc176) will increase coverage by 0.11%.
The diff coverage is 94.11%.

@@            Coverage Diff             @@

##           master    #9993      +/-   ##

==========================================

+ Coverage   81.81%   81.93%   +0.11%     

==========================================

  Files         167      167              

  Lines      187078   187209     +131     

  Branches    42131    42200      +69     

==========================================

+ Hits       153052   153382     +330     

+ Misses      21659    21484     -175     

+ Partials    12367    12343      -24

Flag	Coverage Δ
huge-clang-none	`82.33% <88.23%> (+<0.01%)`	⬆️
huge-gcc-none	`82.67% <88.23%> (+<0.01%)`	⬆️
huge-gcc-testgui	`81.13% <82.35%> (?)`
huge-gcc-unittests	`2.01% <0.00%> (+<0.01%)`	⬆️
linux	`83.92% <88.23%> (+0.32%)`	⬆️
mingw-x64-HUGE	`0.00% <0.00%> (?)`
mingw-x64-HUGE-gui	`77.15% <92.85%> (-0.01%)`	⬇️
windows	`75.94% <92.85%> (-1.23%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/vim9cmds.c	`87.38% <0.00%> (-0.02%)`	⬇️
src/clientserver.c	`77.47% <100.00%> (-0.18%)`	⬇️
src/drawline.c	`81.21% <100.00%> (-0.02%)`	⬇️
src/drawscreen.c	`79.81% <100.00%> (ø)`
src/ex_cmds.c	`85.00% <100.00%> (ø)`
src/fileio.c	`76.01% <100.00%> (+0.04%)`	⬆️
src/message.c	`81.56% <100.00%> (+0.12%)`	⬆️
src/misc2.c	`89.68% <100.00%> (+0.70%)`	⬆️
src/ops.c	`90.70% <100.00%> (ø)`
src/quickfix.c	`91.40% <100.00%> (-0.01%)`	⬇️
... and 62 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 35dc176...11ca6bb. Read the comment docs.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Bram Moolenaar

unread,

Mar 22, 2022, 4:37:12 PM3/22/22

to vim/vim, Subscribed

Thanks. The one in quickfix.c I'm not so sure about, I'll leave it out.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Bram Moolenaar

unread,

Mar 22, 2022, 4:43:04 PM3/22/22

to vim/vim, Subscribed

Closed #9993 via fe15499.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Yegappan Lakshmanan

unread,

Mar 22, 2022, 10:15:31 PM3/22/22

to vim_dev, reply+ACY5DGBEWSYHTYTFW7...@reply.github.com, vim/vim, Subscribed

Hi,

On Tue, Mar 22, 2022 at 1:37 PM Bram Moolenaar <vim-dev...@256bit.org> wrote:

Thanks. The one in quickfix.c I'm not so sure about, I'll leave it out.

The check for a valid qi->qf_curlist in quickfix.c is needed as the quickfix list can be

modified when opening the buffer by an autocmd. So this change cannot be applied.

Regards,

Yegappan

vim-dev ML

unread,

Mar 22, 2022, 10:16:38 PM3/22/22

to vim/vim, vim-dev ML, Your activity

Hi,

On Tue, Mar 22, 2022 at 1:37 PM Bram Moolenaar ***@***.***>

wrote:

> Thanks. The one in quickfix.c I'm not so sure about, I'll leave it out.
>
>
> The check for a valid qi->qf_curlist in quickfix.c is needed as the
quickfix list can be
modified when opening the buffer by an autocmd. So this change cannot be
applied.

Regards,
Yegappan

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

dundargoc

unread,

Mar 23, 2022, 6:51:51 AM3/23/22

to vim/vim, vim-dev ML, Comment

@yegappan I've stared at the quickfix code for 10 minutes and I don't get it. qi doesn't seem to be used in the function (except for comparisons), how can it change? Mind giving me a quick hint?

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.

Yegappan Lakshmanan

unread,

Mar 23, 2022, 10:13:02 AM3/23/22

to vim_dev, reply+ACY5DGHK2ZVRSQNSKJ...@reply.github.com, vim/vim, vim-dev ML, Comment

Hi,

On Wed, Mar 23, 2022 at 3:51 AM dundargoc <vim-dev...@256bit.org> wrote:

@yegappan I've stared at the quickfix code for 10 minutes and I don't get it. qi doesn't seem to be used in the function (except for comparisons), how can it change? Mind giving me a quick hint?

The quickfix list can be changed from an autocmd. For example, the following commands

will make the condition in that if statement false:

:cexpr ""

:autocmd BufEnter *.c colder

:vimgrep buf_T *.c

For other conditions in that if statement, you can replace 'colder' with 'call setqflist([], 'f')'

and 'call setqflist([], 'r').

Regards,

Yegappan

vim-dev ML

unread,

Mar 23, 2022, 10:13:14 AM3/23/22

to vim/vim, vim-dev ML, Your activity

Hi,

On Wed, Mar 23, 2022 at 3:51 AM dundargoc ***@***.***> wrote:

> @yegappan <https://github.com/yegappan> I've stared at the quickfix code

> for 10 minutes and I don't get it. qi doesn't seem to be used in the
> function (except for comparisons), how can it change? Mind giving me a
> quick hint?
>
>
>
The quickfix list can be changed from an autocmd. For example, the
following commands
will make the condition in that if statement false:

:cexpr ""
:autocmd BufEnter *.c colder
:vimgrep buf_T *.c

For other conditions in that if statement, you can replace 'colder' with
'call setqflist([], 'f')'
and 'call setqflist([], 'r').

Regards,
Yegappan

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Yegappan Lakshmanan

unread,

Mar 23, 2022, 12:31:49 PM3/23/22

to vim_dev, reply+ACY5DGHK2ZVRSQNSKJ...@reply.github.com, vim/vim, vim-dev ML, Comment

Hi,

One additional note. 'qi' points to the global variable 'ql_info'or
the window local location
list stack (wp->w_llist). The contents of these structures can be
changed out-of-band
by an autocmd.

- Yegappan

vim-dev ML

unread,

Mar 23, 2022, 12:32:03 PM3/23/22

to vim/vim, vim-dev ML, Your activity

Hi,

On Wed, Mar 23, 2022 at 7:12 AM Yegappan Lakshmanan ***@***.***> wrote:

>
> On Wed, Mar 23, 2022 at 3:51 AM dundargoc ***@***.***> wrote:
>>
>> @yegappan I've stared at the quickfix code for 10 minutes and I don't get it. qi doesn't seem to be used in the function (except for comparisons), how can it change? Mind giving me a quick hint?
>>
>>
>
> The quickfix list can be changed from an autocmd. For example, the following commands
> will make the condition in that if statement false:
>
> :cexpr ""
> :autocmd BufEnter *.c colder
> :vimgrep buf_T *.c
>
> For other conditions in that if statement, you can replace 'colder' with 'call setqflist([], 'f')'
> and 'call setqflist([], 'r').
>

One additional note. 'qi' points to the global variable 'ql_info'or
the window local location
list stack (wp->w_llist). The contents of these structures can be
changed out-of-band
by an autocmd.

- Yegappan

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

dundargoc

unread,

Mar 23, 2022, 2:53:46 PM3/23/22

to vim/vim, vim-dev ML, Comment

Dear lord, how do you even keep track of that? O_O

Anyway, I understand now. Thanks for the help.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.

Yegappan Lakshmanan

unread,

Mar 23, 2022, 3:14:53 PM3/23/22

to vim_dev, reply+ACY5DGAJ2PK67IYBDB...@reply.github.com, vim/vim, vim-dev ML, Comment

Hi,

On Wed, Mar 23, 2022 at 11:53 AM dundargoc <vim-dev...@256bit.org> wrote:

Dear lord, how do you even keep track of that? O_O

Over a period of time, based on the bug reports (fuzzy tests) and the new tests, several

mechanisms were added to detect these race conditions.

1. Use is_qf_entry_present() to make sure the current entry is still present in

the quickfix list.

2. Use qf_changedtick to detect that the entries are not modified in the quickfix list.

3. Use the quickfix list identifier to make sure the list is still present and is present

at the correct index in the stack.

4. Use quickfix_busy to postpone the removal of location lists.

5. Use the qf_delq to queue all the location list delete requests and process them

when it is safer.

Regards,

Yegappan

vim-dev ML

unread,

Mar 23, 2022, 3:15:10 PM3/23/22

to vim/vim, vim-dev ML, Your activity

Hi,

On Wed, Mar 23, 2022 at 11:53 AM dundargoc ***@***.***>

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Reply all

Reply to author

Forward

[vim/vim] remove conditions that are always true or false (PR #9993)

dundargoc

You can view, comment on, or merge this pull request online at:

Commit Summary

File Changes

Patch Links:

codecov[bot]

Codecov Report

Bram Moolenaar

Bram Moolenaar

Yegappan Lakshmanan

vim-dev ML

dundargoc

Yegappan Lakshmanan

vim-dev ML

Yegappan Lakshmanan

vim-dev ML

dundargoc

Yegappan Lakshmanan

vim-dev ML