bug#70183: [PATCH] Fix + ert for multiple watcher notifications (2 of 9)

[Stefan Monnier]

>   Additionally, I moved the XSETSYMBOL to the top from inside the
> localized to point out that while redirection was important, the notes
> there didn't express how important it was to convert the symbol to bare.

Maybe a better option here is to signal an error if passed a non-bare symbol.
At the very least it'd be useful to know how a sympos arrived here, in
order to decide whether we want to tolerate them here or whether we should
better fix the caller(s).

> Subject: [PATCH 02/31] Fix multiple watcher calls in set_default_internal
>  (bug#00001)
>
> * src/data.c (set_default_internal):
> Variable watchers will now execute one time if called using an alias.
> (bug#00001)
>
> ---
>  src/data.c | 43 ++++++++++++++++++++++++++-----------------
>  1 file changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/src/data.c b/src/data.c
> index c42424497ad..48da5cee429 100644
> --- a/src/data.c
> +++ b/src/data.c
> @@ -1986,31 +1986,21 @@ set_default_internal (Lisp_Object symbol, Lisp_Object 
> value,
>  {
>    CHECK_SYMBOL (symbol);
>    struct Lisp_Symbol *sym = XSYMBOL (symbol);
> -  switch (sym->u.s.trapped_write)
> +  /* Update for aliasing and sanitize the input to ensure only a bare
> +     symbol is stored.  It may be accessed with positions disabled.  */
> + start:
> +  XSETSYMBOL (symbol, sym);
> +
> +  if (sym->u.s.trapped_write == SYMBOL_NOWRITE)
>      {
> -    case SYMBOL_NOWRITE:
>        if (NILP (Fkeywordp (symbol))
>            || !EQ (value, Fsymbol_value (symbol)))
>          xsignal1 (Qsetting_constant, symbol);
>        else
>          /* Allow setting keywords to their own value.  */
> -        return;
> -
> -    case SYMBOL_TRAPPED_WRITE:
> -      /* Don't notify here if we're going to call Fset anyway.  */
> -      if (sym->u.s.redirect != SYMBOL_PLAINVAL
> -          /* Setting due to thread switching doesn't count.  */
> -          && bindflag != SET_INTERNAL_THREAD_SWITCH)
> -        notify_variable_watchers (symbol, value, Qset_default, Qnil);
> -      break;
> -
> -    case SYMBOL_UNTRAPPED_WRITE:
> -      break;
> -
> -    default: emacs_abort ();
> +     return;
>      }

I think we should check `sym->u.s.trapped_write`
only after following the alias redirection.


        Stefan