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 3ddff08034)
This commit is contained in:
Stefan Monnier
Noam Postavsky
parent
3ba5fc2bbe
commit
ed962f2b8a
5 changed files with 55 additions and 80 deletions
|
|
@ -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);
|
||||
|
|
|
|||
55
src/buffer.c
55
src/buffer.c
|
|
@ -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.
|
||||
|
|
|
|||
36
src/data.c
36
src/data.c
|
|
@ -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;
|
||||
|
|
|
|||
17
src/lisp.h
17
src/lisp.h
|
|
@ -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'.
|
||||
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
|
||||
`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
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
;;; 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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue