]> git.eshelyaron.com Git - emacs.git/commitdiff
Fix bug#30846, along with misc cleanups found along the way
authorStefan Monnier <monnier@iro.umontreal.ca>
Fri, 23 Mar 2018 15:29:06 +0000 (11:29 -0400)
committerStefan Monnier <monnier@iro.umontreal.ca>
Fri, 23 Mar 2018 15:29:06 +0000 (11:29 -0400)
* 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.

src/alloc.c
src/buffer.c
src/data.c
src/lisp.h
test/src/data-tests.el

index 5eaf7cbc1c6b5e2e965e8f616b0a0ee7fc17f4cf..369592d70ee60f81154118256aa73f438f512a79 100644 (file)
@@ -6342,12 +6342,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);
index f8c57a74b4ea353f12109018e515d140bd943113..14837372d345f7b9e171c0c5efaed8d0be958ad7 100644 (file)
@@ -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));
-       }
-    }
-}
 \f
 /* 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.
index 06308c62c49191dd36a20dbdb185c839d4f3799e..a7fab1ef58a3ce6990b292fafb4fb29c91a7d665 100644 (file)
@@ -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;
 }
 
@@ -1945,30 +1944,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;
 }
 
@@ -2030,11 +2016,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;
index aefdaeaf12f6b8b78200e8bbeda74e289e6b7be0..f0c0c5a14a54f3513d71e678605cbba34f3b58b2 100644 (file)
@@ -2679,18 +2679,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
index 33b00d6c9ad1e208eb9d4c4cb29b40539dff8146..3cd537859fdbc8d823b46d9376938089b9c3e1f1 100644 (file)
@@ -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.
 
@@ -499,3 +499,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