From: Stefan Monnier Date: Fri, 23 Mar 2018 15:29:06 +0000 (-0400) Subject: Fix bug#30846, along with misc cleanups found along the way X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=refs%2Fheads%2Fscratch%2Fnp%2Fbackports-26.2;p=emacs.git 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 3ddff080341580eb6fc18d907181e9cc2301f62d) --- diff --git a/src/alloc.c b/src/alloc.c index 09d61b7e5f3..7baaa512c20 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -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); diff --git a/src/buffer.c b/src/buffer.c index 9b54e4b7787..b0cee717036 100644 --- a/src/buffer.c +++ b/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. diff --git a/src/data.c b/src/data.c index 45b2bf73026..4bee194e296 100644 --- a/src/data.c +++ b/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; diff --git a/src/lisp.h b/src/lisp.h index cd6d07288e0..56ad8b814b6 100644 --- a/src/lisp.h +++ b/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'. - - `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 diff --git a/test/src/data-tests.el b/test/src/data-tests.el index dda1278b6d4..91463db113c 100644 --- a/test/src/data-tests.el +++ b/test/src/data-tests.el @@ -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