Re: [vim/vim] Reduce number of calls to STRLEN() (Issue #14002)

2024-02-13 Fir de Conversatie Christian Brabandt


On Mi, 14 Feb 2024, John Marriott wrote:

> Hi Christian,
> 
> I have made my first PR (#14029).

Thank you!

> 
> When I did so, I got a 9 failing checks which mostly appear to be failing
> tests, like this:
> 
> command line..script
> /home/runner/work/vim/vim/src/testdir/runtest.vim[642]..function 
> RunTheTest[57]..Test_prop_below_split_line[25]..StopVimInTerminal[17]..WaitForAssert[2]..4_WaitForCommon[11]..38
> line 1: Expected 'finished' but got 'running'
> Flaky test failed too often, giving up
> 
> 
> I'm not sure how to proceed with this. This is my first PR.

No problem. If I check the CI on github, I see a few failures, including 
a buffer-overflow 
(https://github.com/vim/vim/actions/runs/7891958995/job/21537397938?pr=14029):
,
| READ of size 1 at 0x502abad3 thread T0
| #0 0x5610bb99642a in ___interceptor_strlen ??:?
| #1 0x5610bbaca046 in open_line 
/home/runner/work/vim/vim/src/change.c:1488:19
| #2 0x5610bbc064ea in ins_eol /home/runner/work/vim/vim/src/edit.c:5162:9
| #3 0x5610bbbf045d in edit /home/runner/work/vim/vim/src/edit.c:1230:10
| #4 0x5610bc01e110 in invoke_edit 
/home/runner/work/vim/vim/src/normal.c:7098:9
| #5 0x5610bbfd8fd9 in normal_cmd 
/home/runner/work/vim/vim/src/normal.c:949:5
| #6 0x5610bc8d1d18 in main_loop /home/runner/work/vim/vim/src/main.c
| #7 0x5610bc8cfc89 in vim_main2 /home/runner/work/vim/vim/src/main.c:895:5
| #8 0x5610bc8c825c in main /home/runner/work/vim/vim/src/main.c:441:12
| #9 0x7f4586229d8f in __libc_start_call_main 
csu/../sysdeps/nptl/libc_start_call_main.h:58:16
| #10 0x7f4586229e3f in __libc_start_main csu/../csu/libc-start.c:392:3
| #1 0x5610bb97ef94 in _start ??:?
`

This indicates that something serious got brocken. You may want to 
compile with ASAN enabled and run the test suite on your side to find 
out which test causes this. The fact that it is finished, could mean 
that the process segfaulted, in which case it would be finished indeed 
:)
I guess this may be the following test.
Test_prop_below_split_line(): Vim(call):E958: Job already finished @ 
command line..script 
/home/runner/work/vim/vim/src/testdir/runtest.vim[607]..function 
RunTheTest[57]..Test_prop_below_split_line[17]..VerifyScreenDump, line 35

Test_prop_below_split_line() sounds like it could invoke the open_line() 
function but I am just guessing here.

If I check the test, line 17:

,
| 1func Test_prop_below_split_line()
| 2  CheckRunVimInTerminal
| 3
| 4  let lines =<< trim END
| 5  vim9script
| 6  setline(1, ['one one one', 'two two two', 'three three three'])
| 7  prop_type_add('test', {highlight: 'Search'})
| 8  prop_add(2, 0, {
| 9  text:  '└─ Virtual text below the 2nd line',
| 10  type: 'test',
| 11  text_align: 'below',
| 12  text_padding_left: 3
| 13  })
| 14  END
| 15  call writefile(lines, 'XscriptPropBelowSpitLine', 'D')
| 16  let buf = RunVimInTerminal('-S XscriptPropBelowSpitLine', #{rows: 8})
| 17  call term_sendkeys(buf, "2GA\xx")
| 18  call VerifyScreenDump(buf, 'Test_prop_below_split_line_1', {})
| 19
| 20  call term_sendkeys(buf, "\:set number\")
| 21  call VerifyScreenDump(buf, 'Test_prop_below_split_line_2', {})
`

Line 17 is the line where we append a  to the buffer, so it could match.

Other than that, screendumps are unfortunately quite flaky, so it is 
okay, if only 1 or 2 tests from the suite fail, but 6 is too much :(

So try to run the failing tests in your local environment and see if you 
can debug what is going wrong. See the readme of the testsuite on how to 
run only a single test: 
https://github.com/vim/vim/blob/master/src/testdir/README.txt

Thanks,
Chris 
-- 
Youth is a blunder, manhood a struggle, old age a regret.
-- Benjamin Disraeli, "Coningsby"

-- 
-- 
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 vim_dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/vim_dev/ZcvYWXoFeIbiD33p%40256bit.org.


Re: [vim/vim] Reduce number of calls to STRLEN() (Issue #14002)

2024-02-13 Fir de Conversatie John Marriott


On 12-Feb-2024 02:32, Christian Brabandt (Vim Github Repository) wrote:


can you make a PR for this please?

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


--
--
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 vim_dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/vim_dev/vim/vim/issues/14002/1937786687%40github.com 
.

Hi Christian,

I have made my first PR (#14029).

When I did so, I got a 9 failing checks which mostly appear to be 
failing tests, like this:


command line..script 
/home/runner/work/vim/vim/src/testdir/runtest.vim[642]..function 
RunTheTest[57]..Test_prop_below_split_line[25]..StopVimInTerminal[17]..WaitForAssert[2]..4_WaitForCommon[11]..38 
line 1: Expected 'finished' but got 'running'

Flaky test failed too often, giving up


I'm not sure how to proceed with this. This is my first PR.

Any suggestions would be appreciated.

Cheers
John

--
--
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 vim_dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/vim_dev/be04c922-1adb-4f1c-a9a2-2642728fddae%40internode.on.net.


Commit: runtime(vim): include Vim Syntax generator

2024-02-13 Fir de Conversatie Christian Brabandt
runtime(vim): include Vim Syntax generator

Commit: 
https://github.com/vim/vim/commit/9b53c052d58f73f2078c61a74622687306e51c17
Author: h-east 
Date:   Tue Feb 13 21:09:22 2024 +0100

runtime(vim): include Vim Syntax generator

fixes: https://github.com/vim/vim/issues/13939
closes: https://github.com/vim/vim/issues/14021
related: vim-jp/syntax-vim-ex#28

Signed-off-by: h-east 
Signed-off-by: Christian Brabandt 

diff --git a/.gitignore b/.gitignore
index 3a55d25b1..5275e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -97,6 +97,11 @@ src/kword_test
 # Generated by "make install"
 runtime/doc/doctags
 
+# Temporarily generated by "runtime/syntax/generator/make"
+runtime/syntax/generator/generator.err
+runtime/syntax/generator/sanity_check.err
+runtime/syntax/generator/vim.vim.rc
+
 # Generated by "make shadow".  The directory names could be anything but we
 # restrict them to shadow (the default) or shadow-*
 src/shadow
diff --git a/Filelist b/Filelist
index 237923e87..acc931822 100644
--- a/Filelist
+++ b/Filelist
@@ -827,6 +827,11 @@ RT_SCRIPTS =   \
runtime/syntax/testdir/runtest.vim \
runtime/syntax/testdir/input/*.* \
runtime/syntax/testdir/dumps/*.dump \
+   runtime/syntax/generator/Makefile \
+   runtime/syntax/generator/README.md \
+   runtime/syntax/generator/gen_syntax_vim.vim \
+   runtime/syntax/generator/update_date.vim \
+   runtime/syntax/generator/vim.vim.base \
 
 # Unix runtime
 RT_UNIX =  \
diff --git a/nsis/gvim.nsi b/nsis/gvim.nsi
index 72252527c..cb0aef9c3 100644
--- a/nsis/gvim.nsi
+++ b/nsis/gvim.nsi
@@ -419,7 +419,7 @@ Section "$(str_section_exe)" id_section_exe
File ${VIMSRC} im.ico
 
SetOutPath $0\syntax
-   File /r /x testdir ${VIMRT}\syntax\*.*
+   File /r /x testdir /x generator ${VIMRT}\syntax\*.*
 
SetOutPath $0\spell
File ${VIMRT}\spell\*.txt
diff --git a/runtime/syntax/generator/Makefile 
b/runtime/syntax/generator/Makefile
new file mode 100644
index 0..33dcfbc2b
--- /dev/null
+++ b/runtime/syntax/generator/Makefile
@@ -0,0 +1,44 @@
+VIM_SRCDIR = ../../../src
+RUN_VIM = $(VIM_SRCDIR)/vim -N -u NONE -i NONE -n
+REVISION ?= $(shell date +%Y-%m-%dT%H:%M:%S%:z)
+
+SRC =  $(VIM_SRCDIR)/eval.c $(VIM_SRCDIR)/ex_cmds.h $(VIM_SRCDIR)/ex_docmd.c \
+   $(VIM_SRCDIR)/fileio.c $(VIM_SRCDIR)/option.c 
$(VIM_SRCDIR)/syntax.c
+
+export VIM_SRCDIR
+
+.PHONY: generate clean
+all: generate
+
+generate: vim.vim
+
+vim.vim: vim.vim.rc update_date.vim
+   @echo "Generating vim.vim ..."
+   @cp -f vim.vim.rc ../vim.vim
+   @$(RUN_VIM) -S update_date.vim
+   @sed -i -e 's/__REVISION__/$(REVISION)/' ../vim.vim
+   @echo "done."
+
+vim.vim.rc: gen_syntax_vim.vim vim.vim.base $(SRC)
+   @echo "Generating vim.vim.rc ..."
+   @rm -f sanity_check.err generator.err
+   @$(RUN_VIM) -S gen_syntax_vim.vim
+   @if test -f sanity_check.err ; then \
+   echo ; \
+   echo "Sanity errors:" ; \
+   cat sanity_check.err ; \
+   exit 1 ; \
+   fi
+   @if test -f generator.err ; then \
+   echo ; \
+   echo "Generator errors:" ; \
+   cat generator.err ; \
+   echo ; \
+   exit 1 ; \
+   fi
+   @echo "done."
+
+clean:
+   rm -f vim.vim.rc
+   rm -f vim.vim
+   rm -f sanity_check.err generator.err
diff --git a/runtime/syntax/generator/README.md 
b/runtime/syntax/generator/README.md
new file mode 100644
index 0..83acedabe
--- /dev/null
+++ b/runtime/syntax/generator/README.md
@@ -0,0 +1,26 @@
+# Generator of Vim Script Syntax File
+
+This directory contains a Vim Script generator, that will parse the Vim source 
file and
+generate a vim.vim syntax file.
+
+Files in this directory where copied from 
https://github.com/vim-jp/syntax-vim-ex/
+and included here on Feb, 13th, 2024 for the Vim Project.
+
+- Maintainer: Hirohito Higashi
+- License: Vim License
+
+## How to generate
+
+$ make
+
+This will generate `../vim.vim`
+
+## Files
+
+Name |Description
+-|--
+`Makefile`   |Makefile to generate ../vim.vim
+`README.md`  |This file
+`gen_syntax_vim.vim` |Script to generate vim.vim
+`update_date.vim`|Script to update "Last Change:"
+`vim.vim.base`   |Template for vim.vim
diff --git a/runtime/syntax/generator/gen_syntax_vim.vim 
b/runtime/syntax/generator/gen_syntax_vim.vim
new file mode 100644
index 0..85f094552
--- /dev/null
+++ b/runtime/syntax/generator/gen_syntax_vim.vim
@@ -0,0 +1,694 @@
+" Vim syntax file generator
+" Language: Vim script
+" Maintainer: Hirohito Higashi (h_east)
+" URL: https://github.com/vim-jp/syntax-vim-ex
+" Last Change: Feb 11, 2024
+" Version: 2.0.0
+
+let s:keepcpo= 
+set cpo
+
+language C

Commit: patch 9.1.0105: Style: typos found

2024-02-13 Fir de Conversatie Christian Brabandt
patch 9.1.0105: Style: typos found

Commit: 
https://github.com/vim/vim/commit/e71022082d6a8bd8ec3d7b9dadf3f9ce46ef339c
Author: zeertzjq 
Date:   Tue Feb 13 20:32:04 2024 +0100

patch 9.1.0105: Style: typos found

Problem:  Style: typos found
Solution: correct them
  (zeertzjq)

closes: #14023

Signed-off-by: zeertzjq 
Signed-off-by: Christian Brabandt 

diff --git a/src/regexp.c b/src/regexp.c
index 73552015e..4373ae0cf 100644
--- a/src/regexp.c
+++ b/src/regexp.c
@@ -1318,8 +1318,7 @@ reg_match_visual(void)
top = curbuf->b_visual.vi_end;
bot = curbuf->b_visual.vi_start;
}
-   // a substitue command may have
-   // removed some lines
+   // a substitute command may have removed some lines
if (bot.lnum > curbuf->b_ml.ml_line_count)
bot.lnum = curbuf->b_ml.ml_line_count;
mode = curbuf->b_visual.vi_mode;
diff --git a/src/testdir/test_mapping.vim b/src/testdir/test_mapping.vim
index e361f3e65..71d90468b 100644
--- a/src/testdir/test_mapping.vim
+++ b/src/testdir/test_mapping.vim
@@ -120,7 +120,7 @@ func Test_map_langmap()
   unmap x
   bwipe!
 
-  " 'langnoremap' follows 'langremap' and vise versa
+  " 'langnoremap' follows 'langremap' and vice versa
   set langremap
   set langnoremap
   call assert_equal(0, )
diff --git a/src/testdir/test_utf8_comparisons.vim 
b/src/testdir/test_utf8_comparisons.vim
index 20b5762c9..2c1972b08 100644
--- a/src/testdir/test_utf8_comparisons.vim
+++ b/src/testdir/test_utf8_comparisons.vim
@@ -93,7 +93,7 @@ func Test_gap()
   call assert_equal(["ABCD", "", "defg"], getline(1,3))
 endfunc
 
-" test that g~, ~ and gU correclty upper-cases ß
+" test that g~, ~ and gU correctly upper-cases ß
 func Test_uppercase_sharp_ss()
   new
   call setline(1, repeat(['ß'], 4))
diff --git a/src/testdir/test_vim9_class.vim b/src/testdir/test_vim9_class.vim
index 62a6d043d..0bf7e9ceb 100644
--- a/src/testdir/test_vim9_class.vim
+++ b/src/testdir/test_vim9_class.vim
@@ -67,7 +67,7 @@ def Test_class_basic()
   END
   v9.CheckSourceFailure(lines, "E488: Trailing characters: | echo 'done'", 3)
 
-  # Use old "this." prefixed member variable declaration syntax (without 
intialization)
+  # Use old "this." prefixed member variable declaration syntax (without 
initialization)
   lines =<< trim END
 vim9script
 class Something
@@ -76,7 +76,7 @@ def Test_class_basic()
   END
   v9.CheckSourceFailure(lines, 'E1318: Not a valid command in a class: 
this.count: number', 3)
 
-  # Use old "this." prefixed member variable declaration syntax (with 
intialization)
+  # Use old "this." prefixed member variable declaration syntax (with 
initialization)
   lines =<< trim END
 vim9script
 class Something
diff --git a/src/testdir/test_vim9_typealias.vim 
b/src/testdir/test_vim9_typealias.vim
index 41557445c..998079cf6 100644
--- a/src/testdir/test_vim9_typealias.vim
+++ b/src/testdir/test_vim9_typealias.vim
@@ -641,7 +641,7 @@ def Test_type_as_func_argument_or_return_value()
   END
   v9.CheckScriptFailure(lines, 'E1407: Cannot use a Typealias as a variable or 
value', 1)
 
-  # check defered function using typealias as arg
+  # check deferred function using typealias as arg
   lines =<< trim END
 vim9script
 type A = number
@@ -764,7 +764,7 @@ def Test_class_as_func_argument_or_return_value()
   END
   v9.CheckScriptFailure(lines, 'E1405: Class "C" cannot be used as a value', 1)
 
-  # check defered function using class typealias as arg
+  # check deferred function using class typealias as arg
   lines =<< trim END
 vim9script
 class C
diff --git a/src/testdir/test_visual.vim b/src/testdir/test_visual.vim
index 066d7ebb2..17f0fd068 100644
--- a/src/testdir/test_visual.vim
+++ b/src/testdir/test_visual.vim
@@ -1008,7 +1008,7 @@ endfunc
 " Test for changing case
 func Test_visual_change_case()
   new
-  " gUe must uppercase a whole word, also when ß changes to SS
+  " gUe must uppercase a whole word, also when ß changes to ẞ
   exe "normal Gothe youtußeuu end\Ypk0wgUe
"
   " gUfx must uppercase until x, inclusive.
   exe "normal O- youßtußexu -\0fogUfx
"
diff --git a/src/version.c b/src/version.c
index 17a789948..65b653099 100644
--- a/src/version.c
+++ b/src/version.c
@@ -704,6 +704,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+105,
 /**/
 104,
 /**/

-- 
-- 
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 vim_dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/vim_dev/E1rZyiI-00D8TH-1T%40256bit.org.