refactor(options): use hashy for finding options (#26573)

Problem:
`findoption()` searches through the options[] table linearly for option
names, even though hashy can be used to generate a compile-time hash
table for it.

Solution:
Use hashy to generate a compile time hash table for finding options.
This also allows handling option aliases, so we don't need separate
options[] table entries for things like 'viminfo'.
This commit is contained in:
Famiu Haque 2023-12-17 05:23:33 +06:00 committed by GitHub
parent 2b1bc94b76
commit 8f08b1efbd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 125 additions and 139 deletions

View File

@ -303,10 +303,12 @@ set(GENERATED_EVENTS_ENUM ${GENERATED_INCLUDES_DIR}/auevents_enum.generated.h)
set(GENERATED_EVENTS_NAMES_MAP ${GENERATED_DIR}/auevents_name_map.generated.h) set(GENERATED_EVENTS_NAMES_MAP ${GENERATED_DIR}/auevents_name_map.generated.h)
set(GENERATED_OPTIONS ${GENERATED_DIR}/options.generated.h) set(GENERATED_OPTIONS ${GENERATED_DIR}/options.generated.h)
set(GENERATED_OPTIONS_ENUM ${GENERATED_DIR}/options_enum.generated.h) set(GENERATED_OPTIONS_ENUM ${GENERATED_DIR}/options_enum.generated.h)
set(GENERATED_OPTIONS_MAP ${GENERATED_DIR}/options_map.generated.h)
set(EX_CMDS_GENERATOR ${GENERATOR_DIR}/gen_ex_cmds.lua) set(EX_CMDS_GENERATOR ${GENERATOR_DIR}/gen_ex_cmds.lua)
set(FUNCS_GENERATOR ${GENERATOR_DIR}/gen_eval.lua) set(FUNCS_GENERATOR ${GENERATOR_DIR}/gen_eval.lua)
set(EVENTS_GENERATOR ${GENERATOR_DIR}/gen_events.lua) set(EVENTS_GENERATOR ${GENERATOR_DIR}/gen_events.lua)
set(OPTIONS_GENERATOR ${GENERATOR_DIR}/gen_options.lua) set(OPTIONS_GENERATOR ${GENERATOR_DIR}/gen_options.lua)
set(OPTIONS_ENUM_GENERATOR ${GENERATOR_DIR}/gen_options_enum.lua)
set(UNICODE_TABLES_GENERATOR ${GENERATOR_DIR}/gen_unicode_tables.lua) set(UNICODE_TABLES_GENERATOR ${GENERATOR_DIR}/gen_unicode_tables.lua)
set(UNICODE_DIR ${PROJECT_SOURCE_DIR}/src/unicode) set(UNICODE_DIR ${PROJECT_SOURCE_DIR}/src/unicode)
set(GENERATED_UNICODE_TABLES ${GENERATED_DIR}/unicode_tables.generated.h) set(GENERATED_UNICODE_TABLES ${GENERATED_DIR}/unicode_tables.generated.h)
@ -615,6 +617,7 @@ list(APPEND NVIM_GENERATED_FOR_HEADERS
"${GENERATED_EX_CMDS_ENUM}" "${GENERATED_EX_CMDS_ENUM}"
"${GENERATED_EVENTS_ENUM}" "${GENERATED_EVENTS_ENUM}"
"${GENERATED_KEYSETS_DEFS}" "${GENERATED_KEYSETS_DEFS}"
"${GENERATED_OPTIONS_ENUM}"
) )
list(APPEND NVIM_GENERATED_FOR_SOURCES list(APPEND NVIM_GENERATED_FOR_SOURCES
@ -622,6 +625,7 @@ list(APPEND NVIM_GENERATED_FOR_SOURCES
"${GENERATED_EX_CMDS_DEFS}" "${GENERATED_EX_CMDS_DEFS}"
"${GENERATED_EVENTS_NAMES_MAP}" "${GENERATED_EVENTS_NAMES_MAP}"
"${GENERATED_OPTIONS}" "${GENERATED_OPTIONS}"
"${GENERATED_OPTIONS_MAP}"
"${GENERATED_UNICODE_TABLES}" "${GENERATED_UNICODE_TABLES}"
"${VIM_MODULE_FILE}" "${VIM_MODULE_FILE}"
) )
@ -647,11 +651,16 @@ add_custom_command(OUTPUT ${GENERATED_EVENTS_ENUM} ${GENERATED_EVENTS_NAMES_MAP}
DEPENDS ${LUA_GEN_DEPS} ${EVENTS_GENERATOR} ${CMAKE_CURRENT_LIST_DIR}/auevents.lua DEPENDS ${LUA_GEN_DEPS} ${EVENTS_GENERATOR} ${CMAKE_CURRENT_LIST_DIR}/auevents.lua
) )
add_custom_command(OUTPUT ${GENERATED_OPTIONS} ${GENERATED_OPTIONS_ENUM} add_custom_command(OUTPUT ${GENERATED_OPTIONS}
COMMAND ${LUA_GEN} ${OPTIONS_GENERATOR} ${GENERATED_OPTIONS} ${GENERATED_OPTIONS_ENUM} COMMAND ${LUA_GEN} ${OPTIONS_GENERATOR} ${GENERATED_OPTIONS}
DEPENDS ${LUA_GEN_DEPS} ${OPTIONS_GENERATOR} ${CMAKE_CURRENT_LIST_DIR}/options.lua DEPENDS ${LUA_GEN_DEPS} ${OPTIONS_GENERATOR} ${CMAKE_CURRENT_LIST_DIR}/options.lua
) )
add_custom_command(OUTPUT ${GENERATED_OPTIONS_ENUM} ${GENERATED_OPTIONS_MAP}
COMMAND ${LUA_GEN} ${OPTIONS_ENUM_GENERATOR} ${GENERATED_OPTIONS_ENUM} ${GENERATED_OPTIONS_MAP}
DEPENDS ${LUA_GEN_DEPS} ${OPTIONS_ENUM_GENERATOR} ${CMAKE_CURRENT_LIST_DIR}/options.lua
)
# NVIM_GENERATED_FOR_SOURCES and NVIM_GENERATED_FOR_HEADERS must be mutually exclusive. # NVIM_GENERATED_FOR_SOURCES and NVIM_GENERATED_FOR_HEADERS must be mutually exclusive.
foreach(hfile ${NVIM_GENERATED_FOR_HEADERS}) foreach(hfile ${NVIM_GENERATED_FOR_HEADERS})
list(FIND NVIM_GENERATED_FOR_SOURCES ${hfile} hfile_idx) list(FIND NVIM_GENERATED_FOR_SOURCES ${hfile} hfile_idx)

View File

@ -643,7 +643,7 @@ static Object get_option_from(void *from, OptReqScope req_scope, String name, Er
return (Object)OBJECT_INIT; return (Object)OBJECT_INIT;
}); });
OptVal value = get_option_value_strict(findoption(name.data), req_scope, from, err); OptVal value = get_option_value_strict(find_option(name.data), req_scope, from, err);
if (ERROR_SET(err)) { if (ERROR_SET(err)) {
return (Object)OBJECT_INIT; return (Object)OBJECT_INIT;
} }
@ -669,7 +669,7 @@ static void set_option_to(uint64_t channel_id, void *to, OptReqScope req_scope,
return; return;
}); });
OptIndex opt_idx = findoption(name.data); OptIndex opt_idx = find_option(name.data);
VALIDATE_S(opt_idx != kOptInvalid, "option name", name.data, { VALIDATE_S(opt_idx != kOptInvalid, "option name", name.data, {
return; return;
}); });

View File

@ -79,7 +79,7 @@ static int validate_option_value_args(Dict(option) *opts, char *name, OptIndex *
return FAIL; return FAIL;
}); });
*opt_idxp = findoption(name); *opt_idxp = find_option(name);
int flags = get_option_attrs(*opt_idxp); int flags = get_option_attrs(*opt_idxp);
if (flags == 0) { if (flags == 0) {
// hidden or unknown option // hidden or unknown option

View File

@ -1943,7 +1943,7 @@ typval_T optval_as_tv(OptVal value, bool numbool)
/// Set option "varname" to the value of "varp" for the current buffer/window. /// Set option "varname" to the value of "varp" for the current buffer/window.
static void set_option_from_tv(const char *varname, typval_T *varp) static void set_option_from_tv(const char *varname, typval_T *varp)
{ {
OptIndex opt_idx = findoption(varname); OptIndex opt_idx = find_option(varname);
if (opt_idx == kOptInvalid) { if (opt_idx == kOptInvalid) {
semsg(_(e_unknown_option2), varname); semsg(_(e_unknown_option2), varname);
return; return;

View File

@ -1,5 +1,4 @@
local options_file = arg[1] local options_file = arg[1]
local options_enum_file = arg[2]
local opt_fd = assert(io.open(options_file, 'w')) local opt_fd = assert(io.open(options_file, 'w'))
@ -171,7 +170,7 @@ local function dump_option(i, o)
end end
if o.varname then if o.varname then
w(' .var=&' .. o.varname) w(' .var=&' .. o.varname)
-- Immutable options should directly point to the default value -- Immutable options can directly point to the default value.
elseif o.immutable then elseif o.immutable then
w((' .var=&options[%u].def_val'):format(i - 1)) w((' .var=&options[%u].def_val'):format(i - 1))
elseif #o.scope == 1 and o.scope[1] == 'window' then elseif #o.scope == 1 and o.scope[1] == 'window' then
@ -248,19 +247,3 @@ w('')
for _, v in ipairs(defines) do for _, v in ipairs(defines) do
w('#define ' .. v[1] .. ' ' .. v[2]) w('#define ' .. v[1] .. ' ' .. v[2])
end end
-- Generate options enum file
opt_fd = assert(io.open(options_enum_file, 'w'))
w('typedef enum {')
w(' kOptInvalid = -1,')
for i, o in ipairs(options.options) do
w((' kOpt%s = %u,'):format(lowercase_to_titlecase(o.full_name), i - 1))
end
w(' // Option count, used when iterating through options')
w('#define kOptIndexCount ' .. tostring(#options.options))
w('} OptIndex;')
opt_fd:close()

View File

@ -0,0 +1,76 @@
-- Generates option index enum and map of option name to option index.
-- Handles option full name, short name and aliases.
local options_enum_file = arg[1]
local options_map_file = arg[2]
local options_enum_fd = assert(io.open(options_enum_file, 'w'))
local options_map_fd = assert(io.open(options_map_file, 'w'))
--- @param s string
local function enum_w(s)
options_enum_fd:write(s .. '\n')
end
--- @param s string
local function map_w(s)
options_map_fd:write(s .. '\n')
end
--- @param s string
--- @return string
local lowercase_to_titlecase = function(s)
return s:sub(1, 1):upper() .. s:sub(2)
end
--- @type vim.option_meta[]
local options = require('options').options
--- @type { [string]: string }
local option_index = {}
-- Generate option index enum and populate the `option_index` dictionary.
enum_w('typedef enum {')
enum_w(' kOptInvalid = -1,')
for i, o in ipairs(options) do
local enum_val_name = 'kOpt' .. lowercase_to_titlecase(o.full_name)
enum_w((' %s = %u,'):format(enum_val_name, i - 1))
option_index[o.full_name] = enum_val_name
if o.abbreviation then
option_index[o.abbreviation] = enum_val_name
end
if o.alias then
o.alias = type(o.alias) == 'string' and { o.alias } or o.alias
for _, v in ipairs(o.alias) do
option_index[v] = enum_val_name
end
end
end
enum_w(' // Option count, used when iterating through options')
enum_w('#define kOptIndexCount ' .. tostring(#options))
enum_w('} OptIndex;')
enum_w('')
options_enum_fd:close()
--- Generate option index map.
local hashy = require('generators.hashy')
local neworder, hashfun = hashy.hashy_hash('find_option', vim.tbl_keys(option_index), function(idx)
return ('option_hash_elems[%s].name'):format(idx)
end)
map_w('static const struct { const char *name; OptIndex opt_idx; } option_hash_elems[] = {')
for _, name in ipairs(neworder) do
assert(option_index[name] ~= nil)
map_w((' { .name = "%s", .opt_idx = %s },'):format(name, option_index[name]))
end
map_w('};\n')
map_w('static ' .. hashfun)
options_map_fd:close()

View File

@ -1,8 +1,8 @@
// User-settable options. Checklist for adding a new option: // User-settable options. Checklist for adding a new option:
// - Put it in options.lua // - Put it in options.lua
// - For a global option: Add a variable for it in option_defs.h. // - For a global option: Add a variable for it in option_vars.h.
// - For a buffer or window local option: // - For a buffer or window local option:
// - Add a BV_XX or WV_XX entry to option_defs.h // - Add a BV_XX or WV_XX entry to option_vars.h
// - Add a variable to the window or buffer struct in buffer_defs.h. // - Add a variable to the window or buffer struct in buffer_defs.h.
// - For a window option, add some code to copy_winopt(). // - For a window option, add some code to copy_winopt().
// - For a window string option, add code to check_winopt() // - For a window string option, add code to check_winopt()
@ -12,7 +12,7 @@
// - For a buffer string option, add code to check_buf_options(). // - For a buffer string option, add code to check_buf_options().
// - If it's a numeric option, add any necessary bounds checks to check_num_option_bounds(). // - If it's a numeric option, add any necessary bounds checks to check_num_option_bounds().
// - If it's a list of flags, add some code in do_set(), search for WW_ALL. // - If it's a list of flags, add some code in do_set(), search for WW_ALL.
// - Add documentation! doc/options.txt, and any other related places. // - Add documentation! "desc" in options.lua, and any other related places.
// - Add an entry in runtime/optwin.vim. // - Add an entry in runtime/optwin.vim.
#define IN_OPTION_C #define IN_OPTION_C
@ -143,15 +143,13 @@ typedef enum {
# include "option.c.generated.h" # include "option.c.generated.h"
#endif #endif
// options[] is initialized here. // options[] is initialized in options.generated.h.
// The order of the options MUST be alphabetic for ":set all" and findoption().
// All option names MUST start with a lowercase letter (for findoption()).
// Exception: "t_" options are at the end.
// The options with a NULL variable are 'hidden': a set command for them is // The options with a NULL variable are 'hidden': a set command for them is
// ignored and they are not printed. // ignored and they are not printed.
#ifdef INCLUDE_GENERATED_DECLARATIONS #ifdef INCLUDE_GENERATED_DECLARATIONS
# include "options.generated.h" # include "options.generated.h"
# include "options_map.generated.h"
#endif #endif
static int p_bin_dep_opts[] = { static int p_bin_dep_opts[] = {
@ -664,7 +662,7 @@ void set_init_3(void)
} }
if (buf_is_empty(curbuf)) { if (buf_is_empty(curbuf)) {
int idx_ffs = findoption_len(S_LEN("ffs")); int idx_ffs = find_option("ffs");
// Apply the first entry of 'fileformats' to the initial buffer. // Apply the first entry of 'fileformats' to the initial buffer.
if (idx_ffs >= 0 && (options[idx_ffs].flags & P_WAS_SET)) { if (idx_ffs >= 0 && (options[idx_ffs].flags & P_WAS_SET)) {
@ -1132,7 +1130,7 @@ const char *find_option_end(const char *arg, OptIndex *opt_idxp)
p++; p++;
} }
*opt_idxp = findoption_len(arg, (size_t)(p - arg)); *opt_idxp = find_option_len(arg, (size_t)(p - arg));
return p; return p;
} }
@ -3009,87 +3007,6 @@ void check_redraw(uint32_t flags)
check_redraw_for(curbuf, curwin, flags); check_redraw_for(curbuf, curwin, flags);
} }
/// Find index for named option
///
/// @param[in] arg Option to find index for.
/// @param[in] len Length of the option.
///
/// @return Index of the option or kOptInvalid if option was not found.
OptIndex findoption_len(const char *const arg, const size_t len)
{
const char *s;
static int quick_tab[27] = { 0, 0 }; // quick access table
// For first call: Initialize the quick-access table.
// It contains the index for the first option that starts with a certain
// letter. There are 26 letters, plus the first "t_" option.
if (quick_tab[1] == 0) {
const char *p = options[0].fullname;
for (OptIndex i = 1; i < kOptIndexCount; i++) {
s = options[i].fullname;
if (s[0] != p[0]) {
if (s[0] == 't' && s[1] == '_') {
quick_tab[26] = i;
} else {
quick_tab[CHAR_ORD_LOW(s[0])] = i;
}
}
p = s;
}
}
// Check for name starting with an illegal character.
if (len == 0 || arg[0] < 'a' || arg[0] > 'z') {
return kOptInvalid;
}
OptIndex opt_idx;
const bool is_term_opt = (len > 2 && arg[0] == 't' && arg[1] == '_');
if (is_term_opt) {
opt_idx = quick_tab[26];
} else {
opt_idx = quick_tab[CHAR_ORD_LOW(arg[0])];
}
// Match full name
for (; opt_idx < kOptIndexCount; opt_idx++) {
s = options[opt_idx].fullname;
// Break if first character no longer matches.
if (s[0] != arg[0]) {
opt_idx = kOptIndexCount;
break;
}
if (strncmp(arg, s, len) == 0 && s[len] == NUL) {
break;
}
}
if (opt_idx == kOptIndexCount && !is_term_opt) {
opt_idx = quick_tab[CHAR_ORD_LOW(arg[0])];
// Match short name
for (; opt_idx < kOptIndexCount; opt_idx++) {
s = options[opt_idx].shortname;
if (s != NULL && strncmp(arg, s, len) == 0 && s[len] == NUL) {
break;
}
}
}
if (opt_idx == kOptIndexCount) {
opt_idx = kOptInvalid;
} else {
// Nvim: handle option aliases.
if (strncmp(options[opt_idx].fullname, "viminfo", 7) == 0) {
if (strlen(options[opt_idx].fullname) == 7) {
return findoption_len("shada", 5);
}
assert(strcmp(options[opt_idx].fullname, "viminfofile") == 0);
return findoption_len("shadafile", 9);
}
}
return opt_idx;
}
bool is_tty_option(const char *name) bool is_tty_option(const char *name)
FUNC_ATTR_NONNULL_ALL FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ALL FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT
{ {
@ -3147,15 +3064,28 @@ bool set_tty_option(const char *name, char *value)
return false; return false;
} }
/// Find index for an option. /// Find index for an option. Don't go beyond `len` length.
/// ///
/// @param[in] arg Option name. /// @param[in] name Option name.
/// @param len Option name length.
/// ///
/// @return Option index or kOptInvalid if option was not found. /// @return Option index or kOptInvalid if option was not found.
OptIndex findoption(const char *const arg) OptIndex find_option_len(const char *const name, size_t len)
FUNC_ATTR_NONNULL_ALL FUNC_ATTR_NONNULL_ALL
{ {
return findoption_len(arg, strlen(arg)); int index = find_option_hash(name, len);
return index >= 0 ? option_hash_elems[index].opt_idx : kOptInvalid;
}
/// Find index for an option.
///
/// @param[in] name Option name.
///
/// @return Option index or kOptInvalid if option was not found.
OptIndex find_option(const char *const name)
FUNC_ATTR_NONNULL_ALL
{
return find_option_len(name, strlen(name));
} }
/// Free an allocated OptVal. /// Free an allocated OptVal.
@ -5229,7 +5159,7 @@ void set_context_in_set_cmd(expand_T *xp, char *arg, int opt_flags)
return; return;
} }
nextchar = *p; nextchar = *p;
opt_idx = findoption_len(arg, (size_t)(p - arg)); opt_idx = find_option_len(arg, (size_t)(p - arg));
if (opt_idx == kOptInvalid || options[opt_idx].var == NULL) { if (opt_idx == kOptInvalid || options[opt_idx].var == NULL) {
xp->xp_context = EXPAND_NOTHING; xp->xp_context = EXPAND_NOTHING;
return; return;
@ -5534,7 +5464,7 @@ int ExpandOldSetting(int *numMatches, char ***matches)
// For a terminal key code expand_option_idx is kOptInvalid. // For a terminal key code expand_option_idx is kOptInvalid.
if (expand_option_idx == kOptInvalid) { if (expand_option_idx == kOptInvalid) {
expand_option_idx = findoption(expand_option_name); expand_option_idx = find_option(expand_option_name);
} }
if (expand_option_idx != kOptInvalid) { if (expand_option_idx != kOptInvalid) {
@ -6146,7 +6076,7 @@ int get_sidescrolloff_value(win_T *wp)
Dictionary get_vimoption(String name, int scope, buf_T *buf, win_T *win, Error *err) Dictionary get_vimoption(String name, int scope, buf_T *buf, win_T *win, Error *err)
{ {
OptIndex opt_idx = findoption_len(name.data, name.size); OptIndex opt_idx = find_option_len(name.data, name.size);
VALIDATE_S(opt_idx != kOptInvalid, "option (not found)", name.data, { VALIDATE_S(opt_idx != kOptInvalid, "option (not found)", name.data, {
return (Dictionary)ARRAY_DICT_INIT; return (Dictionary)ARRAY_DICT_INIT;
}); });

View File

@ -134,7 +134,7 @@ static inline bool option_has_type(OptIndex opt_idx, OptValType type)
// Ensure that the type is valid before accessing type_flags. // Ensure that the type is valid before accessing type_flags.
assert(type > kOptValTypeNil && type < kOptValTypeSize); assert(type > kOptValTypeNil && type < kOptValTypeSize);
// Bitshift 1 by the value of type to get the type's corresponding flag, and check if it's set in // Bitshift 1 by the value of type to get the type's corresponding flag, and check if it's set in
// the type_flags bit_field. // the type_flags bit field.
return get_option(opt_idx)->type_flags & (1 << type); return get_option(opt_idx)->type_flags & (1 << type);
} }

View File

@ -2,6 +2,7 @@
--- @field full_name string --- @field full_name string
--- @field desc? string --- @field desc? string
--- @field abbreviation? string --- @field abbreviation? string
--- @field alias? string|string[]
--- @field short_desc? string|fun(): string --- @field short_desc? string|fun(): string
--- @field varname? string --- @field varname? string
--- @field pv_name? string --- @field pv_name? string
@ -84,6 +85,7 @@ end
return { return {
cstr = cstr, cstr = cstr,
--- @type vim.option_meta[] --- @type vim.option_meta[]
--- The order of the options MUST be alphabetic for ":set all".
options = { options = {
{ {
abbreviation = 'al', abbreviation = 'al',
@ -6809,6 +6811,7 @@ return {
}, },
{ {
abbreviation = 'sd', abbreviation = 'sd',
alias = { 'vi', 'viminfo' },
cb = 'did_set_shada', cb = 'did_set_shada',
defaults = { defaults = {
if_true = "!,'100,<50,s10,h", if_true = "!,'100,<50,s10,h",
@ -6940,6 +6943,7 @@ return {
}, },
{ {
abbreviation = 'sdf', abbreviation = 'sdf',
alias = { 'vif', 'viminfofile' },
defaults = { if_true = '' }, defaults = { if_true = '' },
deny_duplicates = true, deny_duplicates = true,
desc = [=[ desc = [=[
@ -9374,22 +9378,6 @@ return {
type = 'string', type = 'string',
varname = 'p_vop', varname = 'p_vop',
}, },
{
abbreviation = 'vi',
full_name = 'viminfo',
nodefault = true,
scope = { 'global' },
short_desc = N_('Alias for shada'),
type = 'string',
},
{
abbreviation = 'vif',
full_name = 'viminfofile',
nodefault = true,
scope = { 'global' },
short_desc = N_('Alias for shadafile instead'),
type = 'string',
},
{ {
abbreviation = 've', abbreviation = 've',
cb = 'did_set_virtualedit', cb = 'did_set_virtualedit',

View File

@ -1545,7 +1545,7 @@ int build_stl_str_hl(win_T *wp, char *out, size_t outlen, char *fmt, OptIndex op
break; break;
case STL_SHOWCMD: case STL_SHOWCMD:
if (p_sc && (opt_idx == kOptInvalid || findoption(p_sloc) == opt_idx)) { if (p_sc && (opt_idx == kOptInvalid || find_option(p_sloc) == opt_idx)) {
str = showcmd_buf; str = showcmd_buf;
} }
break; break;