Re: `save-excursion' defeated by `set-buffer'

[Tim X <timx@nospam.dev.null>]

Andreas Röhler <andreas.roehler@easy-emacs.de> writes:

> Am 12.03.2011 09:59, schrieb Eli Zaretskii:
>>> From: Stefan Monnier<monnier@iro.umontreal.ca>
>>> Date: Fri, 11 Mar 2011 10:52:14 -0500
>>>
>>> So (save-excursion (goto-char BAR)) is pretty much a no-op.
>>>
>>> But (save-excursion (set-buffer FOO) (goto-char BAR)) is either:
>>> - the same as (save-excursion (goto-char BAR)), in case FOO is already current.
>>> - an inefficient form of (with-current-buffer FOO (goto-char BAR)).
>>> I still haven't found any code out there where this behavior is what
>>> is actually wanted and expected by the programmer.
>>>
>>> In more than 90% of the cases, the intended meaning is
>>> (with-current-buffer FOO (goto-char BAR)) and the behavior if FOO is
>>> current (to additionally preserve point) is harmless, so the warning
>>> simply points out an inefficiency.
>>>
>>> In the remaining cases, FOO is almost always current, but when it's not
>>> the resulting behavior is a bug.  I.e. the intended code is
>>> (with-current-buffer FOO (save-excursion (goto-char BAR))).
>>
>> Then how about changing the text of the warning to something like
>>
>>    Warning: `save-excursion' will not preserve point in the other buffer
>>    set by `set-buffer'
>>
>> ?
>>
>>
>
>
>
> Seems all what's neccessary to know about save-excursion already is expressed
> in it's docstring.
>

Not sure thats true. I've seen a lot of elisp code that does the
save-excursion, set buffer pattern, which makes me think maybe the
documentation is inadequate. 

> If people don't want the buffer restored alongside with point and mark, they
> should not use this form.
>

I think it is a little more subtle than that. People may be using it
under the expectation that point and mark will also be saved in the
buffer accessed by set-buffer. 

> As I used it, compiler may assume I wanted that. No reason for a warning if
> used as provided.
>

Except we know it is often used incorrectly. I think Uday's point is
very valid here. When writing code, part of the aim is to communicate
intentions to others who may need to maintain or update the code in the
future. As Uday pointed out, using save-excursion followed by set-buffer
muddies the intention and decreases clarity. 

> OTOH we need compiler warnings for serious things.
>
> So my suggestion is simply to drop that warning.
>

I would vote to keep the warning, but modify to be 'possibly defeated'
and add a reference to the manual where the issue is clarified. 

If you have a situation where the pattern is legit, the warning can be
turned off. 

Tim

-- 
tcross (at) rapttech dot com dot au