[vim/vim] Add keylen to dictitem_T and dictitem16

John Marriott

unread,

Apr 24, 2026, 9:43:07 PM (4 days ago) Apr 24

to vim/vim, Subscribed

Function dictitem_alloc() calls STRLEN() on it's key argument. This function can be called directly or it can be called indirectly from one of the various dict_add_*() functions.

Approx 90+% of calls to dict_add_*() pass a string literal as it's key argument. This means that the vast majority of these calls are calling STRLEN() on strings for which we already know the length.

This PR addresses this by:
->add field size_t keylen to the dictitem_T and dictitem16_T structures.
->add size_t keylen argument to the dictitem_alloc() and the various dict_add_*() functions for the length of the key argument.

In an attempt to make the call sites tidier, add these macros which automatically populate the calls where a key is passed as a literal string:
DICTITEM_ALLOC()
DICT_ADD_KEY_LITERAL_STRING()
DICT_ADD_KEY_LITERAL_STRING_LEN()
DICT_ADD_KEY_LITERAL_NUMBER()
DICT_ADD_KEY_LITERAL_BOOL()
DICT_ADD_KEY_LITERAL_LIST()
DICT_ADD_KEY_LITERAL_TV()
DICT_ADD_KEY_LITERAL_CALLBACK()
DICT_ADD_KEY_LITERAL_FUNC()
DICT_ADD_KEY_LITERAL_DICT()

In addition:
For consistency, change the type of key to char_u to match it's definition in the structures.
In channel.c:
->In function channel_part_info(), use macro STR_LITERAL_SET().
In evalvars.c:
->Use struct string_T to store field vv_name in structure struct vimvar and initialise it appropriately. At the same time, initialise the di_key and di_keylen struct members at compile time, so we can drop the STRCPY() in evalvars_init().
In popupwin.c:
->Add namelen argument to function get_padding_border() which represents the length of argument name. In tag.c: ->Add field_namelenargument to functionadd_tag_field()which represents the length of argumentfield_name. ->Refactor function get_tags()to exit theforon error. Also, rename some variables to better reflect their purpose. Inuserfunc.c: ->Add namelenargument to functionadd_nr_var()which represents the length of argumentname. ->Use a string_Tto store variablename`.

Cheers
John

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

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

Commit Summary

d37722f Add keylen to dictitem_T and dictitem16_T
e71e6d0 Small updates

File Changes

(53 files)

M src/autocmd.c (14)
M src/buffer.c (5)
M src/change.c (16)
M src/channel.c (69)
M src/cmdexpand.c (8)
M src/dict.c (67)
M src/diff.c (8)
M src/eval.c (2)
M src/evalbuffer.c (30)
M src/evalfunc.c (56)
M src/evalvars.c (22)
M src/evalwindow.c (58)
M src/ex_docmd.c (2)
M src/fileio.c (42)
M src/getchar.c (4)
M src/highlight.c (40)
M src/if_lua.c (6)
M src/if_mzsch.c (5)
M src/if_ruby.c (4)
M src/insexpand.c (54)
M src/job.c (22)
M src/json.c (3)
M src/macros.h (11)
M src/map.c (32)
M src/mark.c (6)
M src/match.c (17)
M src/memline.c (41)
M src/menu.c (32)
M src/misc1.c (4)
M src/mouse.c (30)
M src/move.c (8)
M src/ops.c (24)
M src/option.c (9)
M src/popupmenu.c (12)
M src/popupwin.c (114)
M src/proto/dict.pro (20)
M src/quickfix.c (88)
M src/register.c (12)
M src/scriptfile.c (22)
M src/search.c (10)
M src/sign.c (40)
M src/structs.h (2)
M src/tag.c (106)
M src/term.c (3)
M src/terminal.c (18)
M src/testing.c (8)
M src/textprop.c (40)
M src/time.c (12)
M src/undo.c (26)
M src/userfunc.c (59)
M src/vim9execute.c (4)
M src/vim9expr.c (2)
M src/window.c (23)

Patch Links:

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.

John Marriott

unread,

Apr 24, 2026, 9:54:37 PM (4 days ago) Apr 24

to vim/vim, Push

@basilisk0315 pushed 1 commit.

0e7615c Fix build

—
View it on GitHub or unsubscribe.

Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

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

John Marriott

unread,

Apr 24, 2026, 10:09:18 PM (4 days ago) Apr 24

to vim/vim, Push

@basilisk0315 pushed 1 commit.

b405840 Fix build

—
View it on GitHub or unsubscribe.

Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

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

Christian Brabandt

unread,

Apr 27, 2026, 4:52:37 PM (21 hours ago) Apr 27

to vim/vim, Subscribed

@chrisbra commented on this pull request.

In src/evalvars.c:

> @@ -216,12 +216,11 @@ evalvars_init(void)
     for (i = 0; i < VV_LEN; ++i)
     {
 	p = &vimvars[i];
-	if (STRLEN(p->vv_name) > DICTITEM16_KEY_LEN)
+	if (p->vv_name.length > DICTITEM16_KEY_LEN)

Hm, I think this is will no longer fail, because the initialization is truncated at runtime.

In src/tag.c:

>  	{
 	    ret = FAIL;
-	    vim_free(matches[i]);

this is probably wrong?

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

Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.

Copilot

unread,

Apr 27, 2026, 4:59:08 PM (21 hours ago) Apr 27

to vim/vim, Subscribed

@Copilot commented on this pull request.

Pull request overview

This PR optimizes dictionary item/key handling by storing key lengths in dictitem_T/dictitem16_T and threading explicit keylen parameters through dictitem_alloc() and the various dict_add_*() helpers, reducing repeated STRLEN() calls (especially for literal keys).

Changes:

Add di_keylen to dictionary item structs and update allocation/copy paths to preserve it.
Extend dict_add_*() and dictitem_alloc() APIs to accept keylen, and update call sites (including adding literal-key convenience macros).
Refactor several call sites to use string_T / STR_LITERAL_SET() and safer snprintf length helpers.

Reviewed changes

Copilot reviewed 55 out of 55 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/window.c	Switch dict event population to DICT_ADD_* macros and pass explicit key lengths for dynamic keys.
src/vim9expr.c	Update `dictitem_alloc()` call to pass key length.
src/vim9execute.c	Update `dictitem_alloc()` and `dict_add_tv()` calls to pass key lengths.
src/userfunc.c	Thread name length into fixed-var dictitem setup; use `string_T` for names.
src/undo.c	Replace dict_add_* calls with DICT_ADD_* macros.
src/time.c	Replace dict_add_* calls with DICT_ADD_* macros and DICTITEM_ALLOC().
src/textprop.c	Replace dict_add_* calls with DICT_ADD_* macros (incl. *_LEN).
src/testing.c	Update dict_add_tv() to pass key length (avoid repeated STRLEN in one branch).
src/terminal.c	Replace dict_add_* calls with DICT_ADD_* macros.
src/term.c	Update dict_add_string_len() usage to pass key pointer as `char_u *` and key length.
src/tag.c	Add `field_namelen` plumbing for tag dict fields; refactor `get_tags()` error flow.
src/structs.h	Add `di_keylen` to `dictitem_T` and `dictitem16_T`.
src/sign.c	Replace dict_add_* calls with DICT_ADD_* macros.
src/search.c	Replace dict_add_number() calls with DICT_ADD_NUMBER().
src/scriptfile.c	Replace dict_add_* calls with DICT_ADD_* macros.
src/register.c	Replace dict_add_* calls with DICT_ADD_* macros.
src/quickfix.c	Replace dict_add_* calls with DICT_ADD_* macros; use DICTITEM_ALLOC().
src/proto/dict.pro	Update prototypes for `dictitem_alloc()` and `dict_add_*()` to include `keylen`.
src/popupwin.c	Replace dict_add_* calls with DICT_ADD_* macros; add `namelen` to `get_padding_border()`.
src/popupmenu.c	Replace dict_add_* calls with DICT_ADD_* macros.
src/option.c	Update option dict population to pass explicit key lengths.
src/ops.c	Replace dynamic-key `dict_add_number()` with explicit branches using DICT_ADD_NUMBER().
src/move.c	Replace dict_add_number() calls with DICT_ADD_NUMBER().
src/mouse.c	Replace dict_add_* calls with DICT_ADD_* macros.
src/misc1.c	Replace dict_add_string() calls with DICT_ADD_STRING().
src/menu.c	Replace dict_add_* calls with DICT_ADD_* macros.
src/memline.c	Replace dict_add_* calls with DICT_ADD_* macros; use `STR_LITERAL_SET()` for error strings.
src/match.c	Use `vim_snprintf_safelen()` for dynamic dict keys and switch to DICT_ADD_* macros elsewhere.
src/mark.c	Replace dict_add_* calls with DICT_ADD_* macros.
src/map.c	Replace dict_add_* calls with DICT_ADD_* macros.
src/macros.h	Add DICTITEM_ALLOC/DICT_ADD_* convenience macros based on `STRLEN_LITERAL()`.
src/json.c	Update `dictitem_alloc()` call to pass key length.
src/job.c	Replace dict_add_* calls with DICT_ADD_* macros; use DICTITEM_ALLOC().
src/insexpand.c	Replace dict_add_* calls with DICT_ADD_* macros and update completed-item event dict insertion.
src/if_ruby.c	Update `dictitem_alloc()` call to pass key length.
src/if_py_both.h	Update `dictitem_alloc()` calls to pass key length.
src/if_perl.xs	Update `dictitem_alloc()` call to pass key length.
src/if_mzsch.c	Update `dictitem_alloc()` call to pass key length and avoid duplicate key conversion call.
src/if_lua.c	Update `dictitem_alloc()` calls to pass key length.
src/highlight.c	Replace dict_add_* calls with DICT_ADD_* macros and pass key lengths for non-literal keys where needed.
src/getchar.c	Replace dict_add_* calls with DICT_ADD_* macros.
src/fileio.c	Replace dict_add_* calls with DICT_ADD_* macros.
src/ex_docmd.c	Replace dict_add_string() call with DICT_ADD_STRING().
src/evalwindow.c	Replace dict_add_* calls with DICT_ADD_* macros.
src/evalvars.c	Convert `vv_name` to `string_T` and attempt compile-time init of `vv_di` key/keylen; set `di_keylen` in `init_var_dict()` and new allocations.
src/evalfunc.c	Replace dict_add_* calls with DICT_ADD_* macros; update env dict insertion to pass key length.
src/evalbuffer.c	Replace dict_add_* calls with DICT_ADD_* macros.
src/eval.c	Update `dictitem_alloc()` call to pass key length.
src/diff.c	Replace dict_add_number() calls with DICT_ADD_NUMBER().
src/dict.c	Add keylen-based `dictitem_alloc()`; propagate keylen through dict_add_* APIs; use `di_keylen` in key-related operations.
src/cmdexpand.c	Replace dict_add_* calls with DICT_ADD_* macros.
src/channel.c	Refactor dynamic key construction using `string_T`/`STR_LITERAL_SET()` and pass explicit key lengths to dict_add_* APIs.
src/change.c	Replace dict_add_number() calls with DICT_ADD_NUMBER().
src/buffer.c	Initialize embedded dictitem key and `di_keylen` for `changedtick`.
src/autocmd.c	Replace dict_add_* calls with DICT_ADD_* macros.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

In src/evalvars.c:

> @@ -37,13 +37,13 @@ static hashtab_T	compat_hashtab;
 #define VV_RO		2	// read-only
 #define VV_RO_SBX	4	// read-only in the sandbox
 
-#define VV_NAME(s, t)	s, {{t, 0, {0}}, 0, {0}}
+#define VV_NAME(s, t)	STR_LITERAL_INIT(s), {{t, 0, {0}}, 0, STRLEN_LITERAL(s), {(s)}}

VV_NAME() initializes vv_di.di_key using {(s)}. Because (s) is an expression, the string literal decays to a pointer, which cannot legally initialize the char_u di_key[] array and is likely to fail compilation or mis-initialize the key. Use a real string-literal array initializer (e.g. {s} / s) and ensure the NUL terminator is included, while keeping di_keylen consistent.

⬇️ Suggested change

-#define VV_NAME(s, t)	STR_LITERAL_INIT(s), {{t, 0, {0}}, 0, STRLEN_LITERAL(s), {(s)}}
+#define VV_NAME(s, t)	STR_LITERAL_INIT(s), {{t, 0, {0}}, 0, STRLEN_LITERAL(s), s}

In src/if_perl.xs:

> +		    keylen = strlen(key);
+		    if (!key || !len || keylen < (size_t)len) {
 			semsg("Malformed key Dictionary '%s'", key && *key ? key : "(empty)");
 			break;
 		    }

keylen = strlen(key); is executed before checking key for NULL. This loses the short-circuit safety of the previous if (!key || ...) check and can segfault if hv_iterkey() returns NULL. Compute keylen only after confirming key != NULL (and ideally prefer using the len value returned by hv_iterkey() for the key length).

⬇️ Suggested change

-		    keylen = strlen(key);
-		    if (!key || !len || keylen < (size_t)len) {
-			semsg("Malformed key Dictionary '%s'", key && *key ? key : "(empty)");
-			break;
-		    }
+		    if (key == NULL || len <= 0) {
+			semsg("Malformed key Dictionary '%s'", key && *key ? key : "(empty)");
+			break;
+		    }
+		    keylen = (size_t)len;

In src/tag.c:

> +	if ((dict = dict_alloc()) == NULL ||
+	    list_append_dict(list, dict) == FAIL)
 	{
 	    ret = FAIL;
-	    vim_free(matches[i]);
 	    break;
 	}

On failure of dict_alloc() or list_append_dict(), the loop breaks without freeing matches[i] (and without unref/freeing dict when allocation succeeded but list_append_dict() failed). This introduces a leak in the error path. Split the allocation/append steps and add the appropriate cleanup before breaking.

In src/macros.h:

> +#define DICTITEM_ALLOC(key) dictitem_alloc((char_u *)(key), STRLEN_LITERAL(key))
+#define DICT_ADD_STRING(dict, key, str) dict_add_string((dict), (char_u *)(key), STRLEN_LITERAL(key), (str))
+#define DICT_ADD_STRING_LEN(dict, key, str, len) dict_add_string_len((dict), (char_u *)(key), STRLEN_LITERAL(key), (str), (len))
+#define DICT_ADD_NUMBER(dict, key, nr) dict_add_number((dict), (char_u *)(key), STRLEN_LITERAL(key), (nr))
+#define DICT_ADD_BOOL(dict, key, nr) dict_add_bool((dict), (char_u *)(key), STRLEN_LITERAL(key), (nr))
+#define DICT_ADD_LIST(dict, key, list) dict_add_list((dict), (char_u *)(key), STRLEN_LITERAL(key), (list))
+#define DICT_ADD_TV(dict, key, tv) dict_add_tv((dict), (char_u *)(key), STRLEN_LITERAL(key), (tv))

These DICT_* helper macros hardcode STRLEN_LITERAL(key), which silently produces the wrong length if key is not a literal (e.g. a char_u * variable), potentially causing out-of-bounds reads when dictitem_alloc() copies keylen + 1 bytes. Consider renaming the macros to make the “literal-only” contract explicit and/or adding a compile-time check to prevent passing pointers.

In src/macros.h:

> +#define DICTITEM_ALLOC(key) dictitem_alloc((char_u *)(key), STRLEN_LITERAL(key))
+#define DICT_ADD_STRING(dict, key, str) dict_add_string((dict), (char_u *)(key), STRLEN_LITERAL(key), (str))
+#define DICT_ADD_STRING_LEN(dict, key, str, len) dict_add_string_len((dict), (char_u *)(key), STRLEN_LITERAL(key), (str), (len))
+#define DICT_ADD_NUMBER(dict, key, nr) dict_add_number((dict), (char_u *)(key), STRLEN_LITERAL(key), (nr))
+#define DICT_ADD_BOOL(dict, key, nr) dict_add_bool((dict), (char_u *)(key), STRLEN_LITERAL(key), (nr))
+#define DICT_ADD_LIST(dict, key, list) dict_add_list((dict), (char_u *)(key), STRLEN_LITERAL(key), (list))
+#define DICT_ADD_TV(dict, key, tv) dict_add_tv((dict), (char_u *)(key), STRLEN_LITERAL(key), (tv))
+#define DICT_ADD_CALLBACK(dict, key, cb) dict_add_callback((dict), (char_u *)(key), STRLEN_LITERAL(key), (cb))
+#define DICT_ADD_FUNC(dict, key, func) dict_add_func((dict), (char_u *)(key), STRLEN_LITERAL(key), (func))
+#define DICT_ADD_DICT(dest_dict, key, src_dict) dict_add_dict((dest_dict), (char_u *)(key), STRLEN_LITERAL(key), (src_dict))
+

PR description mentions adding macros like DICT_ADD_KEY_LITERAL_*, but this change instead introduces generic-sounding DICT_ADD_STRING/NUMBER/... macros that only work correctly with literal keys. Either update the PR description to match the actual API or rename/add the macros as described, to avoid confusing future callers.

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

Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.

John Marriott

unread,

Apr 27, 2026, 9:00:26 PM (17 hours ago) Apr 27

to vim/vim, Subscribed

@basilisk0315 commented on this pull request.

In src/evalvars.c:

> @@ -216,12 +216,11 @@ evalvars_init(void)
     for (i = 0; i < VV_LEN; ++i)
     {
 	p = &vimvars[i];
-	if (STRLEN(p->vv_name) > DICTITEM16_KEY_LEN)
+	if (p->vv_name.length > DICTITEM16_KEY_LEN)

Yes.

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

Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.

John Marriott

unread,

Apr 27, 2026, 9:04:30 PM (17 hours ago) Apr 27

to vim/vim, Subscribed

@basilisk0315 commented on this pull request.

In src/tag.c:

>  	{
 	    ret = FAIL;
-	    vim_free(matches[i]);

Yes.

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

Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.

John Marriott

unread,

Apr 27, 2026, 9:05:05 PM (17 hours ago) Apr 27

to vim/vim, Push

@basilisk0315 pushed 1 commit.

37ceb70 Changes/fixes based on feedback

—
View it on GitHub or unsubscribe.

Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.

John Marriott

unread,

Apr 27, 2026, 9:20:56 PM (16 hours ago) Apr 27

to vim/vim, Push

@basilisk0315 pushed 1 commit.

d708060 Fix typo

—
View it on GitHub or unsubscribe.

Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.

h_east

unread,

8:16 AM (5 hours ago) 8:16 AM

to vim/vim, Subscribed

h-east left a comment (vim/vim#20062)

@basilisk0315
Hmm, to be honest, I think this might be overkill.
Dictionary keys aren't usually that long, so removing STRLEN() there probably won't provide a noticeable performance gain.
It would be counterproductive if the change makes the code harder to write or read.

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

Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.

Reply all

Reply to author

Forward

[vim/vim] Add keylen to dictitem_T and dictitem16_T (PR #20062)

John Marriott

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

Commit Summary

File Changes

Patch Links:

John Marriott

John Marriott

Christian Brabandt

Copilot

Pull request overview

Reviewed changes

John Marriott

John Marriott

John Marriott

John Marriott

h_east