From 8f08b1efbd096850c04c2e8e2890d993bd4d9f95 Mon Sep 17 00:00:00 2001 From: Famiu Haque Date: Sun, 17 Dec 2023 05:23:33 +0600 Subject: [PATCH] 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'. --- src/nvim/CMakeLists.txt | 13 ++- src/nvim/api/deprecated.c | 4 +- src/nvim/api/options.c | 2 +- src/nvim/eval/vars.c | 2 +- src/nvim/generators/gen_options.lua | 19 +--- src/nvim/generators/gen_options_enum.lua | 76 ++++++++++++++ src/nvim/option.c | 124 +++++------------------ src/nvim/option.h | 2 +- src/nvim/options.lua | 20 +--- src/nvim/statusline.c | 2 +- 10 files changed, 125 insertions(+), 139 deletions(-) create mode 100644 src/nvim/generators/gen_options_enum.lua diff --git a/src/nvim/CMakeLists.txt b/src/nvim/CMakeLists.txt index 3773168948..a21a87ad5e 100644 --- a/src/nvim/CMakeLists.txt +++ b/src/nvim/CMakeLists.txt @@ -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_OPTIONS ${GENERATED_DIR}/options.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(FUNCS_GENERATOR ${GENERATOR_DIR}/gen_eval.lua) set(EVENTS_GENERATOR ${GENERATOR_DIR}/gen_events.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_DIR ${PROJECT_SOURCE_DIR}/src/unicode) 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_EVENTS_ENUM}" "${GENERATED_KEYSETS_DEFS}" + "${GENERATED_OPTIONS_ENUM}" ) list(APPEND NVIM_GENERATED_FOR_SOURCES @@ -622,6 +625,7 @@ list(APPEND NVIM_GENERATED_FOR_SOURCES "${GENERATED_EX_CMDS_DEFS}" "${GENERATED_EVENTS_NAMES_MAP}" "${GENERATED_OPTIONS}" + "${GENERATED_OPTIONS_MAP}" "${GENERATED_UNICODE_TABLES}" "${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 ) -add_custom_command(OUTPUT ${GENERATED_OPTIONS} ${GENERATED_OPTIONS_ENUM} - COMMAND ${LUA_GEN} ${OPTIONS_GENERATOR} ${GENERATED_OPTIONS} ${GENERATED_OPTIONS_ENUM} +add_custom_command(OUTPUT ${GENERATED_OPTIONS} + COMMAND ${LUA_GEN} ${OPTIONS_GENERATOR} ${GENERATED_OPTIONS} 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. foreach(hfile ${NVIM_GENERATED_FOR_HEADERS}) list(FIND NVIM_GENERATED_FOR_SOURCES ${hfile} hfile_idx) diff --git a/src/nvim/api/deprecated.c b/src/nvim/api/deprecated.c index 371361c5a1..27af581ef4 100644 --- a/src/nvim/api/deprecated.c +++ b/src/nvim/api/deprecated.c @@ -643,7 +643,7 @@ static Object get_option_from(void *from, OptReqScope req_scope, String name, Er 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)) { return (Object)OBJECT_INIT; } @@ -669,7 +669,7 @@ static void set_option_to(uint64_t channel_id, void *to, OptReqScope req_scope, return; }); - OptIndex opt_idx = findoption(name.data); + OptIndex opt_idx = find_option(name.data); VALIDATE_S(opt_idx != kOptInvalid, "option name", name.data, { return; }); diff --git a/src/nvim/api/options.c b/src/nvim/api/options.c index 54a2fbf385..a3ea7d9d47 100644 --- a/src/nvim/api/options.c +++ b/src/nvim/api/options.c @@ -79,7 +79,7 @@ static int validate_option_value_args(Dict(option) *opts, char *name, OptIndex * return FAIL; }); - *opt_idxp = findoption(name); + *opt_idxp = find_option(name); int flags = get_option_attrs(*opt_idxp); if (flags == 0) { // hidden or unknown option diff --git a/src/nvim/eval/vars.c b/src/nvim/eval/vars.c index 6fd707a0b0..9897ca77f1 100644 --- a/src/nvim/eval/vars.c +++ b/src/nvim/eval/vars.c @@ -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. 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) { semsg(_(e_unknown_option2), varname); return; diff --git a/src/nvim/generators/gen_options.lua b/src/nvim/generators/gen_options.lua index 61d5df3c84..2fd11e4c58 100644 --- a/src/nvim/generators/gen_options.lua +++ b/src/nvim/generators/gen_options.lua @@ -1,5 +1,4 @@ local options_file = arg[1] -local options_enum_file = arg[2] local opt_fd = assert(io.open(options_file, 'w')) @@ -171,7 +170,7 @@ local function dump_option(i, o) end if o.varname then 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 w((' .var=&options[%u].def_val'):format(i - 1)) elseif #o.scope == 1 and o.scope[1] == 'window' then @@ -248,19 +247,3 @@ w('') for _, v in ipairs(defines) do w('#define ' .. v[1] .. ' ' .. v[2]) 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() diff --git a/src/nvim/generators/gen_options_enum.lua b/src/nvim/generators/gen_options_enum.lua new file mode 100644 index 0000000000..c3fe9baae6 --- /dev/null +++ b/src/nvim/generators/gen_options_enum.lua @@ -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() diff --git a/src/nvim/option.c b/src/nvim/option.c index 1f4f547759..0d0437d45f 100644 --- a/src/nvim/option.c +++ b/src/nvim/option.c @@ -1,8 +1,8 @@ // User-settable options. Checklist for adding a new option: // - 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: -// - 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. // - For a window option, add some code to copy_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(). // - 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. -// - 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. #define IN_OPTION_C @@ -143,15 +143,13 @@ typedef enum { # include "option.c.generated.h" #endif -// options[] is initialized here. -// 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. +// options[] is initialized in options.generated.h. // The options with a NULL variable are 'hidden': a set command for them is // ignored and they are not printed. #ifdef INCLUDE_GENERATED_DECLARATIONS # include "options.generated.h" +# include "options_map.generated.h" #endif static int p_bin_dep_opts[] = { @@ -664,7 +662,7 @@ void set_init_3(void) } 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. 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++; } - *opt_idxp = findoption_len(arg, (size_t)(p - arg)); + *opt_idxp = find_option_len(arg, (size_t)(p - arg)); return p; } @@ -3009,87 +3007,6 @@ void check_redraw(uint32_t 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) 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; } -/// 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. -OptIndex findoption(const char *const arg) +OptIndex find_option_len(const char *const name, size_t len) 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. @@ -5229,7 +5159,7 @@ void set_context_in_set_cmd(expand_T *xp, char *arg, int opt_flags) return; } 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) { xp->xp_context = EXPAND_NOTHING; return; @@ -5534,7 +5464,7 @@ int ExpandOldSetting(int *numMatches, char ***matches) // For a terminal key code expand_option_idx is 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) { @@ -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) { - 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, { return (Dictionary)ARRAY_DICT_INIT; }); diff --git a/src/nvim/option.h b/src/nvim/option.h index 034c3de148..44cba70302 100644 --- a/src/nvim/option.h +++ b/src/nvim/option.h @@ -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. 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 - // the type_flags bit_field. + // the type_flags bit field. return get_option(opt_idx)->type_flags & (1 << type); } diff --git a/src/nvim/options.lua b/src/nvim/options.lua index 6d61a6d64e..0d5c24a793 100644 --- a/src/nvim/options.lua +++ b/src/nvim/options.lua @@ -2,6 +2,7 @@ --- @field full_name string --- @field desc? string --- @field abbreviation? string +--- @field alias? string|string[] --- @field short_desc? string|fun(): string --- @field varname? string --- @field pv_name? string @@ -84,6 +85,7 @@ end return { cstr = cstr, --- @type vim.option_meta[] + --- The order of the options MUST be alphabetic for ":set all". options = { { abbreviation = 'al', @@ -6809,6 +6811,7 @@ return { }, { abbreviation = 'sd', + alias = { 'vi', 'viminfo' }, cb = 'did_set_shada', defaults = { if_true = "!,'100,<50,s10,h", @@ -6940,6 +6943,7 @@ return { }, { abbreviation = 'sdf', + alias = { 'vif', 'viminfofile' }, defaults = { if_true = '' }, deny_duplicates = true, desc = [=[ @@ -9374,22 +9378,6 @@ return { type = 'string', 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', cb = 'did_set_virtualedit', diff --git a/src/nvim/statusline.c b/src/nvim/statusline.c index c7d9a65b86..f4ffa2a6b8 100644 --- a/src/nvim/statusline.c +++ b/src/nvim/statusline.c @@ -1545,7 +1545,7 @@ int build_stl_str_hl(win_T *wp, char *out, size_t outlen, char *fmt, OptIndex op break; 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; } break;