refactor(options)!: use OptVal for option defaults #26691

Problem: We use `void *` for option default values, which is confusing and can cause problems with type-correctness. It also doesn't accomodate for multitype options. On top of that, it also leads to default boolean option values not behaving correctly on big endian systems.

Solution: Use `OptVal` for option default values.

BREAKING CHANGE:
- `:set {option}<` removes the local value for all global-local options instead of just string global-local options.
- `:setlocal {option}<` copies the global value to the local value for number and boolean global-local options instead of removing the local value.

runtime/doc/news.txt

@@ -104,6 +104,10 @@ OPTIONS
   changes according to related options. It takes care of alignment, 'number',
   'relativenumber' and 'signcolumn' set to "number". The now redundant `%r` item
   is no longer treated specially for 'statuscolumn'.
+• `:set {option}<` removes the local value for all |global-local| options instead
+  of just string |global-local| options.
+• `:setlocal {option}<` copies the global value to the local value for number
+  and boolean |global-local| options instead of removing the local value.
 
 PLUGINS
 

runtime/doc/options.txt

@@ -306,19 +306,13 @@ created, thus they behave slightly differently:
 
 :se[t] {option}<	Set the effective value of {option} to its global
 			value.
-			For string |global-local| options, the local value is
+			For |global-local| options, the local value is removed,
-			removed, so that the global value will be used.
+			so that the global value will be used.
 			For all other options, the global value is copied to
 			the local value.
 
 :setl[ocal] {option}<	Set the effective value of {option} to its global
-			value.
+			value by copying the global value to the local value.
-			For number and boolean |global-local| options, the
-			local value is removed, so that the global value will
-			be used.
-			For all other options, including string |global-local|
-			options, the global value is copied to the local
-			value.
 
 Note that the behaviour for |global-local| options is slightly different
 between string and number-based options.

runtime/doc/vim_diff.txt

@@ -339,10 +339,8 @@ Normal commands:
 
 Options:
 
-Local values for global-local number/boolean options are unset when the option
+- `:set {option}<` removes local value for all |global-local| options.
-is set without a scope (e.g. by using |:set|), similarly to how global-local
+- `:setlocal {option}<` copies global value to local value for all options.
-string options work.
-
 - 'autoread'    works in the terminal (if it supports "focus" events)
 - 'cpoptions'   flags: |cpo-_|
 - 'diffopt'     "linematch" feature

src/nvim/generators/gen_options.lua

@@ -90,6 +90,12 @@ local function get_flags(o)
   return flags
 end
 
+--- @param opt_type vim.option_type
+--- @return string
+local function opt_type_enum(opt_type)
+  return ('kOptValType%s'):format(lowercase_to_titlecase(opt_type))
+end
+
 --- @param o vim.option_meta
 --- @return string
 local function get_type_flags(o)
@@ -99,7 +105,7 @@ local function get_type_flags(o)
 
   for _, opt_type in ipairs(opt_types) do
     assert(type(opt_type) == 'string')
-    type_flags = ('%s | (1 << kOptValType%s)'):format(type_flags, lowercase_to_titlecase(opt_type))
+    type_flags = ('%s | (1 << %s)'):format(type_flags, opt_type_enum(opt_type))
   end
 
   return type_flags
@@ -125,27 +131,48 @@ local function get_cond(c, base_string)
   return cond_string
 end
 
+--- @param s string
+--- @return string
+local static_cstr_as_string = function(s)
+  return ('{ .data = %s, .size = sizeof(%s) - 1 }'):format(s, s)
+end
+
+--- @param v vim.option_value|function
+--- @return string
+local get_opt_val = function(v)
+  --- @type vim.option_type
+  local v_type
+
+  if type(v) == 'function' then
+    v, v_type = v() --[[ @as string, vim.option_type ]]
+
+    if v_type == 'string' then
+      v = static_cstr_as_string(v)
+    end
+  else
+    v_type = type(v) --[[ @as vim.option_type ]]
+
+    if v_type == 'boolean' then
+      v = v and 'true' or 'false'
+    elseif v_type == 'number' then
+      v = ('%iL'):format(v)
+    elseif v_type == 'string' then
+      v = static_cstr_as_string(cstr(v))
+    end
+  end
+
+  return ('{ .type = %s, .data.%s = %s }'):format(opt_type_enum(v_type), v_type, v)
+end
+
+--- @param d vim.option_value|function
+--- @param n string
+--- @return string
+
 local get_defaults = function(d, n)
   if d == nil then
     error("option '" .. n .. "' should have a default value")
   end
-
+  return get_opt_val(d)
-  local value_dumpers = {
-    ['function'] = function(v)
-      return v()
-    end,
-    string = function(v)
-      return '.string=' .. cstr(v)
-    end,
-    boolean = function(v)
-      return '.boolean=' .. (v and 'true' or 'false')
-    end,
-    number = function(v)
-      return ('.number=%iL'):format(v)
-    end,
-  }
-
-  return value_dumpers[type(d)](d)
 end
 
 --- @type [string,string][]
@@ -173,7 +200,7 @@ local function dump_option(i, o)
     w('    .var=&' .. o.varname)
   elseif o.hidden or o.immutable then
     -- Hidden and immutable options can directly point to the default value.
-    w(('    .var=&options[%u].def_val'):format(i - 1))
+    w(('    .var=&options[%u].def_val.data'):format(i - 1))
   elseif #o.scope == 1 and o.scope[1] == 'window' then
     w('    .var=VAR_WIN')
   else
@@ -219,14 +246,16 @@ local function dump_option(i, o)
     if o.defaults.condition then
       w(get_cond(o.defaults.condition))
     end
-    w('    .def_val' .. get_defaults(o.defaults.if_true, o.full_name))
+    w('    .def_val=' .. get_defaults(o.defaults.if_true, o.full_name))
     if o.defaults.condition then
       if o.defaults.if_false then
         w('#else')
-        w('    .def_val' .. get_defaults(o.defaults.if_false, o.full_name))
+        w('    .def_val=' .. get_defaults(o.defaults.if_false, o.full_name))
       end
       w('#endif')
     end
+  else
+    w('    .def_val=NIL_OPTVAL')
   end
   w('  },')
 end

src/nvim/option.c

@@ -176,7 +176,7 @@ static int p_paste_dep_opts[] = {
 void set_init_tablocal(void)
 {
   // susy baka: cmdheight calls itself OPT_GLOBAL but is really tablocal!
-  p_ch = options[kOptCmdheight].def_val.number;
+  p_ch = options[kOptCmdheight].def_val.data.number;
 }
 
 /// Initialize the 'shell' option to a default value.
@@ -291,8 +291,9 @@ static void set_init_default_cdpath(void)
     }
   }
   buf[j] = NUL;
-  options[kOptCdpath].def_val.string = buf;
+  options[kOptCdpath].def_val = CSTR_AS_OPTVAL(buf);
   options[kOptCdpath].flags |= P_DEF_ALLOCED;
+
   xfree(cdpath);
 }
 
@@ -317,12 +318,13 @@ static void set_init_expand_env(void)
       p = option_expand(opt_idx, NULL);
     }
     if (p != NULL) {
-      p = xstrdup(p);
+      set_option_varp(opt_idx, opt->var, CSTR_TO_OPTVAL(p), opt->flags & P_ALLOCED);
-      *(char **)opt->var = p;
+      opt->flags |= P_ALLOCED;
+
       if (opt->flags & P_DEF_ALLOCED) {
-        xfree(opt->def_val.string);
+        optval_free(opt->def_val);
       }
-      opt->def_val.string = p;
+      opt->def_val = CSTR_TO_OPTVAL(p);
       opt->flags |= P_DEF_ALLOCED;
     }
   }
@@ -430,71 +432,54 @@ void set_init_1(bool clean_arg)
   set_helplang_default(get_mess_lang());
 }
 
+/// Get default value for option, based on the option's type and scope.
+///
+/// @param  opt_idx    Option index in options[] table.
+/// @param  opt_flags  Option flags.
+///
+/// @return Default value of option for the scope specified in opt_flags.
+static OptVal get_option_default(const OptIndex opt_idx, int opt_flags)
+{
+  vimoption_T *opt = &options[opt_idx];
+  bool is_global_local_option = opt->indir & PV_BOTH;
+
+#ifdef UNIX
+  if (opt_idx == kOptModeline && getuid() == ROOT_UID) {
+    // 'modeline' defaults to off for root.
+    return BOOLEAN_OPTVAL(false);
+  }
+#endif
+
+  if ((opt_flags & OPT_LOCAL) && is_global_local_option) {
+    // Use unset local value instead of default value for local scope of global-local options.
+    return get_option_unset_value(opt_idx);
+  } else if (option_has_type(opt_idx, kOptValTypeString) && !(opt->flags & P_NO_DEF_EXP)) {
+    // For string options, expand environment variables and ~ since the default value was already
+    // expanded, only required when an environment variable was set later.
+    char *s = option_expand(opt_idx, opt->def_val.data.string.data);
+    return s == NULL ? opt->def_val : CSTR_AS_OPTVAL(s);
+  } else {
+    return opt->def_val;
+  }
+}
+
 /// Set an option to its default value.
 /// This does not take care of side effects!
 ///
-/// @param opt_flags OPT_FREE, OPT_LOCAL and/or OPT_GLOBAL.
+/// @param  opt_idx    Option index in options[] table.
-///
+/// @param  opt_flags  Option flags.
-/// TODO(famiu): Refactor this when def_val uses OptVal.
 static void set_option_default(const OptIndex opt_idx, int opt_flags)
 {
-  bool both = (opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0;
+  OptVal def_val = get_option_default(opt_idx, opt_flags);
+  set_option_direct(opt_idx, def_val, opt_flags, current_sctx.sc_sid);
 
-  // pointer to variable for current option
+  if (opt_idx == kOptScroll) {
-  vimoption_T *opt = &options[opt_idx];
+    win_comp_scroll(curwin);
-  void *varp = get_varp_scope(opt, both ? OPT_LOCAL : opt_flags);
-  uint32_t flags = opt->flags;
-  if (varp != NULL) {       // skip hidden option, nothing to do for it
-    if (option_has_type(opt_idx, kOptValTypeString)) {
-      // Use set_option_direct() for local options to handle freeing and allocating the value.
-      if (opt->indir != PV_NONE) {
-        set_option_direct(opt_idx, CSTR_AS_OPTVAL(opt->def_val.string), opt_flags, 0);
-      } else {
-        if (flags & P_ALLOCED) {
-          free_string_option(*(char **)(varp));
-        }
-        *(char **)varp = opt->def_val.string;
-        opt->flags &= ~P_ALLOCED;
-      }
-    } else if (option_has_type(opt_idx, kOptValTypeNumber)) {
-      if (opt->indir == PV_SCROLL) {
-        win_comp_scroll(curwin);
-      } else {
-        OptInt def_val = opt->def_val.number;
-        if ((OptInt *)varp == &curwin->w_p_so
-            || (OptInt *)varp == &curwin->w_p_siso) {
-          // 'scrolloff' and 'sidescrolloff' local values have a
-          // different default value than the global default.
-          *(OptInt *)varp = -1;
-        } else {
-          *(OptInt *)varp = def_val;
-        }
-        // May also set global value for local option.
-        if (both) {
-          *(OptInt *)get_varp_scope(opt, OPT_GLOBAL) = def_val;
-        }
-      }
-    } else {  // boolean
-      *(int *)varp = opt->def_val.boolean;
-#ifdef UNIX
-      // 'modeline' defaults to off for root
-      if (opt->indir == PV_ML && getuid() == ROOT_UID) {
-        *(int *)varp = false;
-      }
-#endif
-      // May also set global value for local option.
-      if (both) {
-        *(int *)get_varp_scope(opt, OPT_GLOBAL) =
-          *(int *)varp;
-      }
-    }
-
-    // The default value is not insecure.
-    uint32_t *flagsp = insecure_flag(curwin, opt_idx, opt_flags);
-    *flagsp = *flagsp & ~P_INSECURE;
   }
 
-  set_option_sctx(opt_idx, opt_flags, current_sctx);
+  // The default value is not insecure.
+  uint32_t *flagsp = insecure_flag(curwin, opt_idx, opt_flags);
+  *flagsp = *flagsp & ~P_INSECURE;
 }
 
 /// Set all options (except terminal options) to their default value.
@@ -522,6 +507,8 @@ static void set_options_default(int opt_flags)
 /// @param  opt_idx    Option index in options[] table.
 /// @param  val        The value of the option.
 /// @param  allocated  If true, do not copy default as it was already allocated.
+///
+/// TODO(famiu): Remove this.
 static void set_string_default(OptIndex opt_idx, char *val, bool allocated)
   FUNC_ATTR_NONNULL_ALL
 {
@@ -531,10 +518,10 @@ static void set_string_default(OptIndex opt_idx, char *val, bool allocated)
 
   vimoption_T *opt = &options[opt_idx];
   if (opt->flags & P_DEF_ALLOCED) {
-    xfree(opt->def_val.string);
+    optval_free(opt->def_val);
   }
 
-  opt->def_val.string = allocated ? val : xstrdup(val);
+  opt->def_val = CSTR_AS_OPTVAL(allocated ? val : xstrdup(val));
   opt->flags |= P_DEF_ALLOCED;
 }
 
@@ -569,15 +556,6 @@ static char *find_dup_item(char *origval, const char *newval, const size_t newva
   return NULL;
 }
 
-/// Set the Vi-default value of a number option.
-/// Used for 'lines' and 'columns'.
-void set_number_default(OptIndex opt_idx, OptInt val)
-{
-  if (opt_idx != kOptInvalid) {
-    options[opt_idx].def_val.number = val;
-  }
-}
-
 #if defined(EXITFREE)
 /// Free all options.
 void free_all_options(void)
@@ -589,7 +567,7 @@ void free_all_options(void)
         optval_free(optval_from_varp(opt_idx, options[opt_idx].var));
       }
       if (options[opt_idx].flags & P_DEF_ALLOCED) {
-        optval_free(optval_from_varp(opt_idx, &options[opt_idx].def_val));
+        optval_free(options[opt_idx].def_val);
       }
     } else if (options[opt_idx].var != VAR_WIN) {
       // buffer-local option: free global value
@@ -622,7 +600,7 @@ void set_init_2(bool headless)
   if (!option_was_set(kOptWindow)) {
     p_window = Rows - 1;
   }
-  set_number_default(kOptWindow, Rows - 1);
+  options[kOptWindow].def_val = NUMBER_OPTVAL(Rows - 1);
 }
 
 /// Initialize the options, part three: After reading the .vimrc
@@ -640,43 +618,29 @@ void set_init_3(void)
   char *p = (char *)invocation_path_tail(p_sh, &len);
   p = xmemdupz(p, len);
 
-  {
+  bool is_csh = path_fnamecmp(p, "csh") == 0 || path_fnamecmp(p, "tcsh") == 0;
-    //
+  bool is_known_shell = path_fnamecmp(p, "sh") == 0 || path_fnamecmp(p, "ksh") == 0
-    // Default for p_sp is "| tee", for p_srr is ">".
+                        || path_fnamecmp(p, "mksh") == 0 || path_fnamecmp(p, "pdksh") == 0
-    // For known shells it is changed here to include stderr.
+                        || path_fnamecmp(p, "zsh") == 0 || path_fnamecmp(p, "zsh-beta") == 0
-    //
+                        || path_fnamecmp(p, "bash") == 0 || path_fnamecmp(p, "fish") == 0
-    if (path_fnamecmp(p, "csh") == 0
+                        || path_fnamecmp(p, "ash") == 0 || path_fnamecmp(p, "dash") == 0;
-        || path_fnamecmp(p, "tcsh") == 0) {
+
-      if (do_sp) {
+  // Default for p_sp is "| tee", for p_srr is ">".
-        p_sp = "|& tee";
+  // For known shells it is changed here to include stderr.
-        options[kOptShellpipe].def_val.string = p_sp;
+  if (is_csh || is_known_shell) {
-      }
+    if (do_sp) {
-      if (do_srr) {
+      const OptVal sp =
-        p_srr = ">&";
+        is_csh ? STATIC_CSTR_AS_OPTVAL("|& tee") : STATIC_CSTR_AS_OPTVAL("2>&1| tee");
-        options[kOptShellredir].def_val.string = p_srr;
+      set_option_direct(kOptShellpipe, sp, 0, SID_NONE);
-      }
+      options[kOptShellpipe].def_val = sp;
-    } else if (path_fnamecmp(p, "sh") == 0
+    }
-               || path_fnamecmp(p, "ksh") == 0
+    if (do_srr) {
-               || path_fnamecmp(p, "mksh") == 0
+      const OptVal srr = is_csh ? STATIC_CSTR_AS_OPTVAL(">&") : STATIC_CSTR_AS_OPTVAL(">%s 2>&1");
-               || path_fnamecmp(p, "pdksh") == 0
+      set_option_direct(kOptShellredir, srr, 0, SID_NONE);
-               || path_fnamecmp(p, "zsh") == 0
+      options[kOptShellredir].def_val = srr;
-               || path_fnamecmp(p, "zsh-beta") == 0
-               || path_fnamecmp(p, "bash") == 0
-               || path_fnamecmp(p, "fish") == 0
-               || path_fnamecmp(p, "ash") == 0
-               || path_fnamecmp(p, "dash") == 0) {
-      // Always use POSIX shell style redirection if we reach this
-      if (do_sp) {
-        p_sp = "2>&1| tee";
-        options[kOptShellpipe].def_val.string = p_sp;
-      }
-      if (do_srr) {
-        p_srr = ">%s 2>&1";
-        options[kOptShellredir].def_val.string = p_srr;
-      }
     }
-    xfree(p);
   }
+  xfree(p);
 
   if (buf_is_empty(curbuf)) {
     int idx_ffs = find_option("ffs");
@@ -734,12 +698,12 @@ void set_title_defaults(void)
   // icon name.  Saves a bit of time, because the X11 display server does
   // not need to be contacted.
   if (!(options[kOptTitle].flags & P_WAS_SET)) {
-    options[kOptTitle].def_val.boolean = false;
+    options[kOptTitle].def_val = BOOLEAN_OPTVAL(false);
-    p_title = false;
+    p_title = 0;
   }
   if (!(options[kOptIcon].flags & P_WAS_SET)) {
-    options[kOptIcon].def_val.boolean = false;
+    options[kOptIcon].def_val = BOOLEAN_OPTVAL(false);
-    p_icon = false;
+    p_icon = 0;
   }
 }
 
@@ -758,27 +722,6 @@ void ex_set(exarg_T *eap)
   do_set(eap->arg, flags);
 }
 
-/// Get the default value for a string option.
-static char *stropt_get_default_val(OptIndex opt_idx, uint64_t flags)
-{
-  char *newval = options[opt_idx].def_val.string;
-  // expand environment variables and ~ since the default value was
-  // already expanded, only required when an environment variable was set
-  // later
-  if (newval == NULL) {
-    newval = empty_string_option;
-  } else if (!(options[opt_idx].flags & P_NO_DEF_EXP)) {
-    char *s = option_expand(opt_idx, newval);
-    if (s == NULL) {
-      s = newval;
-    }
-    newval = xstrdup(s);
-  } else {
-    newval = xstrdup(newval);
-  }
-  return newval;
-}
-
 /// Copy the new string value into allocated memory for the option.
 /// Can't use set_option_direct(), because we need to remove the backslashes.
 static char *stropt_copy_value(char *origval, char **argp, set_op_T op,
@@ -922,10 +865,7 @@ static void stropt_remove_dupflags(char *newval, uint32_t flags)
   }
 }
 
-/// Get the string value specified for a ":set" command.  The following set
+/// Get the string value specified for a ":set" command.  The following set options are supported:
-/// options are supported:
-///     set {opt}&
-///     set {opt}<
 ///     set {opt}={val}
 ///     set {opt}:{val}
 static char *stropt_get_newval(int nextchar, OptIndex opt_idx, char **argp, void *varp,
@@ -936,61 +876,56 @@ static char *stropt_get_newval(int nextchar, OptIndex opt_idx, char **argp, void
   char *save_arg = NULL;
   char *newval;
   char *s = NULL;
-  if (nextchar == '&') {  // set to default val
-    newval = stropt_get_default_val(opt_idx, flags);
-  } else if (nextchar == '<') {  // set to global val
-    newval = xstrdup(*(char **)get_varp_scope(&(options[opt_idx]), OPT_GLOBAL));
-  } else {
-    arg++;  // jump to after the '=' or ':'
 
-    // Set 'keywordprg' to ":help" if an empty
+  arg++;  // jump to after the '=' or ':'
-    // value was passed to :set by the user.
+
-    if (varp == &p_kp && (*arg == NUL || *arg == ' ')) {
+  // Set 'keywordprg' to ":help" if an empty
-      save_arg = arg;
+  // value was passed to :set by the user.
-      arg = ":help";
+  if (varp == &p_kp && (*arg == NUL || *arg == ' ')) {
+    save_arg = arg;
+    arg = ":help";
+  }
+
+  // Copy the new string into allocated memory.
+  newval = stropt_copy_value(origval, &arg, op, flags);
+
+  // Expand environment variables and ~.
+  // Don't do it when adding without inserting a comma.
+  if (op == OP_NONE || (flags & P_COMMA)) {
+    newval = stropt_expand_envvar(opt_idx, origval, newval, op);
+  }
+
+  // locate newval[] in origval[] when removing it
+  // and when adding to avoid duplicates
+  int len = 0;
+  if (op == OP_REMOVING || (flags & P_NODUP)) {
+    len = (int)strlen(newval);
+    s = find_dup_item(origval, newval, (size_t)len, flags);
+
+    // do not add if already there
+    if ((op == OP_ADDING || op == OP_PREPENDING) && s != NULL) {
+      op = OP_NONE;
+      STRCPY(newval, origval);
     }
 
-    // Copy the new string into allocated memory.
+    // if no duplicate, move pointer to end of original value
-    newval = stropt_copy_value(origval, &arg, op, flags);
+    if (s == NULL) {
-
+      s = origval + (int)strlen(origval);
-    // Expand environment variables and ~.
-    // Don't do it when adding without inserting a comma.
-    if (op == OP_NONE || (flags & P_COMMA)) {
-      newval = stropt_expand_envvar(opt_idx, origval, newval, op);
     }
+  }
 
-    // locate newval[] in origval[] when removing it
+  // concatenate the two strings; add a ',' if needed
-    // and when adding to avoid duplicates
+  if (op == OP_ADDING || op == OP_PREPENDING) {
-    int len = 0;
+    stropt_concat_with_comma(origval, newval, op, flags);
-    if (op == OP_REMOVING || (flags & P_NODUP)) {
+  } else if (op == OP_REMOVING) {
-      len = (int)strlen(newval);
+    // Remove newval[] from origval[]. (Note: "len" has been set above
-      s = find_dup_item(origval, newval, (size_t)len, flags);
+    // and is used here).
+    stropt_remove_val(origval, newval, flags, s, len);
+  }
 
-      // do not add if already there
+  if (flags & P_FLAGLIST) {
-      if ((op == OP_ADDING || op == OP_PREPENDING) && s != NULL) {
+    // Remove flags that appear twice.
-        op = OP_NONE;
+    stropt_remove_dupflags(newval, flags);
-        STRCPY(newval, origval);
-      }
-
-      // if no duplicate, move pointer to end of original value
-      if (s == NULL) {
-        s = origval + (int)strlen(origval);
-      }
-    }
-
-    // concatenate the two strings; add a ',' if needed
-    if (op == OP_ADDING || op == OP_PREPENDING) {
-      stropt_concat_with_comma(origval, newval, op, flags);
-    } else if (op == OP_REMOVING) {
-      // Remove newval[] from origval[]. (Note: "len" has been set above
-      // and is used here).
-      stropt_remove_val(origval, newval, flags, s, len);
-    }
-
-    if (flags & P_FLAGLIST) {
-      // Remove flags that appear twice.
-      stropt_remove_dupflags(newval, flags);
-    }
   }
 
   if (save_arg != NULL) {
@@ -1152,6 +1087,7 @@ const char *find_option_end(const char *arg, OptIndex *opt_idxp)
 }
 
 /// Get new option value from argp. Allocated OptVal must be freed by caller.
+/// Can unset local value of an option when ":set {option}<" is used.
 static OptVal get_option_newval(OptIndex opt_idx, int opt_flags, set_prefix_T prefix, char **argp,
                                 int nextchar, set_op_T op, uint32_t flags, void *varp, char *errbuf,
                                 const size_t errbuflen, const char **errmsg)
@@ -1166,6 +1102,20 @@ static OptVal get_option_newval(OptIndex opt_idx, int opt_flags, set_prefix_T pr
   OptVal oldval = optval_from_varp(opt_idx, oldval_is_global ? get_varp(opt) : varp);
   OptVal newval = NIL_OPTVAL;
 
+  if (nextchar == '&') {
+    // ":set opt&": Reset to default value.
+    // NOTE: Use OPT_GLOBAL instead of opt_flags to ensure we don't use the unset local value for
+    // global-local options when OPT_LOCAL is used.
+    return optval_copy(get_option_default(opt_idx, OPT_GLOBAL));
+  } else if (nextchar == '<') {
+    // ":set opt<": Reset to global value.
+    // ":setlocal opt<": Copy global value to local value.
+    if (option_is_global_local(opt_idx) && !(opt_flags & OPT_LOCAL)) {
+      unset_option_local_value(opt_idx);
+    }
+    return get_option_value(opt_idx, OPT_GLOBAL);
+  }
+
   switch (oldval.type) {
   case kOptValTypeNil:
     abort();
@@ -1173,8 +1123,6 @@ static OptVal get_option_newval(OptIndex opt_idx, int opt_flags, set_prefix_T pr
     TriState newval_bool;
 
     // ":set opt!": invert
-    // ":set opt&": reset to default value
-    // ":set opt<": reset to global value
     if (nextchar == '!') {
       switch (oldval.data.boolean) {
       case kNone:
@@ -1187,15 +1135,6 @@ static OptVal get_option_newval(OptIndex opt_idx, int opt_flags, set_prefix_T pr
         newval_bool = kTrue;
         break;
       }
-    } else if (nextchar == '&') {
-      newval_bool = TRISTATE_FROM_INT(options[opt_idx].def_val.boolean);
-    } else if (nextchar == '<') {
-      // For 'autoread', kNone means to use global value.
-      if ((int *)varp == &curbuf->b_p_ar && opt_flags == OPT_LOCAL) {
-        newval_bool = kNone;
-      } else {
-        newval_bool = TRISTATE_FROM_INT(*(int *)get_varp_scope(&(options[opt_idx]), OPT_GLOBAL));
-      }
     } else {
       // ":set invopt": invert
       // ":set opt" or ":set noopt": set or reset
@@ -1214,31 +1153,15 @@ static OptVal get_option_newval(OptIndex opt_idx, int opt_flags, set_prefix_T pr
     OptInt newval_num;
 
     // Different ways to set a number option:
-    // &            set to default value
-    // <            set to global value
     // <xx>         accept special key codes for 'wildchar' or 'wildcharm'
     // ^x           accept ctrl key codes for 'wildchar' or 'wildcharm'
     // c            accept any non-digit for 'wildchar' or 'wildcharm'
     // [-]0-9       set number
     // other        error
     arg++;
-    if (nextchar == '&') {
+    if (((OptInt *)varp == &p_wc || (OptInt *)varp == &p_wcm)
-      newval_num = options[opt_idx].def_val.number;
+        && (*arg == '<' || *arg == '^'
-    } else if (nextchar == '<') {
+            || (*arg != NUL && (!arg[1] || ascii_iswhite(arg[1])) && !ascii_isdigit(*arg)))) {
-      if ((OptInt *)varp == &curbuf->b_p_ul && opt_flags == OPT_LOCAL) {
-        // for 'undolevels' NO_LOCAL_UNDOLEVEL means using the global newval_num
-        newval_num = NO_LOCAL_UNDOLEVEL;
-      } else if (opt_flags == OPT_LOCAL
-                 && ((OptInt *)varp == &curwin->w_p_siso || (OptInt *)varp == &curwin->w_p_so)) {
-        // for 'scrolloff'/'sidescrolloff' -1 means using the global newval_num
-        newval_num = -1;
-      } else {
-        newval_num = *(OptInt *)get_varp_scope(&(options[opt_idx]), OPT_GLOBAL);
-      }
-    } else if (((OptInt *)varp == &p_wc || (OptInt *)varp == &p_wcm)
-               && (*arg == '<' || *arg == '^'
-                   || (*arg != NUL && (!arg[1] || ascii_iswhite(arg[1]))
-                       && !ascii_isdigit(*arg)))) {
       newval_num = string_to_key(arg);
       if (newval_num == 0) {
         *errmsg = e_invarg;
@@ -3218,6 +3141,19 @@ bool optval_equal(OptVal o1, OptVal o2)
   UNREACHABLE;
 }
 
+/// Get type of option. Does not support multitype options.
+static OptValType option_get_type(const OptIndex opt_idx)
+{
+  assert(!option_is_multitype(opt_idx));
+
+  // If the option only supports a single type, it means that the index of the option's type flag
+  // corresponds to the value of the type enum. So get the index of the type flag using xctz() and
+  // use that as the option's type.
+  OptValType type = xctz(options[opt_idx].type_flags);
+  assert(type > kOptValTypeNil && type < kOptValTypeSize);
+  return type;
+}
+
 /// Create OptVal from var pointer.
 ///
 /// @param       opt_idx  Option index in options[] table.
@@ -3237,11 +3173,7 @@ OptVal optval_from_varp(OptIndex opt_idx, void *varp)
     return varp == NULL ? NIL_OPTVAL : *(OptVal *)varp;
   }
 
-  // If the option only supports a single type, it means that the index of the option's type flag
+  OptValType type = option_get_type(opt_idx);
-  // corresponds to the value of the type enum. So get the index of the type flag using xctz() and
-  // use that as the option's type.
-  OptValType type = xctz(options[opt_idx].type_flags);
-  assert(type > kOptValTypeNil && type < kOptValTypeSize);
 
   switch (type) {
   case kOptValTypeNil:
@@ -3390,6 +3322,11 @@ bool is_option_hidden(OptIndex opt_idx)
   return opt_idx == kOptInvalid ? false : get_varp(&options[opt_idx]) == NULL;
 }
 
+static inline bool option_is_global_local(OptIndex opt_idx)
+{
+  return opt_idx == kOptInvalid ? false : (options[opt_idx].indir & PV_BOTH);
+}
+
 /// Get option flags.
 ///
 /// @param  opt_idx  Option index in options[] table.
@@ -3588,10 +3525,9 @@ static const char *did_set_option(OptIndex opt_idx, void *varp, OptVal old_value
   opt->flags |= P_ALLOCED;
 
   const bool scope_both = (opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0;
-  const bool opt_is_global_local = opt->indir & PV_BOTH;
 
   if (scope_both) {
-    if (opt_is_global_local) {
+    if (option_is_global_local(opt_idx)) {
       // Global option with local value set to use global value.
       // Free the local value and clear it.
       void *varp_local = get_varp_scope(opt, OPT_LOCAL);
@@ -3678,7 +3614,6 @@ static const char *validate_option_value(const OptIndex opt_idx, void *varp, Opt
     if (opt_flags == OPT_GLOBAL) {
       errmsg = _("Cannot unset global option value");
     } else {
-      optval_free(*newval);
       *newval = optval_copy(get_option_unset_value(opt_idx));
     }
   } else if (!option_has_type(opt_idx, newval->type)) {
@@ -3719,25 +3654,28 @@ static const char *set_option(const OptIndex opt_idx, void *varp, OptVal value,
 {
   assert(opt_idx != kOptInvalid);
 
-  const char *errmsg = validate_option_value(opt_idx, varp, &value, opt_flags, errbuf, errbuflen);
+  const char *errmsg = NULL;
 
-  if (errmsg != NULL) {
+  if (!direct) {
-    optval_free(value);
+    errmsg = validate_option_value(opt_idx, varp, &value, opt_flags, errbuf, errbuflen);
-    return errmsg;
+
+    if (errmsg != NULL) {
+      optval_free(value);
+      return errmsg;
+    }
   }
 
   vimoption_T *opt = &options[opt_idx];
   const bool scope_local = opt_flags & OPT_LOCAL;
   const bool scope_global = opt_flags & OPT_GLOBAL;
   const bool scope_both = !scope_local && !scope_global;
-  const bool opt_is_global_local = opt->indir & PV_BOTH;
   // Whether local value of global-local option is unset.
-  // NOTE: When this is true, it also implies that opt_is_global_local is true.
+  // NOTE: When this is true, it also implies that the option is global-local.
   const bool is_opt_local_unset = is_option_local_value_unset(opt_idx);
 
   // When using ":set opt=val" for a global option with a local value the local value will be reset,
   // use the global value here.
-  if (scope_both && opt_is_global_local) {
+  if (scope_both && option_is_global_local(opt_idx)) {
     varp = opt->var;
   }
 
@@ -3823,8 +3761,10 @@ void set_option_direct(OptIndex opt_idx, OptVal value, int opt_flags, scid_T set
   const bool scope_both = (opt_flags & (OPT_LOCAL | OPT_GLOBAL)) == 0;
   void *varp = get_varp_scope(opt, scope_both ? OPT_LOCAL : opt_flags);
 
-  set_option(opt_idx, varp, optval_copy(value), opt_flags, set_sid, true, true, errbuf,
+  const char *errmsg = set_option(opt_idx, varp, optval_copy(value), opt_flags, set_sid, true, true,
-             sizeof(errbuf));
+                                  errbuf, sizeof(errbuf));
+  assert(errmsg == NULL);
+  (void)errmsg;  // ignore unused warning
 }
 
 /// Set option value directly for buffer / window, without processing any side effects.
@@ -3893,6 +3833,17 @@ const char *set_option_value(const OptIndex opt_idx, const OptVal value, int opt
                     sizeof(errbuf));
 }
 
+/// Unset the local value of a global-local option.
+///
+/// @param      opt_idx    Index in options[] table. Must not be kOptInvalid.
+///
+/// @return  NULL on success, an untranslated error message on error.
+static inline const char *unset_option_local_value(const OptIndex opt_idx)
+{
+  assert(option_is_global_local(opt_idx));
+  return set_option_value(opt_idx, get_option_unset_value(opt_idx), OPT_LOCAL);
+}
+
 /// Set the value of an option. Supports TTY options, unlike set_option_value().
 ///
 /// @param      name       Option name. Used for error messages and for setting TTY options.
@@ -4273,7 +4224,7 @@ static int optval_default(OptIndex opt_idx, void *varp)
   }
 
   OptVal current_val = optval_from_varp(opt_idx, varp);
-  OptVal default_val = optval_from_varp(opt_idx, &opt->def_val);
+  OptVal default_val = opt->def_val;
 
   return optval_equal(current_val, default_val);
 }
@@ -5447,7 +5398,7 @@ void reset_modifiable(void)
 {
   curbuf->b_p_ma = false;
   p_ma = false;
-  options[kOptModifiable].def_val.boolean = false;
+  options[kOptModifiable].def_val = BOOLEAN_OPTVAL(false);
 }
 
 /// Set the global value for 'iminsert' to the local value.
@@ -6537,11 +6488,8 @@ static Dict vimoption2dict(vimoption_T *opt, int req_scope, buf_T *buf, win_T *w
   PUT_C(dict, "last_set_linenr", INTEGER_OBJ(last_set.script_ctx.sc_lnum));
   PUT_C(dict, "last_set_chan", INTEGER_OBJ((int64_t)last_set.channel_id));
 
-  // TODO(bfredl): do you even nocp?
+  PUT_C(dict, "type", CSTR_AS_OBJ(optval_type_get_name(option_get_type(get_opt_idx(opt)))));
-  OptVal def = optval_from_varp(get_opt_idx(opt), &opt->def_val);
+  PUT_C(dict, "default", optval_as_object(opt->def_val));
-
-  PUT_C(dict, "type", CSTR_AS_OBJ(optval_type_get_name(def.type)));
-  PUT_C(dict, "default", optval_as_object(def));
   PUT_C(dict, "allows_duplicates", BOOLEAN_OBJ(!(opt->flags & P_NODUP)));
 
   return dict;

src/nvim/option.h

@@ -60,13 +60,7 @@ typedef struct {
   /// cmdline. Only useful for string options.
   opt_expand_cb_T opt_expand_cb;
 
-  // TODO(famiu): Use OptVal for def_val.
+  OptVal def_val;    ///< default value
-  union {
-    int boolean;
-    OptInt number;
-    char *string;
-  } def_val;         ///< default value for variable
-
   LastSet last_set;  ///< script in which the option was last set
 } vimoption_T;
 

src/nvim/options.lua

@@ -8,8 +8,7 @@
 --- @field short_desc? string|fun(): string
 --- @field varname? string
 --- @field pv_name? string
---- @field type 'boolean'|'number'|'string'
+--- @field type vim.option_type|vim.option_type[]
---- @field hidden? boolean
 --- @field immutable? boolean
 --- @field list? 'comma'|'onecomma'|'commacolon'|'onecommacolon'|'flags'|'flagscomma'
 --- @field scope vim.option_scope[]
@@ -43,6 +42,8 @@
 --- @field meta? integer|boolean|string Default to use in Lua meta files
 
 --- @alias vim.option_scope 'global'|'buffer'|'window'
+--- @alias vim.option_type 'boolean'|'number'|'string'
+--- @alias vim.option_value boolean|number|string
 
 --- @alias vim.option_redraw
 --- |'statuslines'
@@ -61,18 +62,11 @@ local function cstr(s)
 end
 
 --- @param s string
---- @return fun(): string
+--- @param t vim.option_type
-local function macros(s)
+--- @return fun(): string, vim.option_type
+local function macros(s, t)
   return function()
-    return '.string=' .. s
+    return s, t
-  end
-end
-
---- @param s string
---- @return fun(): string
-local function imacros(s)
-  return function()
-    return '.number=' .. s
   end
 end
 
@@ -994,7 +988,7 @@ return {
     {
       cb = 'did_set_cedit',
       defaults = {
-        if_true = macros('CTRL_F_STR'),
+        if_true = macros('CTRL_F_STR', 'string'),
         doc = 'CTRL-F',
       },
       desc = [=[
@@ -1288,7 +1282,7 @@ return {
       abbreviation = 'co',
       cb = 'did_set_lines_or_columns',
       defaults = {
-        if_true = imacros('DFLT_COLS'),
+        if_true = macros('DFLT_COLS', 'number'),
         doc = '80 or terminal width',
       },
       desc = [=[
@@ -1630,7 +1624,7 @@ return {
     {
       abbreviation = 'cpo',
       cb = 'did_set_cpoptions',
-      defaults = { if_true = macros('CPO_VIM') },
+      defaults = { if_true = macros('CPO_VIM', 'string') },
       desc = [=[
         A sequence of single character flags.  When a character is present
         this indicates Vi-compatible behavior.  This is used for things where
@@ -2368,7 +2362,7 @@ return {
     {
       abbreviation = 'enc',
       cb = 'did_set_encoding',
-      defaults = { if_true = macros('ENC_DFLT') },
+      defaults = { if_true = macros('ENC_DFLT', 'string') },
       deny_in_modelines = true,
       desc = [=[
         String-encoding used internally and for |RPC| communication.
@@ -2492,7 +2486,7 @@ return {
     },
     {
       abbreviation = 'ef',
-      defaults = { if_true = macros('DFLT_ERRORFILE') },
+      defaults = { if_true = macros('DFLT_ERRORFILE', 'string') },
       desc = [=[
         Name of the errorfile for the QuickFix mode (see |:cf|).
         When the "-q" command-line argument is used, 'errorfile' is set to the
@@ -2514,7 +2508,7 @@ return {
     {
       abbreviation = 'efm',
       defaults = {
-        if_true = macros('DFLT_EFM'),
+        if_true = macros('DFLT_EFM', 'string'),
         doc = 'is very long',
       },
       deny_duplicates = true,
@@ -2706,7 +2700,7 @@ return {
       alloced = true,
       cb = 'did_set_fileformat',
       defaults = {
-        if_true = macros('DFLT_FF'),
+        if_true = macros('DFLT_FF', 'string'),
         doc = 'Windows: "dos", Unix: "unix"',
       },
       desc = [=[
@@ -2739,7 +2733,7 @@ return {
       abbreviation = 'ffs',
       cb = 'did_set_fileformats',
       defaults = {
-        if_true = macros('DFLT_FFS_VIM'),
+        if_true = macros('DFLT_FFS_VIM', 'string'),
         doc = 'Windows: "dos,unix", Unix: "unix,dos"',
       },
       deny_duplicates = true,
@@ -3316,7 +3310,7 @@ return {
       abbreviation = 'fo',
       alloced = true,
       cb = 'did_set_formatoptions',
-      defaults = { if_true = macros('DFLT_FO_VIM') },
+      defaults = { if_true = macros('DFLT_FO_VIM', 'string') },
       desc = [=[
         This is a sequence of letters which describes how automatic
         formatting is to be done.
@@ -3409,7 +3403,7 @@ return {
     },
     {
       abbreviation = 'gfm',
-      defaults = { if_true = macros('DFLT_GREPFORMAT') },
+      defaults = { if_true = macros('DFLT_GREPFORMAT', 'string') },
       deny_duplicates = true,
       desc = [=[
         Format to recognize for the ":grep" command output.
@@ -3773,6 +3767,7 @@ return {
     },
     {
       abbreviation = 'gtl',
+      defaults = { if_true = '' },
       desc = [=[
         When non-empty describes the text to use in a label of the GUI tab
         pages line.  When empty and when the result is empty Vim will use a
@@ -3798,6 +3793,7 @@ return {
     },
     {
       abbreviation = 'gtt',
+      defaults = { if_true = '' },
       desc = [=[
         When non-empty describes the text to use in a tooltip for the GUI tab
         pages line.  When empty Vim will use a default tooltip.
@@ -3817,7 +3813,7 @@ return {
       abbreviation = 'hf',
       cb = 'did_set_helpfile',
       defaults = {
-        if_true = macros('DFLT_HELPFILE'),
+        if_true = macros('DFLT_HELPFILE', 'string'),
         doc = [[(MS-Windows) "$VIMRUNTIME\doc\help.txt"
                   (others) "$VIMRUNTIME/doc/help.txt"]],
       },
@@ -3914,7 +3910,7 @@ return {
     {
       abbreviation = 'hl',
       cb = 'did_set_highlight',
-      defaults = { if_true = macros('HIGHLIGHT_INIT') },
+      defaults = { if_true = macros('HIGHLIGHT_INIT', 'string') },
       deny_duplicates = true,
       full_name = 'highlight',
       list = 'onecomma',
@@ -4080,7 +4076,7 @@ return {
     {
       abbreviation = 'imi',
       cb = 'did_set_iminsert',
-      defaults = { if_true = imacros('B_IMODE_NONE') },
+      defaults = { if_true = macros('B_IMODE_NONE', 'number') },
       desc = [=[
         Specifies whether :lmap or an Input Method (IM) is to be used in
         Insert mode.  Valid values:
@@ -4106,7 +4102,7 @@ return {
     },
     {
       abbreviation = 'ims',
-      defaults = { if_true = imacros('B_IMODE_USE_INSERT') },
+      defaults = { if_true = macros('B_IMODE_USE_INSERT', 'number') },
       desc = [=[
         Specifies whether :lmap or an Input Method (IM) is to be used when
         entering a search pattern.  Valid values:
@@ -4812,7 +4808,7 @@ return {
     {
       cb = 'did_set_lines_or_columns',
       defaults = {
-        if_true = imacros('DFLT_ROWS'),
+        if_true = macros('DFLT_ROWS', 'number'),
         doc = '24 or terminal height',
       },
       desc = [=[
@@ -4900,7 +4896,7 @@ return {
     {
       abbreviation = 'lw',
       defaults = {
-        if_true = macros('LISPWORD_VALUE'),
+        if_true = macros('LISPWORD_VALUE', 'string'),
         doc = 'is very long',
       },
       deny_duplicates = true,
@@ -5210,7 +5206,7 @@ return {
     },
     {
       abbreviation = 'mco',
-      defaults = { if_true = imacros('MAX_MCO') },
+      defaults = { if_true = macros('MAX_MCO', 'number') },
       full_name = 'maxcombine',
       scope = { 'global' },
       short_desc = N_('maximum nr of combining characters displayed'),
@@ -9613,7 +9609,7 @@ return {
       abbreviation = 'wc',
       cb = 'did_set_wildchar',
       defaults = {
-        if_true = imacros('TAB'),
+        if_true = macros('TAB', 'number'),
         doc = '<Tab>',
       },
       desc = [=[

test/old/testdir/test_options.vim

@@ -1386,7 +1386,8 @@ func Test_local_scrolloff()
   call assert_equal(5, &so)
   wincmd w
   call assert_equal(3, &so)
-  setlocal so<
+  "setlocal so<
+  set so<
   call assert_equal(5, &so)
   setglob so=8
   call assert_equal(8, &so)
@@ -1403,7 +1404,8 @@ func Test_local_scrolloff()
   call assert_equal(7, &siso)
   wincmd w
   call assert_equal(3, &siso)
-  setlocal siso<
+  "setlocal siso<
+  set siso<
   call assert_equal(7, &siso)
   setglob siso=4
   call assert_equal(4, &siso)
@@ -1595,17 +1597,17 @@ func Test_set_number_global_local_option()
   call assert_equal(12, &l:scrolloff)
   call assert_equal(12, &scrolloff)
 
-  " :set {option}< set the effective value of {option} to its global value.
+  " :setlocal {option}< set the effective value of {option} to its global value.
-  set scrolloff<
+  "set scrolloff<
-  " Nvim: local value is removed
+  setlocal scrolloff<
-  " call assert_equal(10, &l:scrolloff)
+  call assert_equal(10, &l:scrolloff)
-  call assert_equal(-1, &l:scrolloff)
   call assert_equal(10, &scrolloff)
 
-  " :setlocal {option}< removes the local value, so that the global value will be used.
+  " :set {option}< removes the local value, so that the global value will be used.
   setglobal scrolloff=15
   setlocal scrolloff=18
-  setlocal scrolloff<
+  "setlocal scrolloff<
+  set scrolloff<
   call assert_equal(-1, &l:scrolloff)
   call assert_equal(15, &scrolloff)
 
@@ -1620,17 +1622,17 @@ func Test_set_boolean_global_local_option()
   call assert_equal(0, &l:autoread)
   call assert_equal(0, &autoread)
 
-  " :set {option}< set the effective value of {option} to its global value.
+  " :setlocal {option}< set the effective value of {option} to its global value.
-  set autoread<
+  "set autoread<
-  " Nvim: local value is removed
+  setlocal autoread<
-  " call assert_equal(1, &l:autoread)
+  call assert_equal(1, &l:autoread)
-  call assert_equal(-1, &l:autoread)
   call assert_equal(1, &autoread)
 
-  " :setlocal {option}< removes the local value, so that the global value will be used.
+  " :set {option}< removes the local value, so that the global value will be used.
   setglobal noautoread
   setlocal autoread
-  setlocal autoread<
+  "setlocal autoread<
+  set autoread<
   call assert_equal(-1, &l:autoread)
   call assert_equal(0, &autoread)
 

test/old/testdir/test_undo.vim

@@ -187,7 +187,8 @@ func Test_global_local_undolevels()
 
   " Resetting the local 'undolevels' value to use the global value
   setlocal undolevels=5
-  setlocal undolevels<
+  "setlocal undolevels<
+  set undolevels<
   call assert_equal(-123456, &l:undolevels)
 
   " Drop created windows