Commit 2305ee79 authored by Stefan Monnier's avatar Stefan Monnier Committed by Noam Postavsky

Fix bug#30846, along with misc cleanups found along the way

* test/src/data-tests.el (data-tests-kill-all-local-variables): New test.

* src/buffer.c (swap_out_buffer_local_variables): Remove.
Fuse the body of its loop into that of reset_buffer_local_variables.
(Fkill_buffer, Fkill_all_local_variables): Don't call it any more.
(reset_buffer_local_variables): Make sure the buffer's local binding
is swapped out before removing it from the alist (bug#30846).
Call watchers before actually killing the var.

* src/data.c (Fmake_local_variable): Simplify.
Use swap_in_global_binding to swap out any local binding, instead of
a mix of find_symbol_value followed by messing with where&found.
Don't call swap_in_symval_forwarding since the currently swapped
binding is never one we've modified.
(Fkill_local_variable): Use swap_in_global_binding rather than messing
with where&found to try and trick find_symbol_value into doing the same.

* src/alloc.c (mark_localized_symbol): 'where' can't be a frame any more.

(cherry picked from commit 3ddff080)
parent 47ab98eb
Pipeline #31 failed with stage
......@@ -6334,12 +6334,8 @@ mark_localized_symbol (struct Lisp_Symbol *ptr)
{
struct Lisp_Buffer_Local_Value *blv = SYMBOL_BLV (ptr);
Lisp_Object where = blv->where;
/* If the value is set up for a killed buffer or deleted
frame, restore its global binding. If the value is
forwarded to a C variable, either it's not a Lisp_Object
var, or it's staticpro'd already. */
if ((BUFFERP (where) && !BUFFER_LIVE_P (XBUFFER (where)))
|| (FRAMEP (where) && !FRAME_LIVE_P (XFRAME (where))))
/* If the value is set up for a killed buffer restore its global binding. */
if ((BUFFERP (where) && !BUFFER_LIVE_P (XBUFFER (where))))
swap_in_global_binding (ptr);
mark_object (blv->where);
mark_object (blv->valcell);
......
......@@ -108,7 +108,6 @@ int last_per_buffer_idx;
static void call_overlay_mod_hooks (Lisp_Object list, Lisp_Object overlay,
bool after, Lisp_Object arg1,
Lisp_Object arg2, Lisp_Object arg3);
static void swap_out_buffer_local_variables (struct buffer *b);
static void reset_buffer_local_variables (struct buffer *, bool);
/* Alist of all buffer names vs the buffers. This used to be
......@@ -991,10 +990,29 @@ reset_buffer_local_variables (struct buffer *b, bool permanent_too)
else
{
Lisp_Object tmp, last = Qnil;
Lisp_Object buffer;
XSETBUFFER (buffer, b);
for (tmp = BVAR (b, local_var_alist); CONSP (tmp); tmp = XCDR (tmp))
{
Lisp_Object local_var = XCAR (XCAR (tmp));
Lisp_Object prop = Fget (local_var, Qpermanent_local);
Lisp_Object sym = local_var;
/* Watchers are run *before* modifying the var. */
if (XSYMBOL (local_var)->u.s.trapped_write == SYMBOL_TRAPPED_WRITE)
notify_variable_watchers (local_var, Qnil,
Qmakunbound, Fcurrent_buffer ());
eassert (XSYMBOL (sym)->u.s.redirect == SYMBOL_LOCALIZED);
/* Need not do anything if some other buffer's binding is
now cached. */
if (EQ (SYMBOL_BLV (XSYMBOL (sym))->where, buffer))
{
/* Symbol is set up for this buffer's old local value:
swap it out! */
swap_in_global_binding (XSYMBOL (sym));
}
if (!NILP (prop))
{
......@@ -1034,10 +1052,6 @@ reset_buffer_local_variables (struct buffer *b, bool permanent_too)
bset_local_var_alist (b, XCDR (tmp));
else
XSETCDR (last, XCDR (tmp));
if (XSYMBOL (local_var)->u.s.trapped_write == SYMBOL_TRAPPED_WRITE)
notify_variable_watchers (local_var, Qnil,
Qmakunbound, Fcurrent_buffer ());
}
}
......@@ -1867,7 +1881,6 @@ cleaning up all windows currently displaying the buffer to be killed. */)
won't be protected from GC. They would be protected
if they happened to remain cached in their symbols.
This gets rid of them for certain. */
swap_out_buffer_local_variables (b);
reset_buffer_local_variables (b, 1);
bset_name (b, Qnil);
......@@ -2737,11 +2750,6 @@ the normal hook `change-major-mode-hook'. */)
{
run_hook (Qchange_major_mode_hook);
/* Make sure none of the bindings in local_var_alist
remain swapped in, in their symbols. */
swap_out_buffer_local_variables (current_buffer);
/* Actually eliminate all local bindings of this buffer. */
reset_buffer_local_variables (current_buffer, 0);
......@@ -2753,31 +2761,6 @@ the normal hook `change-major-mode-hook'. */)
return Qnil;
}
/* Make sure no local variables remain set up with buffer B
for their current values. */
static void
swap_out_buffer_local_variables (struct buffer *b)
{
Lisp_Object oalist, alist, buffer;
XSETBUFFER (buffer, b);
oalist = BVAR (b, local_var_alist);
for (alist = oalist; CONSP (alist); alist = XCDR (alist))
{
Lisp_Object sym = XCAR (XCAR (alist));
eassert (XSYMBOL (sym)->u.s.redirect == SYMBOL_LOCALIZED);
/* Need not do anything if some other buffer's binding is
now cached. */
if (EQ (SYMBOL_BLV (XSYMBOL (sym))->where, buffer))
{
/* Symbol is set up for this buffer's old local value:
swap it out! */
swap_in_global_binding (XSYMBOL (sym));
}
}
}
/* Find all the overlays in the current buffer that contain position POS.
Return the number found, and store them in a vector in *VEC_PTR.
......
......@@ -1188,7 +1188,7 @@ swap_in_global_binding (struct Lisp_Symbol *symbol)
/* Indicate that the global binding is set up now. */
set_blv_where (blv, Qnil);
set_blv_found (blv, 0);
set_blv_found (blv, false);
}
/* Set up the buffer-local symbol SYMBOL for validity in the current buffer.
......@@ -1257,7 +1257,6 @@ find_symbol_value (Lisp_Object symbol)
swap_in_symval_forwarding (sym, blv);
return blv->fwd ? do_symval_forwarding (blv->fwd) : blv_value (blv);
}
/* FALLTHROUGH */
case SYMBOL_FORWARDED:
return do_symval_forwarding (SYMBOL_FWD (sym));
default: emacs_abort ();
......@@ -1366,7 +1365,7 @@ set_internal (Lisp_Object symbol, Lisp_Object newval, Lisp_Object where,
tem1 = assq_no_quit (symbol,
BVAR (XBUFFER (where), local_var_alist));
set_blv_where (blv, where);
blv->found = 1;
blv->found = true;
if (NILP (tem1))
{
......@@ -1381,7 +1380,7 @@ set_internal (Lisp_Object symbol, Lisp_Object newval, Lisp_Object where,
if (bindflag || !blv->local_if_set
|| let_shadows_buffer_binding_p (sym))
{
blv->found = 0;
blv->found = false;
tem1 = blv->defcell;
}
/* If it's a local_if_set, being set not bound,
......@@ -1796,7 +1795,7 @@ make_blv (struct Lisp_Symbol *sym, bool forwarded,
blv->local_if_set = 0;
set_blv_defcell (blv, tem);
set_blv_valcell (blv, tem);
set_blv_found (blv, 0);
set_blv_found (blv, false);
return blv;
}
......@@ -1946,30 +1945,17 @@ Instead, use `add-hook' and specify t for the LOCAL argument. */)
CALLN (Fmessage, format, SYMBOL_NAME (variable));
}
/* Swap out any local binding for some other buffer, and make
sure the current value is permanently recorded, if it's the
default value. */
find_symbol_value (variable);
if (BUFFERP (blv->where) && current_buffer == XBUFFER (blv->where))
/* Make sure the current value is permanently recorded, if it's the
default value. */
swap_in_global_binding (sym);
bset_local_var_alist
(current_buffer,
Fcons (Fcons (variable, XCDR (blv->defcell)),
BVAR (current_buffer, local_var_alist)));
/* Make sure symbol does not think it is set up for this buffer;
force it to look once again for this buffer's value. */
if (current_buffer == XBUFFER (blv->where))
set_blv_where (blv, Qnil);
set_blv_found (blv, 0);
}
/* If the symbol forwards into a C variable, then load the binding
for this buffer now. If C code modifies the variable before we
load the binding in, then that new value will clobber the default
binding the next time we unload it. */
if (blv->fwd)
swap_in_symval_forwarding (sym, blv);
return variable;
}
......@@ -2031,11 +2017,7 @@ From now on the default value will apply in this buffer. Return VARIABLE. */)
{
Lisp_Object buf; XSETBUFFER (buf, current_buffer);
if (EQ (buf, blv->where))
{
set_blv_where (blv, Qnil);
blv->found = 0;
find_symbol_value (variable);
}
swap_in_global_binding (sym);
}
return variable;
......
......@@ -2587,18 +2587,15 @@ struct Lisp_Buffer_Objfwd
in the buffer structure itself. They are handled differently,
using struct Lisp_Buffer_Objfwd.)
The `realvalue' slot holds the variable's current value, or a
forwarding pointer to where that value is kept. This value is the
one that corresponds to the loaded binding. To read or set the
variable, you must first make sure the right binding is loaded;
then you can access the value in (or through) `realvalue'.
`where' is the buffer for which the loaded binding was found. If
it has changed, to make sure the right binding is loaded it is
The `valcell' slot holds the variable's current value (unless `fwd'
is set). This value is the one that corresponds to the loaded binding.
To read or set the variable, you must first make sure the right binding
is loaded; then you can access the value in (or through) `valcell'.
`where' is the buffer for which the loaded binding was found.
If it has changed, to make sure the right binding is loaded it is
necessary to find which binding goes with the current buffer, then
load it. To load it, first unload the previous binding, then copy
the value of the new binding into `realvalue' (or through it).
Also update LOADED-BINDING to point to the newly loaded binding.
load it. To load it, first unload the previous binding.
`local_if_set' indicates that merely setting the variable creates a
local binding for the current buffer. Otherwise the latter, setting
......
;;; data-tests.el --- tests for src/data.c
;;; data-tests.el --- tests for src/data.c -*- lexical-binding:t -*-
;; Copyright (C) 2013-2018 Free Software Foundation, Inc.
......@@ -484,3 +484,20 @@ comparing the subr with a much slower lisp implementation."
(remove-variable-watcher 'data-tests-lvar collect-watch-data)
(setq data-tests-lvar 6)
(should (null watch-data)))))
(ert-deftest data-tests-kill-all-local-variables () ;bug#30846
(with-temp-buffer
(setq-local data-tests-foo1 1)
(setq-local data-tests-foo2 2)
(setq-local data-tests-foo3 3)
(let ((oldfoo2 nil))
(add-variable-watcher 'data-tests-foo2
(lambda (&rest _)
(setq oldfoo2 (bound-and-true-p data-tests-foo2))))
(kill-all-local-variables)
(should (equal oldfoo2 '2)) ;Watcher is run before changing the var.
(should (not (or (bound-and-true-p data-tests-foo1)
(bound-and-true-p data-tests-foo2)
(bound-and-true-p data-tests-foo3)))))))
;;; data-tests.el ends here
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment