save-excursion again

save-excursion again

[Uday S Reddy]

I am just catching up belatedly with this issue of save-excursion getting 
defeated by set-buffer.  Here is my understanding.  Please let me know if I am 
missing anything.  If I have a piece of code like this that runs in a buffer A:

      (save-excursion
         (set-buffer B)
         ....X....
      )

then:

- if the code X has no possibility of getting back to the buffer A and moving 
point, then save-excursion can be replaced by save-current-buffer (and the byte 
compiler gives you a brownie point).

- if the code X has a possibility of getting back to the buffer A and moving 
around, then save-excursion should stay (despite getting smacked by the byte 
compiler).

So, every time we want to please the byte compiler, we need to prove a little 
theorem to the effect that the code X doesn't enter the buffer A?  (No doubt 
some of these theorems will be obvious.)

Cheers,
Uday

Re: save-excursion again

[Stefan Monnier]

> I am just catching up belatedly with this issue of save-excursion getting
> defeated by set-buffer.  Here is my understanding.  Please let me know if
> I am missing anything.  If I have a piece of code like this that runs
> in a buffer A:

>      (save-excursion
>         (set-buffer B)
>         ....X....
>      )

> then:

> - if the code X has no possibility of getting back to the buffer A and
> moving point, then save-excursion can be replaced by save-current-buffer
> (and the byte compiler gives you a brownie point).

Yes.

> - if the code X has a possibility of getting back to the buffer A and moving
> around, then save-excursion should stay (despite getting smacked by the byte
> compiler).

It's actually slightly more subtle than that, because it depends on what
you want to happen when you "get back to A and move around": it depends
on whether or not you want the movement in A to be undone when you exit
this code.

If you do want to undo this movement, then indeed your `save-excursion'
and your `set-buffer' are fundamentally unrelated (they just happen to
be next to each other by chance) and the byte-compiler warning is in
error.  This is very rare in my experience (contrary to the case where
the correct code is (with-current-buffer B (save-excursion ....)), which
tho not common, has happened a few times).

> So, every time we want to please the byte compiler, we need to
> prove a little theorem to the effect that the code X doesn't enter the
> buffer A?  (No doubt some of these theorems will be obvious.)

Indeed.  The problem being that the precise semantics of such
a construction is pretty subtle, so the byte-compiler can't do the proof
for you.  In some cases the proof is easy (e.g. there's no movement at
all in that piece of code), but often it's very difficult (tho my
experience might be made worse since I usually make such changes to code
with which I'm not familiar).  My approach is to basically replace all
such code with just `with-current-buffer', then let users find the
counter examples.


        Stefan

Re: save-excursion again

[David Kastrup]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> So, every time we want to please the byte compiler, we need to prove
>> a little theorem to the effect that the code X doesn't enter the
>> buffer A?  (No doubt some of these theorems will be obvious.)
>
> Indeed.  The problem being that the precise semantics of such a
> construction is pretty subtle, so the byte-compiler can't do the proof
> for you.  In some cases the proof is easy (e.g. there's no movement at
> all in that piece of code), but often it's very difficult (tho my
> experience might be made worse since I usually make such changes to
> code with which I'm not familiar).  My approach is to basically
> replace all such code with just `with-current-buffer', then let users
> find the counter examples.

I don't see how this process is supposed to terminate if users find
counterexamples.  Presumably the original code is then reinstated, and
the whole procedure starts from the beginning.  It would not appear that
there is a way to _intentionally_ use that construct without you
eventually replacing it.

-- 
David Kastrup

Re: save-excursion again

[Davis Herring]

> - if the code X has no possibility of getting back to the buffer A and
> moving point, then save-excursion can be replaced by
> save-current-buffer (and the byte compiler gives you a brownie point).

Be careful: you might not "get back" to buffer A and yet be in it if (eq A
B).

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.

Re: save-excursion again

[Stefan Monnier]

> I don't see how this process is supposed to terminate if users find
> counterexamples.  Presumably the original code is then reinstated, and
> the whole procedure starts from the beginning.

No, when we find a bug we replace the code with something *else*.
Typically we replace (with-current-buffer B ...) by
(with-current-buffer B (save-excursion ...)), or is some other case we
have to replace it with (save-excursion B (with-current-buffer ...)).

> It would not appear that there is a way to _intentionally_ use that
> construct without you eventually replacing it.

That's right.


        Stefan

Re: save-excursion again

[Uday S Reddy]

Stefan Monnier writes:

> It's actually slightly more subtle than that, because it depends on what
> you want to happen when you "get back to A and move around": it depends
> on whether or not you want the movement in A to be undone when you exit
> this code.
> 
> If you do want to undo this movement, then indeed your `save-excursion'
> and your `set-buffer' are fundamentally unrelated (they just happen to
> be next to each other by chance) and the byte-compiler warning is in
> error.  This is very rare in my experience (contrary to the case where
> the correct code is (with-current-buffer B (save-excursion ....)), which
> tho not common, has happened a few times).

On the other hand, it seems to be very common in my experience.

In VM, the code is generally playing around with multiple buffers:
there is a Folder buffer (where the mail is stored), a Presentation
buffer (where it is displayed), a Summary buffer, and possibly one or
more process buffers (for talking to mail servers) and tons of
temporary buffers that are used for MIME encoding/decoding and other
processing.  Whenever, we switch to a different buffer, we almost
always want to come back to the old place, as if nothing had happened.
Further more, the code is modular.  It doesn't really know where it is
running, only what it needs to do.  So, the save-excursion/set-buffer
pattern is not only the correct thing to do, but also the *safest*
thing to do.  Changing it can break the code in quite subtle ways
which can be difficult to track down.

When I first got involved with VM, there were a host of problems that
were called "jumping cursor" issues.  You do something and the cursor
ends up at a different place.  If you then delete a message, you end
up deleting the wrong message.  Fixing these problems always involved
adding plenty of save-excursion's so that the cursor doesn't move
while work was going on.

In fact, I would have rather liked a compiler that asked, do you
really want to do set-buffer without a save-excursion first?  That is,
set-buffer without save-excursion is dangerous, because it might lead
to the "jumping cursor" problem.

So, I am a bit surprised that anybody should think of save-excursion
as being a bad idea to be avoided.  I am still trying to understand
this.

> Indeed.  The problem being that the precise semantics of such
> a construction is pretty subtle, so the byte-compiler can't do the proof
> for you.  In some cases the proof is easy (e.g. there's no movement at
> all in that piece of code), but often it's very difficult (tho my
> experience might be made worse since I usually make such changes to code
> with which I'm not familiar).  My approach is to basically replace all
> such code with just `with-current-buffer', then let users find the
> counter examples.

I will assume you are joking.  But, why bother with any of this at
all?  What is wrong with the original code in the first place? 

I guess this might have been discussed in the previous rounds of
discussions, but I really couldn't find the answer.

Cheers,
Uday

Re: save-excursion again

[Stefan Monnier]

> When I first got involved with VM, there were a host of problems that
> were called "jumping cursor" issues.  You do something and the cursor
> ends up at a different place.  If you then delete a message, you end
> up deleting the wrong message.  Fixing these problems always involved
> adding plenty of save-excursion's so that the cursor doesn't move
> while work was going on.

Yes, that's what save-excursion is for.  But placing it right before
`set-buffer' is usually not the best answer.

> In fact, I would have rather liked a compiler that asked, do you
> really want to do set-buffer without a save-excursion first?  That is,
> set-buffer without save-excursion is dangerous, because it might lead
> to the "jumping cursor" problem.

Most of the "nasty jumping cursors" I've seen were due to code like:

  (save-excursion
    (set-buffer FOO)
    ...move around...)

Which should instead be written

  (with-current-buffer FOO
    (save-excursion
      ...move around...))

I.e. the save-excursion needs to be around the point-movement, not
around the buffer-change.

> So, I am a bit surprised that anybody should think of save-excursion
> as being a bad idea to be avoided.

It's not.

> I am still trying to understand this.
>> Indeed.  The problem being that the precise semantics of such
>> a construction is pretty subtle, so the byte-compiler can't do the proof
>> for you.  In some cases the proof is easy (e.g. there's no movement at
>> all in that piece of code), but often it's very difficult (tho my
>> experience might be made worse since I usually make such changes to code
>> with which I'm not familiar).  My approach is to basically replace all
>> such code with just `with-current-buffer', then let users find the
>> counter examples.
> I will assume you are joking.  But, why bother with any of this at
> all?  What is wrong with the original code in the first place? 

That the save-excursion in

  (save-excursion
    (set-buffer FOO)
    ...move around...)

will not undo the point movement in FOO (unless current-buffer is
already FOO to start with).


        Stefan

Re: save-excursion again

[Stephen J. Turnbull]

Uday S Reddy writes:

 > When I first got involved with VM, there were a host of problems that
 > were called "jumping cursor" issues.  You do something and the cursor
 > ends up at a different place.

The point is that if

    ;; current buffer is "bar"
    (save-excursion
     (set-buffer "foo")
     (frob-current-buffer))

protects "bar" from jumping cursor, then `frob-current-buffer' must
switch back to "bar" and move the cursor at some point.

If your code is doing that kind of thing, then you end up with

 > In fact, I would have rather liked a compiler that asked, do you
 > really want to do set-buffer without a save-excursion first?

But that's *not good enough*.  Suppose a new feature requires this
code:

    ;; current buffer is "bar"
    (frob-current-buffer)

Now you're going to see jumping cursor without a set-buffer to warn
you.  The obvious try is

    ;; current buffer is "bar"
    (save-excursion (frob-current-buffer))

but since `frob-current-buffer' is used in situations where jumping
cursor is bad, why not

(fset 'frob-current-buffer-internal (symbol-function 'frob-current-buffer))
(defun frob-current-buffer (&rest args)
  (save-excursion (apply #'frob-current-buffer-internal args)))

and save yourself future aggravation by using `frob-current-buffer'
except when point motion is the intent?

 > That is, set-buffer without save-excursion is dangerous, because it
 > might lead to the "jumping cursor" problem.

No.  It's the code *after* the set-buffer that's dangerous, not the
set-buffer.  That fact that you consider set-buffer per se dangerous
is a strong suggestion that your program is rotten with code that does
set-buffer and then moves point without a save-excursion.

 > I will assume you are joking.  But, why bother with any of this at
 > all?  What is wrong with the original code in the first place? 

Nothing, assuming that the whole project is already using functions
that have nasty side-effects of moving point in buffers they aren't
called from.  In that case it might be cheaper for you to wrap every
set-buffer and following code in a save-excursion, but it's not 100%
reliable.  And it's *much* better to write new code to avoid side
effects except in functions designed to produce side effects.

It's true that using the set-buffer heuristic you'll probably catch
*most* such issues, because a lot of the functions of the form

(defun find-something (quux)
  (set-buffer "bar")
  ;; I should have put a save-excursion *here* but didn't.  Oops.
  (if (search-forward quux nil t)
      (extract-info-from-line-in-bar)
    (error "No quux here, boss!")))

are mostly called from other buffers (in a context like VM, if you're
in "bar", you're probably already on the appropriate line, so you call
`extract-info-from-line-in-bar' directly, not via `find-something').

But you can't guarantee that.

Re: save-excursion again

[Lennart Borgman]

On Sat, Jun 19, 2010 at 4:50 PM, Stephen J. Turnbull
<turnbull@sk.tsukuba.ac.jp> wrote:
> Uday S Reddy writes:
>
> The point is that if
>
>    ;; current buffer is "bar"
>    (save-excursion
>     (set-buffer "foo")
>     (frob-current-buffer))


Just a simple question from someone that does not understand:

What is wrong with just saving the point in the buffer you are
interested and then returning to that, i.e.

  (let ((here (point)) ;; maybe (point-marker) sometimes
    ...
    (prog1

Re: save-excursion again

[Lennart Borgman]

On Sat, Jun 19, 2010 at 4:56 PM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
> On Sat, Jun 19, 2010 at 4:50 PM, Stephen J. Turnbull
> <turnbull@sk.tsukuba.ac.jp> wrote:
>> Uday S Reddy writes:
>>
>> The point is that if
>>
>>    ;; current buffer is "bar"
>>    (save-excursion
>>     (set-buffer "foo")
>>     (frob-current-buffer))
>
>
> Just a simple question from someone that does not understand:
>
> What is wrong with just saving the point in the buffer you are
> interested and then returning to that, i.e.
>
>  (let ((here (point)) ;; maybe (point-marker) sometimes
>    ...
>    (prog1

Eh...

(let ((here (point)))
   ...
   (prog1
        return_value
     (goto-char here)))

Re: save-excursion again

[Uday S Reddy]

Dear Stefan and Stephen,

Yes, I do understand what you are saying.  Thanks for the detailed explanations.

There are 176 instances of the save-excursion/set-buffer pattern in VM.  I 
would guess that the majority of these save-excursion's can be converted to 
save-current-buffer's.  But it is going to be difficult to figure out which 
ones.  It is possible that at least some of the developers were confused about 
the issue as you describe.

But you have definitely managed to convince me that I need to look at these 
instances and see what is going on.

Cheers,
Uday

Re: save-excursion again

[Stephen J. Turnbull]

Lennart Borgman writes:

 > > What is wrong with just saving the point in the buffer you are
 > > interested and then returning to that, i.e.
 > 
 > (let ((here (point)))
 >    ...
 >    (prog1
 >         return_value
 >      (goto-char here)))

No protection against abnormal exits or changes to the mark.  The
more important issue is abnormal exits, eg, if a search fails, since
code shouldn't mess with the mark anyway.

Which brings up a question for Stefan.  Are you saying that the
condition-case-save-point-and-mark aspect of `save-excursion' should
be abstracted out (say as `save-region') and separated from the
save-buffer aspect?

Re: save-excursion again

[Lennart Borgman]

On Sat, Jun 19, 2010 at 7:23 PM, Stephen J. Turnbull <stephen@xemacs.org> wrote:
> Lennart Borgman writes:
>
>  > > What is wrong with just saving the point in the buffer you are
>  > > interested and then returning to that, i.e.
>  >
>  > (let ((here (point)))
>  >    ...
>  >    (prog1
>  >         return_value
>  >      (goto-char here)))
>
> No protection against abnormal exits or changes to the mark.

I see, thanks.

> Which brings up a question for Stefan.  Are you saying that the
> condition-case-save-point-and-mark aspect of `save-excursion' should
> be abstracted out (say as `save-region') and separated from the
> save-buffer aspect?

I really think that would be useful.

Re: save-excursion again

[Stefan Monnier]

> Which brings up a question for Stefan.  Are you saying that the
> condition-case-save-point-and-mark aspect of `save-excursion' should
> be abstracted out (say as `save-region') and separated from the
> save-buffer aspect?

Not sure what you mean.  If you mean we should introduce
a save-excursion-but-not-buffer, then I think it's a bad idea, because
such a thing would still have to save a marker (so there's no
improvement in terms of the amount of data saved) and the semantics of
"restore point in another buffer" is murky in Emacs.


        Stefan

Re: save-excursion again

[Stephen J. Turnbull]

Stefan Monnier writes:
 > > Which brings up a question for Stefan.  Are you saying that the
 > > condition-case-save-point-and-mark aspect of `save-excursion' should
 > > be abstracted out (say as `save-region') and separated from the
 > > save-buffer aspect?
 > 
 > Not sure what you mean.

Well, ditto.  I'm really unclear on why with-current-buffer is always
an appropriate substitute for (save-excursion (set-buffer fu-boofer)
...), for the same kinds of reasons I'm kinda unclear on why global
match data is ever a good idea.  Not everybody programs in functional
style; you may have cause to call code which has nasty side effects on
the buffer you're explicitly working on.

 > the semantics of "restore point in another buffer" is murky in
 > Emacs.

I don't really see why.  It would be the same as now except that you
don't do a final set-buffer.  Whether that would be useful or not is
another question.  Evidently you don't see it as useful, though, which
answers my question.

Re: save-excursion again

[Uday S Reddy]

On 6/25/2010 10:20 PM, Stefan Monnier wrote:
>> Which brings up a question for Stefan.  Are you saying that the
>> condition-case-save-point-and-mark aspect of `save-excursion' should
>> be abstracted out (say as `save-region') and separated from the
>> save-buffer aspect?
>
> Not sure what you mean.  If you mean we should introduce
> a save-excursion-but-not-buffer, then I think it's a bad idea, because
> such a thing would still have to save a marker (so there's no
> improvement in terms of the amount of data saved) and the semantics of
> "restore point in another buffer" is murky in Emacs.

I think the point is that save-excursion has been saving too much.  So, when we 
see a save-excursion in a piece of code, we are not sure whether it was meant 
to save the point and mark, or the current-buffer, or both.  It seems that the 
original developers were themselves unsure after a while.

I took Stephen's point as saying that, if we have separate save-current-buffer 
and save-region, with save-excursion strongly discouraged, we would promote a 
clearer programming style.

Cheers,
Uday

Re: save-excursion again

[Stefan Monnier]

> Well, ditto.  I'm really unclear on why with-current-buffer is always
> an appropriate substitute for (save-excursion (set-buffer fu-boofer)
> ...),

That's an easy one: it's *not* always an appropriate substitute.


        Stefan

Re: save-excursion again

[Stefan Monnier]

> I think the point is that save-excursion has been saving too much.  So, when
> we see a save-excursion in a piece of code, we are not sure whether it was
> meant to save the point and mark, or the current-buffer, or both.  It seems
> that the original developers were themselves unsure after a while.

Yes, it's part of the tension between "DWIMish user commands" and
"simple&predictable Lisp functions", where save-excursion has a kind of
"DWIMish user-command" feeling to it (it does various things and you can
keep using that single form for various purposes).

> I took Stephen's point as saying that, if we have separate
> save-current-buffer and save-region, with save-excursion strongly
> discouraged, we would promote a clearer programming style.

Probably.  But then, for the same reasons, you'd also want to have
save-point since save-region is so rarely needed.


        Stefan

Re: save-excursion again

[Lennart Borgman]

On Thu, Jul 1, 2010 at 2:30 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
>> I took Stephen's point as saying that, if we have separate
>> save-current-buffer and save-region, with save-excursion strongly
>> discouraged, we would promote a clearer programming style.
>
> Probably.  But then, for the same reasons, you'd also want to have
> save-point since save-region is so rarely needed.

So what is wrong with having those?

Re: save-excursion again

[Stephen J. Turnbull]

Stefan Monnier writes:
 > > Well, ditto.  I'm really unclear on why with-current-buffer is always
 > > an appropriate substitute for (save-excursion (set-buffer fu-boofer)
 > > ...),
 > 
 > That's an easy one: it's *not* always an appropriate substitute.

Stefan's in a frisky mood; *don't* tag anything stable today!

Re: save-excursion again

[Stefan Monnier]

>> > Well, ditto.  I'm really unclear on why with-current-buffer is always
>> > an appropriate substitute for (save-excursion (set-buffer fu-boofer)
>> > ...),
>> That's an easy one: it's *not* always an appropriate substitute.
> Stefan's in a frisky mood; *don't* tag anything stable today!

No worry: I'm on the road, with very spotty internet access, and not
much less spotty time to devote to Emacs.


        Stefan "away until October"

Re: save-excursion again

[Stefan Monnier]

>>> I took Stephen's point as saying that, if we have separate
>>> save-current-buffer and save-region, with save-excursion strongly
>>> discouraged, we would promote a clearer programming style.
>> Probably.  But then, for the same reasons, you'd also want to have
>> save-point since save-region is so rarely needed.
> So what is wrong with having those?

Nothing in itself.  Tho, I don't see a particular need for it either.


        Stefan