Dominique Pellé <[email protected]> wrote: > Dominique Pellé wrote: > > > Tony Mechelynck <[email protected]> wrote: > > > > > After applying patches (8.1.) 953 to 956 I get the following warning > > > in Tiny and Small builds (only): > > > > > > gcc -c -I. -Iproto -DHAVE_CONFIG_H -O2 -fno-strength-reduce -Wall > > > -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -o objects/ex_cmds.o > > > ex_cmds.c > > > ex_cmds.c: In function ‘ex_sort’: > > > ex_cmds.c:557:43: warning: overflow in implicit constant conversion > > > [-Woverflow] > > > nrs[lnum - eap->line1].st_u.value = -MAXLNUM; > > > ^ > > > > I can reproduce it indeed with tiny builds with 8.1.0953 > > and beyond. > > > > When pre-processing, the line where it warns is: > > > > nrs[lnum - eap->line1].st_u.value = -0x7fffffffffffffffL; > > > > Type of expression on the left (st_u.value) is varnumber_T > > which is defined in struct.h:1219 as: > > - 'int' when FEAT_NUM64 is not defined > > - a 64-bits type when FEAT_NUM64 is defined. > > > > I don't know why -MAXLNUM is used here as it st_u.value > > does not seem to contain a line number. It seems wrong. > > Maybe -VARNUM_MAX was meant to be used as follows > > (or perhaps VARNUM_MIN?): > > > > diff --git a/src/ex_cmds.c b/src/ex_cmds.c > > index da10c1291..baa65d040 100644 > > --- a/src/ex_cmds.c > > +++ b/src/ex_cmds.c > > @@ -554,7 +554,7 @@ ex_sort(exarg_T *eap) > > --s; /* include preceding negative sign */ > > if (*s == NUL) > > /* empty line should sort before any number */ > > - nrs[lnum - eap->line1].st_u.value = -MAXLNUM; > > + nrs[lnum - eap->line1].st_u.value = -VARNUM_MAX; > > else > > vim_str2nr(s, NULL, NULL, sort_what, > > &nrs[lnum - eap->line1].st_u.value, NULL, 0); > > > > It looks like this was a silent bug in :sort even before 8.1.0953 > > when FEAT_NUM64 was defined as -0x7fffffff was then assigned > > to a 64-bits number which looks suspicious. It should be possible > > to find a case where :sort was misbehaving. > > > > Dominique > > I see that behavior of :sort n' was wrong > both before and after 8.1.953 but in different > ways in vim huge as well as in Vim tiny, it was > also wrong before and after 8.1.953. > > Example with vim tiny >= 8.1.953 > > $ cat > numbers.txt <<EOF > 1 > -1 > 0 > 2147483647 > > 2147483647 > > 2147483646 > -2147483648 > -2147483647 > -2147483646 > EOF > > > $ vim-8.1.956.tiny --clean numbers.txt -c 'sort n' > gives: > === > -2147483648 > -2147483647 > -2147483646 > -1 > 0 > 1 > > > 2147483646 > 2147483647 > 2147483647 > === > > Prior to 8.1.953, vim.tiny was also sorting > incorrectly but differently as: > === > 2147483648 > > > -2147483647 > -2147483646 > -1 > 0 > 1 > 2147483646 > 2147483647 > 2147483647 > === > > Now when applying the following patch... > > diff --git a/src/ex_cmds.c b/src/ex_cmds.c > index da10c1291..3cb4641e8 100644 > --- a/src/ex_cmds.c > +++ b/src/ex_cmds.c > @@ -554,7 +554,7 @@ ex_sort(exarg_T *eap) > --s; /* include preceding negative sign */ > if (*s == NUL) > /* empty line should sort before any number */ > - nrs[lnum - eap->line1].st_u.value = -MAXLNUM; > + nrs[lnum - eap->line1].st_u.value = VARNUM_MIN; > else > vim_str2nr(s, NULL, NULL, sort_what, > &nrs[lnum - eap->line1].st_u.value, NULL, 0); > > I then get the expected output with vim.tiny: > > === > > -2147483648 > -2147483647 > -2147483646 > -1 > 0 > 1 > 2147483646 > 2147483647 > 2147483647 > === > > However, ordering of empty lines > and -2147483648 are treated the same and > ordering is thus sometimes wrong as in this > example: > > $ cat > numbers.txt <<EOF > 1 > -1 > 0 > -2147483648 > 2147483647 > > 2147483647 > > 2147483646 > -2147483648 > -2147483647 > -2147483646 > EOF > > Which gets ordered as: > > === > 2147483648 > > > -2147483648 > -2147483647 > -2147483646 > -1 > 0 > 1 > 2147483646 > 2147483647 > 2147483647 > === > > I'll see how this can be improved.
PR https://github.com/vim/vim/pull/4017 should fix the compilation warning and the incorrect numerical sort. Dominique -- -- You received this message from the "vim_dev" maillist. Do not top-post! Type your reply below the text you are replying to. For more information, visit http://www.vim.org/maillist.php --- You received this message because you are subscribed to the Google Groups "vim_dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
