internal handling of buffer-local variables

[Stefan Monnier]

While tracking down some unrelated problem, I bumped into the appended code.

As you can seem, the comment says that swap_out_buffer_local_variables
is supposed to make sure no var has the bindings for B loaded.
But then the code goes on and checks whether the binding for
current_buffer is loaded.  Worse yet: when it encounters such a thing,
it loads the global binding but sets the symbols' `buffer' field to
claim that the loaded binding is for B, thus leading to an
inconsistent state.

This can lead to problems further down where a buffer that is now dead
(and whose local_var_alist is empty) has a binding loaded (and with
found_for_buffer set).  I'm not sure the problem can really cause
trouble because found_for_buffer is not used systematically
(e.g. Flocal_variable_p ignores it), but it looks dangerous.
I'm tempted to replace the body of the if with a call to
swap_in_global_binding.  I've tried this change here and it seems to
work correctly, but I'd like to hear what other people think about it.


        Stefan


/* Make sure no local variables remain set up with buffer B
   for their current values.  */

static void
swap_out_buffer_local_variables (b)
     struct buffer *b;
{
  Lisp_Object oalist, alist, sym, tem, buffer;

  XSETBUFFER (buffer, b);
  oalist = b->local_var_alist;

  for (alist = oalist; CONSP (alist); alist = XCDR (alist))
    {
      sym = XCAR (XCAR (alist));

      /* Need not do anything if some other buffer's binding is now encached.  
*/
      tem = XBUFFER_LOCAL_VALUE (SYMBOL_VALUE (sym))->buffer;
      if (BUFFERP (tem) && XBUFFER (tem) == current_buffer)
        {
          /* Symbol is set up for this buffer's old local value.
             Set it up for the current buffer with the default value.  */

          tem = XBUFFER_LOCAL_VALUE (SYMBOL_VALUE (sym))->cdr;
          /* Store the symbol's current value into the alist entry
             it is currently set up for.  This is so that, if the
             local is marked permanent, and we make it local again
             later in Fkill_all_local_variables, we don't lose the value.  */
          XSETCDR (XCAR (tem),
                   do_symval_forwarding (XBUFFER_LOCAL_VALUE (SYMBOL_VALUE 
(sym))->realvalue));
          /* Switch to the symbol's default-value alist entry.  */
          XSETCAR (tem, tem);
          /* Mark it as current for buffer B.  */
          XBUFFER_LOCAL_VALUE (SYMBOL_VALUE (sym))->buffer = buffer;
          /* Store the current value into any forwarding in the symbol.  */
          store_symval_forwarding (sym,
                                   XBUFFER_LOCAL_VALUE (SYMBOL_VALUE 
(sym))->realvalue,
                                   XCDR (tem), NULL);
        }
    }
}