* bug#3467: 23.0.94; let + make-local-variable => let value made global
@ 2009-06-04 21:14 Lennart Borgman
2009-06-05 14:21 ` Lennart Borgman
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Lennart Borgman @ 2009-06-04 21:14 UTC (permalink / raw
To: emacs-pretest-bug
If you eval these lines
(defvar w14 "global")
(defvar w15 "global")
(let ((w14 "let")
(w15 "let"))
(set (make-local-variable 'w14) "local")
(message "w14 maybe let: in buffer=%S, global=%S" w14 (default-value 'w14))
(message "w15 maybe let: in buffer=%S, global=%S" w15 (default-value 'w15)))
(message "w14 top level: in buffer=%S, global=%S" w14 (default-value 'w14))
(message "w15 top level: in buffer=%S, global=%S" w15 (default-value 'w15))
the output will be
w14 maybe let: in buffer="local", global="let"
w15 maybe let: in buffer="let", global="let"
w14 top level: in buffer="global", global="let"
w15 top level: in buffer="global", global="global"
All values here except w14 global value on next last line are arguably
correct. The last value of w14 should be "global", not "let".
It looks like perhaps the call to (make-local-variable w14) does not
mark the "global let" value of w14 as let bound (or removes that
mark).
In GNU Emacs 23.0.94.1 (i386-mingw-nt5.1.2600)
of 2009-05-23
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (3.4) --no-opt --cflags
-Ic:/g/include -fno-crossjumping'
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#3467: 23.0.94; let + make-local-variable => let value made global
2009-06-04 21:14 bug#3467: 23.0.94; let + make-local-variable => let value made global Lennart Borgman
@ 2009-06-05 14:21 ` Lennart Borgman
2009-06-05 14:36 ` Stefan Monnier
2009-06-06 20:05 ` bug#3467: 23.0.94; let + make-local-variable => let value made global Richard Stallman
2 siblings, 0 replies; 13+ messages in thread
From: Lennart Borgman @ 2009-06-05 14:21 UTC (permalink / raw
To: 3467
On Thu, Jun 4, 2009 at 11:14 PM, Lennart
Borgman<lennart.borgman@gmail.com> wrote:
> If you eval these lines
>
> (defvar w14 "global")
> (defvar w15 "global")
> (let ((w14 "let")
> (w15 "let"))
> (set (make-local-variable 'w14) "local")
> (message "w14 maybe let: in buffer=%S, global=%S" w14 (default-value 'w14))
> (message "w15 maybe let: in buffer=%S, global=%S" w15 (default-value 'w15)))
> (message "w14 top level: in buffer=%S, global=%S" w14 (default-value 'w14))
> (message "w15 top level: in buffer=%S, global=%S" w15 (default-value 'w15))
>
> the output will be
>
> w14 maybe let: in buffer="local", global="let"
> w15 maybe let: in buffer="let", global="let"
> w14 top level: in buffer="global", global="let"
> w15 top level: in buffer="global", global="global"
>
> All values here except w14 global value on next last line are arguably
> correct. The last value of w14 should be "global", not "let".
>
> It looks like perhaps the call to (make-local-variable w14) does not
> mark the "global let" value of w14 as let bound (or removes that
> mark).
I think the semantics of let, make-variable-buffer-local and buffer
local variables must be examined a bit to clarify this:
I think let should preserve "buffer localness" otherwise it might do
something quite different from what a user probably expect.
Fortunately this as far as I can see requires only a change in Flet.
After Fprogn and before calling unbound_to the variable values must be
reset to those matching the buffer and frame localness before let.
spec_bind has saved the buffer and frame localness just before Fprogn
so I guess one can compare with what is saved there.
However no one would want me to write the C code for this... ;-) -
Someone who is better than me at that should preferrably do it.
(BTW: gmail does not automatically enter the reply address when I make
a reply to the original bug mail. Is that a bug in gmail?)
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#3467: 23.0.94; let + make-local-variable => let value made global
2009-06-04 21:14 bug#3467: 23.0.94; let + make-local-variable => let value made global Lennart Borgman
2009-06-05 14:21 ` Lennart Borgman
@ 2009-06-05 14:36 ` Stefan Monnier
2009-06-05 15:04 ` Lennart Borgman
2009-06-06 20:05 ` bug#3467: 23.0.94; let + make-local-variable => let value made global Richard Stallman
2 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2009-06-05 14:36 UTC (permalink / raw
To: Lennart Borgman; +Cc: emacs-pretest-bug, 3467
> If you eval these lines
> (defvar w14 "global")
> (defvar w15 "global")
> (let ((w14 "let")
> (w15 "let"))
> (set (make-local-variable 'w14) "local")
> (message "w14 maybe let: in buffer=%S, global=%S" w14 (default-value 'w14))
> (message "w15 maybe let: in buffer=%S, global=%S" w15 (default-value 'w15)))
> (message "w14 top level: in buffer=%S, global=%S" w14 (default-value 'w14))
> (message "w15 top level: in buffer=%S, global=%S" w15 (default-value 'w15))
> the output will be
> w14 maybe let: in buffer="local", global="let"
> w15 maybe let: in buffer="let", global="let"
> w14 top level: in buffer="global", global="let"
> w15 top level: in buffer="global", global="global"
> All values here except w14 global value on next last line are arguably
> correct. The last value of w14 should be "global", not "let".
> It looks like perhaps the call to (make-local-variable w14) does not
> mark the "global let" value of w14 as let bound (or removes that
> mark).
Given the way let-binding and buffer-local bindings are currently
implemented, it's difficult to make it work correctly in all corner
cases, and even more so without slowing down the common case.
So don't hold your breath.
Stefan
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#3467: 23.0.94; let + make-local-variable => let value made global
2009-06-05 14:36 ` Stefan Monnier
@ 2009-06-05 15:04 ` Lennart Borgman
2009-06-05 22:48 ` Lennart Borgman
0 siblings, 1 reply; 13+ messages in thread
From: Lennart Borgman @ 2009-06-05 15:04 UTC (permalink / raw
To: Stefan Monnier; +Cc: emacs-pretest-bug, 3467
On Fri, Jun 5, 2009 at 4:36 PM, Stefan Monnier<monnier@iro.umontreal.ca> wrote:
>> If you eval these lines
>> (defvar w14 "global")
>> (defvar w15 "global")
>> (let ((w14 "let")
>> (w15 "let"))
>> (set (make-local-variable 'w14) "local")
>> (message "w14 maybe let: in buffer=%S, global=%S" w14 (default-value 'w14))
>> (message "w15 maybe let: in buffer=%S, global=%S" w15 (default-value 'w15)))
>> (message "w14 top level: in buffer=%S, global=%S" w14 (default-value 'w14))
>> (message "w15 top level: in buffer=%S, global=%S" w15 (default-value 'w15))
>
>> the output will be
>
>> w14 maybe let: in buffer="local", global="let"
>> w15 maybe let: in buffer="let", global="let"
>> w14 top level: in buffer="global", global="let"
>> w15 top level: in buffer="global", global="global"
>
>> All values here except w14 global value on next last line are arguably
>> correct. The last value of w14 should be "global", not "let".
>
>> It looks like perhaps the call to (make-local-variable w14) does not
>> mark the "global let" value of w14 as let bound (or removes that
>> mark).
>
> Given the way let-binding and buffer-local bindings are currently
> implemented, it's difficult to make it work correctly in all corner
> cases, and even more so without slowing down the common case.
> So don't hold your breath.
I gave a suggestion in the next message for how to implement this:
Check buffer and frame localness before unbind_to in Flet. Would that
really be expensive?
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#3467: 23.0.94; let + make-local-variable => let value made global
2009-06-05 15:04 ` Lennart Borgman
@ 2009-06-05 22:48 ` Lennart Borgman
2009-06-05 23:08 ` Lennart Borgman
2009-06-06 20:11 ` Lennart Borgman
0 siblings, 2 replies; 13+ messages in thread
From: Lennart Borgman @ 2009-06-05 22:48 UTC (permalink / raw
To: Stefan Monnier; +Cc: emacs-pretest-bug, 3467
On Fri, Jun 5, 2009 at 5:04 PM, Lennart
Borgman<lennart.borgman@gmail.com> wrote:
> On Fri, Jun 5, 2009 at 4:36 PM, Stefan Monnier<monnier@iro.umontreal.ca> wrote:
>>> If you eval these lines
>>> (defvar w14 "global")
>>> (defvar w15 "global")
>>> (let ((w14 "let")
>>> (w15 "let"))
>>> (set (make-local-variable 'w14) "local")
>>> (message "w14 maybe let: in buffer=%S, global=%S" w14 (default-value 'w14))
>>> (message "w15 maybe let: in buffer=%S, global=%S" w15 (default-value 'w15)))
>>> (message "w14 top level: in buffer=%S, global=%S" w14 (default-value 'w14))
>>> (message "w15 top level: in buffer=%S, global=%S" w15 (default-value 'w15))
>>
>>> the output will be
>>
>>> w14 maybe let: in buffer="local", global="let"
>>> w15 maybe let: in buffer="let", global="let"
>>> w14 top level: in buffer="global", global="let"
>>> w15 top level: in buffer="global", global="global"
>>
>>> All values here except w14 global value on next last line are arguably
>>> correct. The last value of w14 should be "global", not "let".
>>
>>> It looks like perhaps the call to (make-local-variable w14) does not
>>> mark the "global let" value of w14 as let bound (or removes that
>>> mark).
>>
>> Given the way let-binding and buffer-local bindings are currently
>> implemented, it's difficult to make it work correctly in all corner
>> cases, and even more so without slowing down the common case.
>> So don't hold your breath.
>
> I gave a suggestion in the next message for how to implement this:
> Check buffer and frame localness before unbind_to in Flet. Would that
> really be expensive?
Sigh, and my suggestion was of course unnecessary stupid. What is
needed is of course to record values and frame+buffer localness and
dito values and reset them. Nothing less than this will ever work
correctly, or?
And does not this apply to all uses of specbind + unbind_to?
Can it be sufficient to just change specbind and unbind_to? Is there
anything else that will be affected by changes in the specbind stack?
Since info about buffer+frame is alwas needed should specbinding be
changed to the below form?
struct specbinding
{
Lisp_Object symbol;
Lisp_Object old_value;
Lisp_Object old_buffer_value;
Lisp_Object old_frame_value;
specbinding_func func;
Lisp_Object unused; /* Dividing by 16 is faster than by 12 */
Lisp_Object unused;
Lisp_Object unused;
};
Is this structure used by other functions than specbind and unbind_to?
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#3467: 23.0.94; let + make-local-variable => let value made global
2009-06-05 22:48 ` Lennart Borgman
@ 2009-06-05 23:08 ` Lennart Borgman
2009-06-05 23:32 ` Lennart Borgman
2009-06-06 20:11 ` Lennart Borgman
1 sibling, 1 reply; 13+ messages in thread
From: Lennart Borgman @ 2009-06-05 23:08 UTC (permalink / raw
To: Stefan Monnier; +Cc: emacs-pretest-bug, 3467
On Sat, Jun 6, 2009 at 12:48 AM, Lennart
Borgman<lennart.borgman@gmail.com> wrote:
>
> Sigh, and my suggestion was of course unnecessary stupid. What is
> needed is of course to record values and frame+buffer localness and
> dito values and reset them. Nothing less than this will ever work
> correctly, or?
>
> And does not this apply to all uses of specbind + unbind_to?
>
> Can it be sufficient to just change specbind and unbind_to? Is there
> anything else that will be affected by changes in the specbind stack?
> Since info about buffer+frame is alwas needed should specbinding be
> changed to the below form?
>
> struct specbinding
> {
> Lisp_Object symbol;
> Lisp_Object old_value;
> Lisp_Object old_buffer_value;
> Lisp_Object old_frame_value;
> specbinding_func func;
> Lisp_Object unused; /* Dividing by 16 is faster than by 12 */
> Lisp_Object unused;
> Lisp_Object unused;
> };
>
> Is this structure used by other functions than specbind and unbind_to?
... and defvar ... but I do not think there are any more uses of
specbinding's structure. Or does some allocation need to change?
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#3467: 23.0.94; let + make-local-variable => let value made global
2009-06-05 23:08 ` Lennart Borgman
@ 2009-06-05 23:32 ` Lennart Borgman
0 siblings, 0 replies; 13+ messages in thread
From: Lennart Borgman @ 2009-06-05 23:32 UTC (permalink / raw
To: Stefan Monnier; +Cc: emacs-pretest-bug, 3467
On Sat, Jun 6, 2009 at 1:08 AM, Lennart
Borgman<lennart.borgman@gmail.com> wrote:
>
> ... and defvar ... but I do not think there are any more uses of
> specbinding's structure. Or does some allocation need to change?
... and let_shadows_buffer_binding_p which is a bit lonely without
its sister let_shadows_frame_binding_p (or is such a sister not of any
importance? I do not really know about frame local variables, but
specbind seems to know) ...
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#3467: 23.0.94; let + make-local-variable => let value made global
2009-06-04 21:14 bug#3467: 23.0.94; let + make-local-variable => let value made global Lennart Borgman
2009-06-05 14:21 ` Lennart Borgman
2009-06-05 14:36 ` Stefan Monnier
@ 2009-06-06 20:05 ` Richard Stallman
2009-06-06 20:13 ` Lennart Borgman
2 siblings, 1 reply; 13+ messages in thread
From: Richard Stallman @ 2009-06-06 20:05 UTC (permalink / raw
To: Lennart Borgman, 3467; +Cc: emacs-pretest-bug
I think this is a use case that should be avoided.
Maybe it can be made to work "right", but if that's difficult,
I'd rather tell people not to do it.
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#3467: 23.0.94; let + make-local-variable => let value made global
2009-06-05 22:48 ` Lennart Borgman
2009-06-05 23:08 ` Lennart Borgman
@ 2009-06-06 20:11 ` Lennart Borgman
2009-06-07 1:11 ` Lennart Borgman
1 sibling, 1 reply; 13+ messages in thread
From: Lennart Borgman @ 2009-06-06 20:11 UTC (permalink / raw
To: Stefan Monnier; +Cc: 3467
[-- Attachment #1: Type: text/plain, Size: 1555 bytes --]
On Sat, Jun 6, 2009 at 12:48 AM, Lennart
Borgman<lennart.borgman@gmail.com> wrote:
>
> Sigh, and my suggestion was of course unnecessary stupid. What is
> needed is of course to record values and frame+buffer localness and
> dito values and reset them. Nothing less than this will ever work
> correctly, or?
>
> And does not this apply to all uses of specbind + unbind_to?
>
> Can it be sufficient to just change specbind and unbind_to? Is there
> anything else that will be affected by changes in the specbind stack?
> Since info about buffer+frame is alwas needed should specbinding be
> changed to the below form?
>
> struct specbinding
> {
> Lisp_Object symbol;
> Lisp_Object old_value;
> Lisp_Object old_buffer_value;
> Lisp_Object old_frame_value;
> specbinding_func func;
> Lisp_Object unused; /* Dividing by 16 is faster than by 12 */
> Lisp_Object unused;
> Lisp_Object unused;
> };
Here is a bit modified suggestion and some code where I try to
implement it. Please notice that I have not tested the code. It is
very possible that I have misunderstood something so please look at
the code.
struct specbinding
{
Lisp_Object symbol;
Lisp_Object old_value;
specbinding_func func;
// total 8 fields
Lisp_Object need_test_local;
Lisp_Object old_buffer;
Lisp_Object old_buffer_value;
Lisp_Object old_frame;
Lisp_Object old_frame_value;
//Lisp_Object unused; /* Dividing by 16 is faster than by 12 */
};
[-- Attachment #2: eval.diff --]
[-- Type: application/octet-stream, Size: 7463 bytes --]
*** eval.c 2009-06-06 22:06:33.359375000 +0200
--- eval-orig.c 2009-03-27 09:21:26.453125000 +0100
***************
*** 3298,3304 ****
specpdl_ptr->symbol = symbol;
specpdl_ptr->old_value = valcontents;
specpdl_ptr->func = NULL;
- //specpdl_ptr->need_test_local = Qnil; // no need to set this
++specpdl_ptr;
SET_SYMBOL_VALUE (symbol, value);
}
--- 3298,3303 ----
***************
*** 3306,3317 ****
{
Lisp_Object ovalue = find_symbol_value (symbol);
specpdl_ptr->func = 0;
! //specpdl_ptr->old_value = ovalue;
! /* Fix-me: Store symbol global value in old_value */
! specpdl_ptr->old_value = do_symval_forwarding (valcontents);
! specpdl_ptr->need_test_local = Qt;
! specpdl_ptr->old_buffer= Qunbound;
! specpdl_ptr->old_frame= Qunbound;
valcontents = XSYMBOL (symbol)->value;
--- 3305,3311 ----
{
Lisp_Object ovalue = find_symbol_value (symbol);
specpdl_ptr->func = 0;
! specpdl_ptr->old_value = ovalue;
valcontents = XSYMBOL (symbol)->value;
***************
*** 3324,3349 ****
/* For a local variable, record both the symbol and which
buffer's or frame's value we are saving. */
! /* Fix-me: This only saves the current used local binding,
! not both as it should */
! if (!NILP (Flocal_variable_p (symbol, Qnil))) {
where = current_buffer;
! specpdl_ptr->old_buffer = where;
! specpdl_ptr->old_buffer_value = ovalue;
! } else if (BUFFER_LOCAL_VALUEP (valcontents)
! && XBUFFER_LOCAL_VALUE (valcontents)->found_for_frame) {
where = XBUFFER_LOCAL_VALUE (valcontents)->frame;
! specpdl_ptr->old_frame = where;
! specpdl_ptr->old_frame_value = ovalue;
! } else {
where = Qnil;
- }
/* We're not using the `unused' slot in the specbinding
structure because this would mean we have to do more
work for simple variables. */
! //specpdl_ptr->symbol = Fcons (symbol, Fcons (where, current_buffer));
! specpdl_ptr->symbol = symbol;
/* If SYMBOL is a per-buffer variable which doesn't have a
buffer-local value here, make the `let' change the global
--- 3318,3335 ----
/* For a local variable, record both the symbol and which
buffer's or frame's value we are saving. */
! if (!NILP (Flocal_variable_p (symbol, Qnil)))
where = current_buffer;
! else if (BUFFER_LOCAL_VALUEP (valcontents)
! && XBUFFER_LOCAL_VALUE (valcontents)->found_for_frame)
where = XBUFFER_LOCAL_VALUE (valcontents)->frame;
! else
where = Qnil;
/* We're not using the `unused' slot in the specbinding
structure because this would mean we have to do more
work for simple variables. */
! specpdl_ptr->symbol = Fcons (symbol, Fcons (where, current_buffer));
/* If SYMBOL is a per-buffer variable which doesn't have a
buffer-local value here, make the `let' change the global
***************
*** 3419,3479 ****
binding. WHERE nil means that the variable had the default
value when it was bound. CURRENT-BUFFER is the buffer that
was current when the variable was bound. */
! /* else if (CONSP (this_binding.symbol)) */
! /* { */
! /* Lisp_Object symbol, where; */
!
! /* symbol = XCAR (this_binding.symbol); */
! /* where = XCAR (XCDR (this_binding.symbol)); */
!
! /* if (NILP (where)) */
! /* Fset_default (symbol, this_binding.old_value); */
! /* else if (BUFFERP (where)) */
! /* set_internal (symbol, this_binding.old_value, XBUFFER (where), 1); */
! /* else */
! /* set_internal (symbol, this_binding.old_value, NULL, 1); */
! /* } */
! /* else */
! /* { */
! /* /\* If variable has a trivial value (no forwarding), we can */
! /* just set it. No need to check for constant symbols here, */
! /* since that was already done by specbind. *\/ */
! /* if (!MISCP (SYMBOL_VALUE (this_binding.symbol))) */
! /* SET_SYMBOL_VALUE (this_binding.symbol, this_binding.old_value); */
! /* else */
! /* set_internal (this_binding.symbol, this_binding.old_value, 0, 1); */
! /* } */
! else {
! /* Fix-me: Before restoring anything the buffer/frame
! localness must be checked. I am not sure how to do that,
! but see below for my attempts.
! */
! /* Set global value */
if (!MISCP (SYMBOL_VALUE (this_binding.symbol)))
SET_SYMBOL_VALUE (this_binding.symbol, this_binding.old_value);
else
! set_default (this_binding.symbol, this_binding.old_value, 0, 1);
!
! {
! register Lisp_Object valcontents = SYMBOL_VALUE (symbol);
! int is_local_now = BUFFER_LOCAL_VALUEP (valcontents) || BUFFER_OBJFWDP (valcontents);
! int is_buffer_local_now = is_local_now && Flocal_variable_p (this_binding.symbol, Qnil);
! int is_frame_local_now = is_local_now && !is_buffer_local_now;
! int was_buffer_local_before = this_binding.old_buffer /= Qunbound;
! int was_local_before = (was_buffer_local_before ||
! this_binding.old_frame /= Qunbound);
! int was_frame_local_before = (was_local_before && ! was_buffer_local_before);
! /* Fix me: restore localness */
! if (was_buffer_local_before_specbind) {
! if (!is_buffer_local_now) Fmake_local_variable (this_binding.symbol);
! /* Set buffer local value */
! set_internal (symbol, this_binding.old_buffer_value, XBUFFER (this_binding.old_buffer), 1);
! } else if (was_frame_local_before_specbind) {
! if (!is_frame_local_now) Fmake_variable_frame_local (this_binding.symbol);
! /* Set frame local value */
! set_internal (symbol, this_binding.old_frame_value, NULL, 1);
! }
! }
}
}
--- 3405,3433 ----
binding. WHERE nil means that the variable had the default
value when it was bound. CURRENT-BUFFER is the buffer that
was current when the variable was bound. */
! else if (CONSP (this_binding.symbol))
! {
! Lisp_Object symbol, where;
!
! symbol = XCAR (this_binding.symbol);
! where = XCAR (XCDR (this_binding.symbol));
!
! if (NILP (where))
! Fset_default (symbol, this_binding.old_value);
! else if (BUFFERP (where))
! set_internal (symbol, this_binding.old_value, XBUFFER (where), 1);
! else
! set_internal (symbol, this_binding.old_value, NULL, 1);
! }
! else
! {
! /* If variable has a trivial value (no forwarding), we can
! just set it. No need to check for constant symbols here,
! since that was already done by specbind. */
if (!MISCP (SYMBOL_VALUE (this_binding.symbol)))
SET_SYMBOL_VALUE (this_binding.symbol, this_binding.old_value);
else
! set_internal (this_binding.symbol, this_binding.old_value, 0, 1);
}
}
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#3467: 23.0.94; let + make-local-variable => let value made global
2009-06-06 20:05 ` bug#3467: 23.0.94; let + make-local-variable => let value made global Richard Stallman
@ 2009-06-06 20:13 ` Lennart Borgman
0 siblings, 0 replies; 13+ messages in thread
From: Lennart Borgman @ 2009-06-06 20:13 UTC (permalink / raw
To: rms; +Cc: emacs-pretest-bug, 3467
On Sat, Jun 6, 2009 at 10:05 PM, Richard Stallman<rms@gnu.org> wrote:
> I think this is a use case that should be avoided.
> Maybe it can be made to work "right", but if that's difficult,
> I'd rather tell people not to do it.
I can't see how it can be avoided. Whenever make-local-variable or
kill-local-variable this problem can occur (if the variable happen to
be let bound of course).
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#3467: 23.0.94; let + make-local-variable => let value made global
2009-06-06 20:11 ` Lennart Borgman
@ 2009-06-07 1:11 ` Lennart Borgman
2009-06-12 22:24 ` Lennart Borgman
0 siblings, 1 reply; 13+ messages in thread
From: Lennart Borgman @ 2009-06-07 1:11 UTC (permalink / raw
To: Stefan Monnier; +Cc: 3467
Rethinking. Everything is wrong in my solution and suggestion. Coming back.
On Sat, Jun 6, 2009 at 10:11 PM, Lennart
Borgman<lennart.borgman@gmail.com> wrote:
> On Sat, Jun 6, 2009 at 12:48 AM, Lennart
> Borgman<lennart.borgman@gmail.com> wrote:
>>
>> Sigh, and my suggestion was of course unnecessary stupid. What is
>> needed is of course to record values and frame+buffer localness and
>> dito values and reset them. Nothing less than this will ever work
>> correctly, or?
>>
>> And does not this apply to all uses of specbind + unbind_to?
>>
>> Can it be sufficient to just change specbind and unbind_to? Is there
>> anything else that will be affected by changes in the specbind stack?
>> Since info about buffer+frame is alwas needed should specbinding be
>> changed to the below form?
>>
>> struct specbinding
>> {
>> Lisp_Object symbol;
>> Lisp_Object old_value;
>> Lisp_Object old_buffer_value;
>> Lisp_Object old_frame_value;
>> specbinding_func func;
>> Lisp_Object unused; /* Dividing by 16 is faster than by 12 */
>> Lisp_Object unused;
>> Lisp_Object unused;
>> };
>
> Here is a bit modified suggestion and some code where I try to
> implement it. Please notice that I have not tested the code. It is
> very possible that I have misunderstood something so please look at
> the code.
>
> struct specbinding
> {
> Lisp_Object symbol;
> Lisp_Object old_value;
> specbinding_func func;
> // total 8 fields
> Lisp_Object need_test_local;
> Lisp_Object old_buffer;
> Lisp_Object old_buffer_value;
> Lisp_Object old_frame;
> Lisp_Object old_frame_value;
> //Lisp_Object unused; /* Dividing by 16 is faster than by 12 */
> };
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#3467: 23.0.94; let + make-local-variable => let value made global
2009-06-07 1:11 ` Lennart Borgman
@ 2009-06-12 22:24 ` Lennart Borgman
2009-06-27 0:24 ` bug#3467: 23.0.94; let + make-local-variable => let value made Stefan Monnier
0 siblings, 1 reply; 13+ messages in thread
From: Lennart Borgman @ 2009-06-12 22:24 UTC (permalink / raw
To: Stefan Monnier; +Cc: 3467
[-- Attachment #1: Type: text/plain, Size: 1569 bytes --]
On Sun, Jun 7, 2009 at 3:11 AM, Lennart
Borgman<lennart.borgman@gmail.com> wrote:
> Rethinking. Everything is wrong in my solution and suggestion. Coming back.
Ok, here is what I believe is a reasonable solution.
The first problem is that the semantics of let,
make-variable-buffer-local etc are not clearly defined. I suggest the
following semantics (instead of the one I proposed before):
- When (let ...) shadows a variable it is exactly this variable (ie
default or buffer local version) that should be revived after (let
...).
- No buffer local variables should be deleted or created by the
reviving of the shadowed variables after the (let ...) clause.
- More exactly: if the variable is buffer local in the current buffer
before (let ..) it is the buffer local variable in that buffer that
should be revived. If it does not exist after (let..) nothing should
be done.
- And if a shadowed variable have been made buffer local during the
let binding then it is still the default value that should be revived.
The buffer local value should in this case not be changed after the
(let ..) clause.
I think with this semantics everything will be nearly as fast as
before and there will be no such surprises as shown in this bug. What
do others think?
I have made a patch for this with some comments and questions. I have
included this in my patched version of Emacs+EmacsW32 and have been
using it myself for a couple of days without any problems.
However please review this carefully and especially what I have called
/* Case 1 in specbind */ in unbind_to.
[-- Attachment #2: eval-bug3467.diff --]
[-- Type: application/octet-stream, Size: 5510 bytes --]
Index: eval.c
===================================================================
RCS file: /sources/emacs/emacs/src/eval.c,v
retrieving revision 1.310
diff -u -b -r1.310 eval.c
--- eval.c 24 Mar 2009 16:37:25 -0000 1.310
+++ eval.c 12 Jun 2009 21:25:13 -0000
@@ -31,6 +31,8 @@
#include "xterm.h"
#endif
+#include <stdio.h> /* For fprintf. */
+
/* This definition is duplicated in alloc.c and keyboard.c */
/* Putting it in lisp.h makes cc bomb out! */
@@ -3295,6 +3297,7 @@
valcontents = SYMBOL_VALUE (symbol);
if (!MISCP (valcontents) && !SYMBOL_CONSTANT_P (symbol))
{
+ /* Case 1 */
specpdl_ptr->symbol = symbol;
specpdl_ptr->old_value = valcontents;
specpdl_ptr->func = NULL;
@@ -3304,7 +3307,7 @@
else
{
Lisp_Object ovalue = find_symbol_value (symbol);
- specpdl_ptr->func = 0;
+ specpdl_ptr->func = 0; /* Fix-me: Are NULL === 0? */
specpdl_ptr->old_value = ovalue;
valcontents = XSYMBOL (symbol)->value;
@@ -3319,11 +3322,14 @@
/* For a local variable, record both the symbol and which
buffer's or frame's value we are saving. */
if (!NILP (Flocal_variable_p (symbol, Qnil)))
+ /* Case 2 */
where = current_buffer;
else if (BUFFER_LOCAL_VALUEP (valcontents)
&& XBUFFER_LOCAL_VALUE (valcontents)->found_for_frame)
+ /* Case 3 */
where = XBUFFER_LOCAL_VALUE (valcontents)->frame;
else
+ /* Case 4 */
where = Qnil;
/* We're not using the `unused' slot in the specbinding
@@ -3397,6 +3403,7 @@
this_binding = *--specpdl_ptr;
if (this_binding.func != 0)
+ /* From record_unwind_protect */
(*this_binding.func) (this_binding.old_value);
/* If the symbol is a list, it is really (SYMBOL WHERE
. CURRENT-BUFFER) where WHERE is either nil, a buffer, or a
@@ -3413,21 +3420,72 @@
where = XCAR (XCDR (this_binding.symbol));
if (NILP (where))
+ {
+ /* Case 4 in specbind */
+ /* We should reset default value. (Note: The default
+ value is either let bound or global.) */
Fset_default (symbol, this_binding.old_value);
+ //printf ("unbind_to: Fset_default where=nil %s\n", SDATA (SYMBOL_NAME (symbol)));
+ }
else if (BUFFERP (where))
+ {
+ /* Case 2 in specbind */
+ /* We should reset buffer local binding, but only if it
+ is buffer local now or will be when set. */
+ if (!NILP (Flocal_variable_p (symbol, where)) ||
+ !NILP (Flocal_variable_if_set_p (symbol, where)))
+ {
set_internal (symbol, this_binding.old_value, XBUFFER (where), 1);
+ //printf ("unbind_to: set_internal in buffer %s\n", SDATA (SYMBOL_NAME (symbol)));
+ }
+ }
else
+ /* Case 3 in specbind */
+ /* Fix-me: This is the frame local case. I believe it
+ is broken, but I do not know if it is worth fixing it
+ since frame local variable are obsolete, or?
+
+ Note: I do not understand NULL buf arg here, but I
+ assume that NULL === 0 here because set_internal
+ checks for 0 and does things like buf->value. */
set_internal (symbol, this_binding.old_value, NULL, 1);
}
else
{
+ /* Case 1 in specbind */
/* If variable has a trivial value (no forwarding), we can
just set it. No need to check for constant symbols here,
since that was already done by specbind. */
if (!MISCP (SYMBOL_VALUE (this_binding.symbol)))
+ {
+ /* Fix-me: I do not understand this case. I assume that
+ if we are here the variable had no buffer local
+ binding in specbind so that we should set the default
+ value. Can SET_SYMBOL_VALUE be used even if the
+ variable now have a buffer local value? Doesn't it
+ erase a buffer local binding that has been done after
+ specbind? Can Fset_default be used in that case?
+ How can we test if make-variable-buffer-local has
+ been done? */
SET_SYMBOL_VALUE (this_binding.symbol, this_binding.old_value);
+ //printf ("unbind_to: SET_SYMBOL_VALUE %s\n", SDATA (SYMBOL_NAME (this_binding.symbol)));
+ }
else
+ /* Fix-me: I do not understand why set_internal were used
+ here. Could the variable have had a buffer local
+ binding in specbind if we are here? Should not just
+ Fset_default be called here? */
+ if (0) //(NILP (Flocal_variable_p (this_binding.symbol, Qnil)))
+ {
set_internal (this_binding.symbol, this_binding.old_value, 0, 1);
+ //printf ("unbind_to: set_internal 0 %s\n", SDATA (SYMBOL_NAME (this_binding.symbol)));
+ }
+ else
+ {
+ /* Set the default value since we had no buffer local binding before. */
+ Fset_default (this_binding.symbol, this_binding.old_value);
+ //printf ("unbind_to: Fset_default no need %s\n", SDATA (SYMBOL_NAME (this_binding.symbol)));
+ }
}
}
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#3467: 23.0.94; let + make-local-variable => let value made
2009-06-12 22:24 ` Lennart Borgman
@ 2009-06-27 0:24 ` Stefan Monnier
0 siblings, 0 replies; 13+ messages in thread
From: Stefan Monnier @ 2009-06-27 0:24 UTC (permalink / raw
To: Lennart Borgman; +Cc: 3467
> The first problem is that the semantics of let,
> make-variable-buffer-local etc are not clearly defined. I suggest the
> following semantics (instead of the one I proposed before):
That's the intuitive semantics, indeed.
> I have made a patch for this with some comments and questions. I have
> included this in my patched version of Emacs+EmacsW32 and have been
> using it myself for a couple of days without any problems.
- NULL is equal to 0 by definition.
- Why do you check
if (!NILP (Flocal_variable_p (symbol, where)) ||
!NILP (Flocal_variable_if_set_p (symbol, where)))
i.s.o.
if (!NILP (Flocal_variable_p (symbol, where)))
That looks wrong since it would undo a kill-local-variable that was
performed within the let.
- Regarding "Case 1 in specbind", I have (in my local changes)
completely rewritten the variable-binding code so as not to use
special MISCP values. I find it makes the code clearer, tho maybe
others will disagree. I intend to install it in the trunk for 23.2
and it of course conflicts completely with your patch ;-)
If you want, I can try and extract the corresponding patch and send it
to you.
Stefan
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2009-06-27 0:24 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-06-04 21:14 bug#3467: 23.0.94; let + make-local-variable => let value made global Lennart Borgman
2009-06-05 14:21 ` Lennart Borgman
2009-06-05 14:36 ` Stefan Monnier
2009-06-05 15:04 ` Lennart Borgman
2009-06-05 22:48 ` Lennart Borgman
2009-06-05 23:08 ` Lennart Borgman
2009-06-05 23:32 ` Lennart Borgman
2009-06-06 20:11 ` Lennart Borgman
2009-06-07 1:11 ` Lennart Borgman
2009-06-12 22:24 ` Lennart Borgman
2009-06-27 0:24 ` bug#3467: 23.0.94; let + make-local-variable => let value made Stefan Monnier
2009-06-06 20:05 ` bug#3467: 23.0.94; let + make-local-variable => let value made global Richard Stallman
2009-06-06 20:13 ` Lennart Borgman
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.