save-excursion and save-current-buffer - edits to the manual

save-excursion and save-current-buffer - edits to the manual

[Uday S Reddy]

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 378 bytes --]

The debate on save-excursion getting "defeated" flared up again on
gnu.emacs.help.  Somebody noticed that the Lisp manual hasn't been
updated properly to take account of the new compiler warning.

The attached patch/bundle is an attempt to explain the situation as
well as to provide guidance on how to use these forms correctly.
Please let me have any comments.

Cheers,
Uday


[-- Attachment #2: save-excursion and save-current-buffer - edits to the manual --]
[-- Type: text/plain, Size: 7482 bytes --]

# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: u.s.reddy@cs.bham.ac.uk-20110313160841-rowr3yt2r3iv5s5x
# target_branch: file:///D:/gnu/emacs-23-dev/
# testament_sha1: 2bb9efa1bb4c2102b3bb3a153847419b08046134
# timestamp: 2011-03-13 16:09:13 +0000
# base_revision_id: lekktu@gmail.com-20110312194442-ujp5i3ojwxqo62uv
# 
# Begin patch
=== modified file 'doc/lispref/buffers.texi'
--- doc/lispref/buffers.texi	2011-01-02 23:50:46 +0000
+++ doc/lispref/buffers.texi	2011-03-13 16:08:41 +0000
@@ -113,8 +113,8 @@
 as well as from the command loop; it is convenient for the caller if
 the subroutine does not change which buffer is current (unless, of
 course, that is the subroutine's purpose).  Therefore, you should
-normally use @code{set-buffer} within a @code{save-current-buffer} or
-@code{save-excursion} (@pxref{Excursions}) form that will restore the
+normally use @code{set-buffer} within a @code{save-current-buffer}
+that will restore the 
 current buffer when your function is done.  Here, as an example, is a
 simplified version of the command @code{append-to-buffer}:
 
@@ -149,8 +149,8 @@
 binding's scope.  Otherwise you might bind it in one buffer and unbind
 it in another!  There are two ways to do this.  In simple cases, you may
 see that nothing ever changes the current buffer within the scope of the
-binding.  Otherwise, use @code{save-current-buffer} or
-@code{save-excursion} to make sure that the buffer current at the
+binding.  Otherwise, use @code{save-current-buffer}
+to make sure that the buffer current at the
 beginning is current again whenever the variable is unbound.
 
   Do not rely on using @code{set-buffer} to change the current buffer
@@ -171,6 +171,36 @@
 Using @code{save-current-buffer}, as we did, handles quitting, errors,
 and @code{throw}, as well as ordinary evaluation.
 
+Sometimes the @code{save-excursion} (@pxref{Excursions}) form is used
+instead of @code{save-current-buffer} to restore the current-buffer.
+@code{save-excursion} restores the current buffer and, in addition,
+restores the point and mark in that buffer as well.  However, it is
+not a recommended use of @code{save-excursion} for reverting buffer
+changes.  If you use it, the byte compiler gives the somewhat
+mysterious warning:
+
+@example
+Warning: @code{save-excursion} defeated by @code{set-buffer}
+@end example
+
+@noindent The reasons for the warning are:
+
+@itemize
+@item 
+The @code{save-excursion} only restores the original buffer's point and
+mark, not in the target of the @code{set-buffer}.
+@item
+If the body of the @code{save-excursion} somehow changes the original
+buffer's point and mark,  then it is best to place
+@code{save-excursion} around the portion of the code that does so.
+@end itemize
+
+@noindent Replacing @code{save-excursion} by
+@code{save-current-buffer} is always a good way to get around the
+compiler warning.  However, before you can do the replacement, you
+will need to ensure that all the changes to the point and mark inside
+the body are properly protected by @code{save-excursion} forms.
+
 @defun current-buffer
 This function returns the current buffer.
 

=== modified file 'doc/lispref/positions.texi'
--- doc/lispref/positions.texi	2011-01-02 23:50:46 +0000
+++ doc/lispref/positions.texi	2011-03-13 16:08:41 +0000
@@ -808,7 +808,7 @@
 described elsewhere (see @ref{Window Configurations}, and @pxref{Frame
 Configurations}).  When only the identity of the current buffer needs
 to be saved and restored, it is preferable to use
-@code{save-current-buffer} instead.
+@code{save-current-buffer} instead.  (see @ref{Current Buffer}).
 
 @defspec save-excursion body@dots{}
 @cindex mark excursion
@@ -867,6 +867,25 @@
 @code{deactivate-mark}, and thus causing the deactivation of the mark
 after the command finishes.  @xref{The Mark}.
 
+  It is recommended practice to place @code{save-excursion} as close
+as possible to the portions of code that change the point or mark.  In
+particular, there should be no calls to @code{set-buffer} in the
+intervening code inside the @code{save-excursion} form before the
+changes to point or mark, because @code{save-excursion} only restores the point
+and mark of the current buffer, not for the other buffers that may be
+the target of @code{set-buffer}.  If a @code{save-excursion} form
+begins with a call to @code{set-buffer}, the byte compiler
+produces the warning:
+
+@example
+Warning: @code{save-excursion} defeated by @code{set-buffer}
+@end example
+
+@noindent You may replace the @code{save-excursion} forms by
+@code{save-current-buffer} forms to get around the warning, but you
+would also need to ensure that the changes to point and mark in the
+target buffers are protected by @code{save-excursion} forms locally.
+
 @node Narrowing
 @section Narrowing
 @cindex narrowing

# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWQDx/gEAAzLfgAAQUOf/93pg
nqC////6YAhnPuV2dcUpN1hHdu+QaG2L205ISiIaT1MFMxBNNPU9TIHqZADQzUNBKIAEaBTySBpt
QAAADQA5gE0wCZDAAEwTAAABIkCp6aTQ9GlP0k9NEZPSaDQDTIDTQRSmknqeUeoPQnpNBpjU0GgA
AABIkE0CaAmk8k9TNKemiHqGQaNNGgjSlLkoqyugc0cz2dVeV48LvepPxVlk4ixCtYGDLVNulxRn
BYHfLxCDerEKKDm54JGSztq9Xsc2ebhLNpyOfWot4zV5hcLZwV1a+Y7jSNtDZxfgNM7ZQdWyJOqi
jlk9hS060bMdvdj3s8OklOZQaZ6Ybm6HGTNSX0lZmm3z9k5vpKJMPZdPVPk3SKXc+jtrL7iOsF9a
4XhcbdnfP+PHStm5LKlTLUpMwIEHUYeH86tCRXKmcYLRataqmr8XxMK3ahYnp1m0Z1JMjQyZa9xp
BMLFRs1Y4iFGEGDlrlLOuEy+9aRfZ1gMCoRmyjMnSZDlJI/EWzXq9hRSzZsGztB7tKspUV5FzqMM
FZF9DoYUexjWRrHG2GWzXYthrNPUWt2INVAoKUfOVZiHRvglTbLbG01hY5qwHDsFckrJZMQOOngL
qB9nP1HFYy2d76b+G7pK26efdwQUQXLgQgaBgeSDkbeWREZYbDPreNwlDlomKY0J4btshkyEijgX
kMj6EiRocgKimaF4WT6RGYZjUiQGF+XN547DEQ90+DPCZFBOoSQkXXGEZQujpeC99nXuSAxEZ4g8
zP1THWcrGIyoHDhoOwtvzdwJgkImRwx0CRtJG+tEBTERTAhDaYWJUNyZMeb2LZ9eRbZmLA9oDWJs
MtVrlRIYk45HKhSeFaGhU1J4GyKBS22chvGAcLcB6IcjAuI1NSqQqPIGk2DppEzFenCUYVqTo6pU
gPMHjFNIb7VaVhXVbYVPqLkv8A9ouoFK7eMtxmB0frFmTMFM3FYODTk8KHTr0VmTKMarTpKUB6XM
aCZb3K8rHJH7GgHV3TWFBaayLWzzSGmhdIurNglxM1M2pYk0qbjApcZFqVMR58+HkVZ0MbsQd+Gc
a25wtklWAddCKqyLrPX6Lf2YaDe1N+I93wOti9MbBNgdSCEb11kC+VFeSGNvukvKfnlJqjSY5QRP
jFnVDVsgCFBgnSaDIOz5OOYuQNVqHbXc8wk7OgnxRIsGk2dN3WUcGKCpr9wUuqjJlhcGGE1glmUq
AfBH1Kqrve7C5ok1Eh5bEC8ZGB9vYT5XMt3bCPcVzPNrR80EdsjgeyETf3jPPzbkhS9BMCgjUg9D
qpkPXPaNZ6HftnHp4VvWTdSUkfJCW9ZG6ZddVBqqSZaq2Ni6gieJ4EBA22i6FCatEsr90i/lKhQH
WaP7t7fAuziK50+vNzjwGrVQlkdSn7bWWLvL7iBUgaDsxN+MpGuYU7Ik6tQ2+BgdQ1YtSzmg7Bjf
ayKDB3R8Y4KBO3qQJwsHMXkRKNwPOc7rdWVKoYvMZxOO3ZxAyRQxWaSC3xIGMgEd5WZyYTbWsWBC
WQmGi04KUtmetu5ht1G/RBlvXCYc5yGCam0iaYsscGXOG2UGrbb69kdaid0qTEqcssvW1bXIO+nJ
1FNFkpbyYo2m4yiGyrPYi7D8xbDNBEwSMhluOMzYNI20wwXDMK0WvOK+5oMJohpITNARRInTEQtI
wlSSoUlI7URx0OQu5BAuc0B9WJDYGbRXDUB5iiZvUjOs40j1/6ihEuaqty7BdVrKrd1aPA96PBcd
/Hbe1owteVdgI/Xn2yvN7GJtg0BusjX9iPaiS1dxDoSuDy6ga4JFu9bFuzSkA4j1c0dF2qRYG+9T
DBEoDmi17ICpyYcU1v+J8TPMZm2RmZa/PJvj2kjQXTbMFVBqJtXUaV7hMoV3l2etzHOZJUooUBcF
pHnZRS6JA0mXk1NNGic0SGEykApi776sVCvJYEHuOYqLBgwsREIaGmME2mw0MZKYB4dA3sXqaXrj
WL5KRxdu0ANxzKlNqDFeoDorLL6OBa1MOqTyVL9bOWNiRBEB3EiTXC1t5smSdoK4d6/zy31WiPEk
B0eSzehjDdNZxqZuUBoJCOLx8W5dX2owjFnXoYC968DUloeLhjFDR8gOIGyiK1K6KYewnmuiCyNW
XYhVSqWBo2PZEKRNTMvOUyUAOEiEumNGEl5AaIVwmCJQXJjTEtvM+F8tdcOviTpOdWB0WhUzWt01
rCDFBN6iir6QM+50dgm+rACut0MoDPVfhskRhNeCNrmsxQxBkFTQOnC9cLI2WZgcc0SE5XNXDRWI
RQZDiCRY9kplCFynFGFqKo02mDvA5CsiwW4ZHwLFKGZwLzJG2mPaBiLbWEs2kNoQ2LB/9PCePYbN
5Eh1bCyC9pZpGYUYU9FUmUZzOXMGv1329jLg+IRIhxtqF+s0w2B/E6rLBbCU8rhAzjeQcji23G6k
JVUO/ExLelRBkKVAN4ves0rC+YwCGB3L3QleXMNcPKM5M4RE25oNQT8/yAsS2IxMcDEhCWIbUhts
BBbAf4u5IpwoSAB4/wCA

RE: save-excursion and save-current-buffer - edits to the manual

[Drew Adams]

> The debate on save-excursion getting "defeated" flared up again on
> gnu.emacs.help.  Somebody noticed that the Lisp manual hasn't been
> updated properly to take account of the new compiler warning.

No, someone simply pointed out that the Lisp manual does not say what you were
trying to make it say.  What it says is correct.  It just hasn't yet been purged
of `save-excursion' the way you would like.  And that's a good thing.

> The attached patch/bundle is an attempt to explain the situation as
> well as to provide guidance on how to use these forms correctly.
> Please let me have any comments.

My comment is to please leave it as it was.

You have replaced mention of using `save-excursion' or `save-current-buffer'
when one uses `set-buffer' with just mention of using `save-current-buffer' with
it.  Both can be useful here.

You've added this:

"However, it is not a recommended use of `save-excursion' for reverting buffer
changes."

The word "it" references nothing here - no meaning.

And `save-excursion' is not about reverting buffer changes - never has been.
And no one would ever think that it is.  And the warning you then try to explain
also has nothing to do with reverting buffer changes.

And your explanation of the warning, like the warning itself, does not help.

And in your "recommended practice" section, you completely miss that
`save-excursion' is not only about restoring point (and mark).  It is
specifically designed to restore `current-buffer' as well.

And there should be no "there should be"s in technical doc:

"there should be no calls to `set-buffer' in the intervening code
 inside the `save-excursion' form before the changes to point or mark,"

Nonsense.  Let's help users understand, not just give them a catechism to follow
blindly.

"because `save-excursion' only restores the point and mark of
 the current buffer, not for the other buffers that may be the target
 of `set-buffer'"

Doesn't follow at all.  That certainly is not a reason not to use `set-buffer'
in the body of `save-excursion' before any point movements.  No connection.  In
your "because A", A is true - so what?  It does not follow that you should not
move point in buffer X after calling (set-buffer X).

This is getting sillier and sillier.  All because of a warning that no one can
decipher and the explanations for which go round and round...nowhere.

This is how we end up with "guidance" in the manual that is later pointed to as
proof, The Word, and is hammered into user heads as a catechism.  This is
obviously a controversial area where judgments differ and programmer judgment is
called for.  Please do not try to carve one view of this in stone.

Just give users the facts of what these functions do, and let them decide for
themselves whether and how to use them.  Users are not idiots, in general.

Underlying your attempt to revise the manual here, your real contribution is in
underlining that this message is incomprehensible and unhelpful on its own.  The
right solution is to get rid of the misguided message, IMO.

Re: save-excursion and save-current-buffer - edits to the manual

[Uday S Reddy]

On 3/13/2011 6:27 PM, Drew Adams wrote:

>> The debate on save-excursion getting "defeated" flared up again on
>> gnu.emacs.help.  Somebody noticed that the Lisp manual hasn't been
>> updated properly to take account of the new compiler warning.
>
> No, someone simply pointed out that the Lisp manual does not say what you were
> trying to make it say.  What it says is correct.  It just hasn't yet been purged
> of `save-excursion' the way you would like.  And that's a good thing.

I wasn't trying to settle the dispute in the manual, which is not the 
place for it anyway.  The point is merely to make it faithful to the 
compiler and put enough helpful text for people that would be mystified 
by the new compiler warning.

The Current Buffer section has not been updated after the compiler 
warning was introduced.  So, it does need revision.  In particular, it 
contradicts the compiler warning by saying explicitly that one can use 
save-excursion whereas the compiler says no.

>> The attached patch/bundle is an attempt to explain the situation as
>> well as to provide guidance on how to use these forms correctly.
>> Please let me have any comments.
>
> My comment is to please leave it as it was.

We can't, because it contradicts the compiler.

> You have replaced mention of using `save-excursion' or `save-current-buffer'
> when one uses `set-buffer' with just mention of using `save-current-buffer' with
> it.  Both can be useful here.

No, the compiler complains about save-excursion being used for this purpose.

> You've added this:
>
> "However, it is not a recommended use of `save-excursion' for reverting buffer
> changes."
>
> The word "it" references nothing here - no meaning.

If you really want "it" to refer to something, think of "it" as the 
compiler.

> And `save-excursion' is not about reverting buffer changes - never has been.
> And no one would ever think that it is.  And the warning you then try to explain
> also has nothing to do with reverting buffer changes.

But this section is about buffer changes.  And, in any case, the 
sentence before the one you quoted says:

   "save-excursion restores the current buffer and, in addition, 
restores the point and mark in that buffer as well."

> And your explanation of the warning, like the warning itself, does not help.

Since you think the warning doesn't help, I am not surprised that you 
don't find the explanation helpful.  But, perhaps, you can be a bit more 
objective here?  In what way does it not help?

> And in your "recommended practice" section, you completely miss that
> `save-excursion' is not only about restoring point (and mark).  It is
> specifically designed to restore `current-buffer' as well.
>
> And there should be no "there should be"s in technical doc:
>
> "there should be no calls to `set-buffer' in the intervening code
>   inside the `save-excursion' form before the changes to point or mark,"
>
> Nonsense.  Let's help users understand, not just give them a catechism to follow
> blindly.
>
> "because `save-excursion' only restores the point and mark of
>   the current buffer, not for the other buffers that may be the target
>   of `set-buffer'"
>
> Doesn't follow at all.

It does follow.  Here is the logic, spelled out:

1. save-excursion should be placed as close as possible to the 
point/mark-movements.

2. save-excursion only restores the current buffer's point/mark.

So, if there is a set-buffer between the save-excursion and the 
point/mark-movements, then it is not as close as possible to the 
point/mark-movements.  (In fact, in the normal cases where the 
set-buffer's are not no-op's , there would have to be at least *two* 
set-buffer's between the save-excursion and the point/mark-movements 
that it is reverting.)

> This is getting sillier and sillier.  All because of a warning that no one can
> decipher and the explanations for which go round and round...nowhere.

Sorry, I have been able to decipher it just fine.  On the other hand, I 
haven't been able to decipher what your objection is about.  So, we 
could perhaps make some progress if you can help me understand your 
objection.  If it is just a political position on what compilers should 
or should not do, I couldn't be interested because I didn't write the 
compiler.  But if there is a technical point, what is it?

Cheers,
Uday

RE: save-excursion and save-current-buffer - edits to the manual

[Drew Adams]

> The point is merely to make it faithful to the compiler...
> ... it contradicts the compiler warning by saying explicitly that 
> one can use save-excursion whereas the compiler says no.
> 
> > My comment is to please leave it as it was.
> 
> We can't, because it contradicts the compiler.
> 
> > Both [functions] can be useful here.
> 
> No, the compiler complains about save-excursion being used 
> for this purpose.

So now we've apparently gone all the way - from a warning issued supposedly just
in case, because a tiny minority of cases where `save-excursion' is used with
`set-buffer' are deemed (by some) to be inappropriate or not ideal, to your
claiming outright that use of `save-excursion' "contradicts the compiler" and
"the compiler says no", it simply cannot be used.

This is exactly how we go (quickly, it seems) down the road from carefully
reasoned judgment calls to black & white catechism.  Sheesh.  Those who come
later will have no inkling of understanding (certainly not from your addition to
the manual), but will no doubt feel proud to parrot "The Rule".  Sorry, but this
is a disservice.

You yourself have indicated that of the zillions of cases the compiler flagged
for you in the code you inherited, you changed only a small minority in the end.
So certainly it is not a no-no to use `save-excursion' with `set-buffer'.  Even
by your own estimation of what was needed in your code.

Sorry, but both `save-excursion' and `save-current-buffer' _can_ be useful here.
And it is simply a bad idea to purge the manual this way.  Regardless of what
the compiler has been made to say or suggest recently.  The manual will
hopefully offer more understanding about this than the compiler does!

And the manual does so already, in the doc of `save-excursion'.  It is very
clear, from both the manual and the doc string, what `save-excursion' does.  But
one has to take the trouble to read it.

What is in the body of `save-excursion' is irrelevant to what `save-excursion'
does.  The function simply makes the original buffer, point, and mark current
again when it ends.  To understand that is to understand everything about it,
including how and when to use it.

> > "However, it is not a recommended use of `save-excursion' 
> > for reverting buffer changes."
> >
> > The word "it" references nothing here - no meaning.
> 
> If you really want "it" to refer to something, think of "it" as the 
> compiler.

What do _you_ really want?  "However, _the compiler_ is not a recommended use of
`save-excursion' for reverting buffer changes"?  From awful to worse...

> > And `save-excursion' is not about reverting buffer changes 
> > - never has been.  And no one would ever think that it is.
> > And the warning you then try to explain also has nothing to do
> > with reverting buffer changes.
> 
> But this section is about buffer changes.  And, in any case, the 
> sentence before the one you quoted says:
> 
>    "save-excursion restores the current buffer and, in addition, 
> restores the point and mark in that buffer as well."

Yes, but none of those things have anything to do with changing the content of a
buffer, which is what _reverting buffer changes_ refers to.  Changing which
buffer is current is not related to reverting buffer changes.  Nor is changing
point or mark.  It is only your proposed addition that introduces "reverting
buffer changes".

> > And your explanation of the warning, like the warning 
> > itself, does not help.
> 
> Since you think the warning doesn't help, 

Believe me, I'm not the only one who thinks that.  Not that it matters...

> I am not surprised that you don't find the explanation helpful.
> But, perhaps, you can be a bit more objective here?  In what way
> does it not help?

See above.  You give the distinct impression that it is _wrong_ to use
`save-excursion' with `set-buffer'.  It is not wrong.  That mistaken impression
does not help.

> > "there should be no calls to `set-buffer' in the intervening code
> >  inside the `save-excursion' form before the changes to 
> >  point or mark, because `save-excursion' only restores the point
> >  and mark of the current buffer, not for the other buffers that
> >  may be the target of `set-buffer'"
> >
> > Doesn't follow at all.
> 
> It does follow.  Here is the logic, spelled out:

No, here is _your_ logic spelled out - your logic and your words:

1. "`save-excursion' only restores the point and mark of the
   current buffer, not for other buffers that may be the target
   of `set-buffer'."

2. THEREFORE, do not call `set-buffer' inside `save-excursion'
   and then move point or mark.

Look at the quotation just above this to see if I'm not right.
That's just what you said: #1 implies #2; #2 is "because" of #1.

#2 simply does _not_ follow from #1.  No way, no how.

#1 just says what `save-excursion' does, nothing more.  No objection.
#2 is your rule, parachuted in from nowhere.  Nothing in #1 implies #2.

> 1. save-excursion should be placed as close as possible to the 
>    point/mark-movements.
> 2. save-excursion only restores the current buffer's point/mark.
> 
> So, if there is a set-buffer between the save-excursion and the 
> point/mark-movements, then it is not as close as possible to the 
> point/mark-movements.

Huh?  First, you are conflating things, bringing in a general principle of
software engineering that no one disagrees with and that does not apply here
specially: do not needlessly extend scope.

More importantly, changing point and mark after invoking `set-buffer' changes
them in another buffer, the target of `set-buffer'.  `save-excursion' has _no
effect_ whatsoever on point and mark in that buffer (unless it is also the
orginal buffer).  Surely that should be clear by now.

It is a good idea (but not cause for any warning) to wrap a construct such as
`save-excursion' as tightly as possible around the things it is intended to
undo/restore.  No one argues the contrary.

If the `set-buffer' is to be reversed, i.e., the original buffer is to be
restored after the `set-buffer', then the scope must extend beyond the
`set-buffer', but certainly not because of any point or mark movements in that
buffer - `save-excursion' has no effect on them.  (Unless of course it is the
same buffer.)

And if there is code after the `set-buffer' that is also in the category of
changes to be reversed/restored, then the scope should be closed only after that
as well.  If not, then its scope should be ended before that.  But again,
`save-excursion' cannot restore anything more than (a) which buffer was
originally current, (b) its point, and (c) its mark.

At any rate, none of this is what the warning is about, according to the various
defenses of it that have been presented.  The warning does not say "The scope of
`save-excursion' is looser than it needs to be in this occurrence".  If it did,
and it were accurate, I would likely be in favor of it (but still not as a
_warning_).

> (In fact, in the normal cases where the 
> set-buffer's are not no-op's , there would have to be at least *two* 
> set-buffer's between the save-excursion and the point/mark-movements 
> that it is reverting.)

Huh?  I have no idea what you are talking about now.  `save-excursion' does not
revert point/mark in any buffers except the original one.  How many
`set-buffer's there are and which buffers become current at different points in
the body are irrelevant.  And that includes any `set-buffer's back to the
original buffer.  The only point movements in the body of a `save-excursion'
that are affected (rolled back) are those in the orginal buffer.

`save-excursion' always makes the original buffer current again and always
restores its original point and mark.  Only.  Nothing more.

What you do in the body of the `save-excursion' has no effect on this
restoration action (well, you could kill the original buffer...), and
`save-excursion' has no other action.  None.

> > This is getting sillier and sillier.  All because of a 
> > warning that no one can decipher and the explanations for
> > which go round and round...nowhere.
> 
> Sorry, I have been able to decipher it just fine.  

Yeah, and there are those who claim to hear directly from Ms Supreme Being and
impart Her wisdom to us mortels who cannot hear Her.  But each such prophet
hears a different story - go figure.  Kool Aid.

I'm sure you're deciphering.  I'm not sure you're deciphering the same thing as
others (there have been several different explanations/interpretations of what
is being warned about, including a few by Stefan himself).

But even if you are deciphering exactly what Stefan intends that doesn't make
the warning a good one.  It doesn't even make the idea of trying to have such a
warning a good idea.  The warning idea is entirely misguided, IMHO.

What's called for is making clear what `save-excursion' does and does not do.
That's all.  Help users understand what really happens and it will be clear
enough to them how and when to use it.  Will there be zero mistakes made then?
No.  But there is nothing to warn about, nothing at all.

> On the other hand, I haven't been able to decipher what your
> objection is about.

And yet I've been explicit.  Let me repeat it for you:

1. I object to having the warning at all (annoying, misleading).

2. I object to the warning's being a WARNING rather than an informative msg.
There is no DANGER here.

3. I object to the warning's not doing any good: it is unable to target real or
even possible problems narrowly.  Catching all occurrences of `save-excursion'
with `set-buffer' indicts far more innocent than guilty cases, even by Stefan's
(and your own previous) judgment.  It is a blanket alarm.

4. I object to the warning's doing real harm:

(a) It and you seem to claim that using `save-excursion' with `set-buffer' is
always or even usually a bad idea.  And that's just not true.

(b) It scares users when they byte-compile a library, and scares them only
abstractly, without imparting any understanding of what the supposed dangers are
- it's just a big, wild, noisy, scary scream: DANGER!!! LOCK THE DOOR!

(c) Overuse of WARNINGs for things that are not dangerous dilutes the meaning of
warnings - ask Chicken Little and the Boy Who Cried "Wolf!".

What would you think of a loud speaker and flashing red lights in your car
announcing every 30 seconds while you drive, "ATTENTION!! DRIVING CAN BE
DANGEROUS"?  Think it's helpful?  And yet in that case there is a _real danger_!

5. I object to your interepreting the problem, in the manual, as being about
placing `save-excursion' closer to point movements (regardless of which buffer
they are in).  That is not the crux of the problems cited by Stefan or others.
That's just sane coding practice - it applies equally to `let', and it certainly
has nothing to do with a warning or danger.  And the warning does not reflect
scope tightness; it is a knee-jerk alarm.

Re: save-excursion and save-current-buffer - edits to the manual

[Tim Cross]

[-- Attachment #1: Type: text/plain, Size: 13518 bytes --]

On Mon, Mar 14, 2011 at 11:26 AM, Drew Adams <drew.adams@oracle.com> wrote:

> > The point is merely to make it faithful to the compiler...
> > ... it contradicts the compiler warning by saying explicitly that
> > one can use save-excursion whereas the compiler says no.
> >
> > > My comment is to please leave it as it was.
> >
> > We can't, because it contradicts the compiler.
> >
> > > Both [functions] can be useful here.
> >
> > No, the compiler complains about save-excursion being used
> > for this purpose.
>
> So now we've apparently gone all the way - from a warning issued supposedly
> just
> in case, because a tiny minority of cases where `save-excursion' is used
> with
> `set-buffer' are deemed (by some) to be inappropriate or not ideal, to your
> claiming outright that use of `save-excursion' "contradicts the compiler"
> and
> "the compiler says no", it simply cannot be used.
>
> This is exactly how we go (quickly, it seems) down the road from carefully
> reasoned judgment calls to black & white catechism.  Sheesh.  Those who
> come
> later will have no inkling of understanding (certainly not from your
> addition to
> the manual), but will no doubt feel proud to parrot "The Rule".  Sorry, but
> this
> is a disservice.
>
> You yourself have indicated that of the zillions of cases the compiler
> flagged
> for you in the code you inherited, you changed only a small minority in the
> end.
> So certainly it is not a no-no to use `save-excursion' with `set-buffer'.
>  Even
> by your own estimation of what was needed in your code.
>
> Sorry, but both `save-excursion' and `save-current-buffer' _can_ be useful
> here.
> And it is simply a bad idea to purge the manual this way.  Regardless of
> what
> the compiler has been made to say or suggest recently.  The manual will
> hopefully offer more understanding about this than the compiler does!
>
> And the manual does so already, in the doc of `save-excursion'.  It is very
> clear, from both the manual and the doc string, what `save-excursion' does.
>  But
> one has to take the trouble to read it.
>
> What is in the body of `save-excursion' is irrelevant to what
> `save-excursion'
> does.  The function simply makes the original buffer, point, and mark
> current
> again when it ends.  To understand that is to understand everything about
> it,
> including how and when to use it.
>
> > > "However, it is not a recommended use of `save-excursion'
> > > for reverting buffer changes."
> > >
> > > The word "it" references nothing here - no meaning.
> >
> > If you really want "it" to refer to something, think of "it" as the
> > compiler.
>
> What do _you_ really want?  "However, _the compiler_ is not a recommended
> use of
> `save-excursion' for reverting buffer changes"?  From awful to worse...
>
> > > And `save-excursion' is not about reverting buffer changes
> > > - never has been.  And no one would ever think that it is.
> > > And the warning you then try to explain also has nothing to do
> > > with reverting buffer changes.
> >
> > But this section is about buffer changes.  And, in any case, the
> > sentence before the one you quoted says:
> >
> >    "save-excursion restores the current buffer and, in addition,
> > restores the point and mark in that buffer as well."
>
> Yes, but none of those things have anything to do with changing the content
> of a
> buffer, which is what _reverting buffer changes_ refers to.  Changing which
> buffer is current is not related to reverting buffer changes.  Nor is
> changing
> point or mark.  It is only your proposed addition that introduces
> "reverting
> buffer changes".
>
> > > And your explanation of the warning, like the warning
> > > itself, does not help.
> >
> > Since you think the warning doesn't help,
>
> Believe me, I'm not the only one who thinks that.  Not that it matters...
>
> > I am not surprised that you don't find the explanation helpful.
> > But, perhaps, you can be a bit more objective here?  In what way
> > does it not help?
>
> See above.  You give the distinct impression that it is _wrong_ to use
> `save-excursion' with `set-buffer'.  It is not wrong.  That mistaken
> impression
> does not help.
>
> > > "there should be no calls to `set-buffer' in the intervening code
> > >  inside the `save-excursion' form before the changes to
> > >  point or mark, because `save-excursion' only restores the point
> > >  and mark of the current buffer, not for the other buffers that
> > >  may be the target of `set-buffer'"
> > >
> > > Doesn't follow at all.
> >
> > It does follow.  Here is the logic, spelled out:
>
> No, here is _your_ logic spelled out - your logic and your words:
>
> 1. "`save-excursion' only restores the point and mark of the
>   current buffer, not for other buffers that may be the target
>   of `set-buffer'."
>
> 2. THEREFORE, do not call `set-buffer' inside `save-excursion'
>   and then move point or mark.
>
> Look at the quotation just above this to see if I'm not right.
> That's just what you said: #1 implies #2; #2 is "because" of #1.
>
> #2 simply does _not_ follow from #1.  No way, no how.
>
> #1 just says what `save-excursion' does, nothing more.  No objection.
> #2 is your rule, parachuted in from nowhere.  Nothing in #1 implies #2.
>
> > 1. save-excursion should be placed as close as possible to the
> >    point/mark-movements.
> > 2. save-excursion only restores the current buffer's point/mark.
> >
> > So, if there is a set-buffer between the save-excursion and the
> > point/mark-movements, then it is not as close as possible to the
> > point/mark-movements.
>
> Huh?  First, you are conflating things, bringing in a general principle of
> software engineering that no one disagrees with and that does not apply
> here
> specially: do not needlessly extend scope.
>
> More importantly, changing point and mark after invoking `set-buffer'
> changes
> them in another buffer, the target of `set-buffer'.  `save-excursion' has
> _no
> effect_ whatsoever on point and mark in that buffer (unless it is also the
> orginal buffer).  Surely that should be clear by now.
>
> It is a good idea (but not cause for any warning) to wrap a construct such
> as
> `save-excursion' as tightly as possible around the things it is intended to
> undo/restore.  No one argues the contrary.
>
> If the `set-buffer' is to be reversed, i.e., the original buffer is to be
> restored after the `set-buffer', then the scope must extend beyond the
> `set-buffer', but certainly not because of any point or mark movements in
> that
> buffer - `save-excursion' has no effect on them.  (Unless of course it is
> the
> same buffer.)
>
> And if there is code after the `set-buffer' that is also in the category of
> changes to be reversed/restored, then the scope should be closed only after
> that
> as well.  If not, then its scope should be ended before that.  But again,
> `save-excursion' cannot restore anything more than (a) which buffer was
> originally current, (b) its point, and (c) its mark.
>
> At any rate, none of this is what the warning is about, according to the
> various
> defenses of it that have been presented.  The warning does not say "The
> scope of
> `save-excursion' is looser than it needs to be in this occurrence".  If it
> did,
> and it were accurate, I would likely be in favor of it (but still not as a
> _warning_).
>
> > (In fact, in the normal cases where the
> > set-buffer's are not no-op's , there would have to be at least *two*
> > set-buffer's between the save-excursion and the point/mark-movements
> > that it is reverting.)
>
> Huh?  I have no idea what you are talking about now.  `save-excursion' does
> not
> revert point/mark in any buffers except the original one.  How many
> `set-buffer's there are and which buffers become current at different
> points in
> the body are irrelevant.  And that includes any `set-buffer's back to the
> original buffer.  The only point movements in the body of a
> `save-excursion'
> that are affected (rolled back) are those in the orginal buffer.
>
> `save-excursion' always makes the original buffer current again and always
> restores its original point and mark.  Only.  Nothing more.
>
> What you do in the body of the `save-excursion' has no effect on this
> restoration action (well, you could kill the original buffer...), and
> `save-excursion' has no other action.  None.
>
> > > This is getting sillier and sillier.  All because of a
> > > warning that no one can decipher and the explanations for
> > > which go round and round...nowhere.
> >
> > Sorry, I have been able to decipher it just fine.
>
> Yeah, and there are those who claim to hear directly from Ms Supreme Being
> and
> impart Her wisdom to us mortels who cannot hear Her.  But each such prophet
> hears a different story - go figure.  Kool Aid.
>
> I'm sure you're deciphering.  I'm not sure you're deciphering the same
> thing as
> others (there have been several different explanations/interpretations of
> what
> is being warned about, including a few by Stefan himself).
>
> But even if you are deciphering exactly what Stefan intends that doesn't
> make
> the warning a good one.  It doesn't even make the idea of trying to have
> such a
> warning a good idea.  The warning idea is entirely misguided, IMHO.
>
> What's called for is making clear what `save-excursion' does and does not
> do.
> That's all.  Help users understand what really happens and it will be clear
> enough to them how and when to use it.  Will there be zero mistakes made
> then?
> No.  But there is nothing to warn about, nothing at all.
>
> > On the other hand, I haven't been able to decipher what your
> > objection is about.
>
> And yet I've been explicit.  Let me repeat it for you:
>
> 1. I object to having the warning at all (annoying, misleading).
>
> 2. I object to the warning's being a WARNING rather than an informative
> msg.
> There is no DANGER here.
>
> 3. I object to the warning's not doing any good: it is unable to target
> real or
> even possible problems narrowly.  Catching all occurrences of
> `save-excursion'
> with `set-buffer' indicts far more innocent than guilty cases, even by
> Stefan's
> (and your own previous) judgment.  It is a blanket alarm.
>
> 4. I object to the warning's doing real harm:
>
> (a) It and you seem to claim that using `save-excursion' with `set-buffer'
> is
> always or even usually a bad idea.  And that's just not true.
>
> (b) It scares users when they byte-compile a library, and scares them only
> abstractly, without imparting any understanding of what the supposed
> dangers are
> - it's just a big, wild, noisy, scary scream: DANGER!!! LOCK THE DOOR!
>
> (c) Overuse of WARNINGs for things that are not dangerous dilutes the
> meaning of
> warnings - ask Chicken Little and the Boy Who Cried "Wolf!".
>
> What would you think of a loud speaker and flashing red lights in your car
> announcing every 30 seconds while you drive, "ATTENTION!! DRIVING CAN BE
> DANGEROUS"?  Think it's helpful?  And yet in that case there is a _real
> danger_!
>
> 5. I object to your interepreting the problem, in the manual, as being
> about
> placing `save-excursion' closer to point movements (regardless of which
> buffer
> they are in).  That is not the crux of the problems cited by Stefan or
> others.
> That's just sane coding practice - it applies equally to `let', and it
> certainly
> has nothing to do with a warning or danger.  And the warning does not
> reflect
> scope tightness; it is a knee-jerk alarm.
>
>
> It would seem two different issues are being mixed up here.

Issue 1 appears to be a debate concerning whether the situation covered by
the warning is serious enough or sufficient to be classified as a warning.
 At best, I suspect it is borderline - possibly even less so given that it
appears to be somewhat 'simplistic' and cannot identify situations where
set-buffer within a save-excursion is legit from those where it is either
the source of an error or could be replaced with just a with-current-buffer.
 I do think the information can be useful and would have no issue with it
being reported as a notice or even removed and only included in something
like elint.  The distinction between notice and warning is blurred and
changing to notice may not actually help - I tend to think of a notice as
more like information rather than a warning of a possible problem. Whether
it is a warning, a notice or removed altogether, is largely irrelevant from
my standpoint.

Issue 2. is, given that the compiler does report this warning, is the manual
clear enough or detailed enough that the programmer can understand what the
warning means and when to be concerned or not. I think this is the real
issue that needs to be addressed. I've not read Uday's suggested
contribution and therefore won't comment on it. However, as things currently
stand, I don't think the manual addresses the matter sufficiently and that
without this, the warning will continue to be a source of confusion (and
probably future debate). Either the warning should be removed (which seems
unlikely) or we try to enhance the manual to explain what the warning means
and provide the background information that would prevent confusion and
possibly make the warning more useful.

Lets separate the two issues and deal with them individually. Those who want
to debate the merits/validity of the warning can do so. Those who want the
warning to be less confusing and possibly more informative might like to try
and suggest patches to the manual.

Tim

[-- Attachment #2: Type: text/html, Size: 15581 bytes --]

Re: save-excursion and save-current-buffer - edits to the manual

[Richard Stallman]

    So now we've apparently gone all the way - from a warning issued supposedly just
    in case, because a tiny minority of cases where `save-excursion' is used with
    `set-buffer' are deemed (by some) to be inappropriate or not ideal, to your
    claiming outright that use of `save-excursion' "contradicts the compiler" and
    "the compiler says no", it simply cannot be used.

Compiler warnings should be programmers' assistants, not their
masters.  That is why we have with-no-warnings.  We implement helpful
warnings about constructions that _might_ be mistakes, but that
doesn't mean they are taboo.


-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org, www.gnu.org

Re: save-excursion and save-current-buffer - edits to the manual

[Uday S Reddy]

Richard Stallman writes:

> Compiler warnings should be programmers' assistants, not their
> masters.  That is why we have with-no-warnings.  We implement
> helpful warnings about constructions that _might_ be mistakes, but
> that doesn't mean they are taboo.

Of course, the manuals never stop the programmers from doing what they
want to do.  They can at best explain and advise, and leave the final
judgement to the programmer.

But we need a coherent story to tell between the compiler and the
manual.  If the comiler warns "there is a problem doing this" and the
manual says "go ahead and do it", then we don't have a coherent
picture.

Cheers,
Uday

Re: save-excursion and save-current-buffer - edits to the manual

[Stefan Monnier]

> The right solution is to get rid of the misguided message, IMO.

If you want your messages to be useful, better start by forgetting about
this option.


        Stefan

RE: save-excursion and save-current-buffer - edits to the manual

[Drew Adams]

> > The right solution is to get rid of the misguided message, IMO.
> 
> If you want your messages to be useful, better start by 
> forgetting about this option.

As I said earlier, I made my peace with this misguided message long ago, knowing
your intention to keep it.

Nevertheless, it is MHO that it should be removed, and I haven't been convinced
otherwise.  I think my messages about this are generally thought out and useful,
even if they don't persuade you in particular.  Just because someone expresses
disagreement, that does not make his expression useless.

Re: save-excursion and save-current-buffer - edits to the manual

[Chong Yidong]

Uday S Reddy <u.s.reddy@cs.bham.ac.uk> writes:

> The debate on save-excursion getting "defeated" flared up again on
> gnu.emacs.help.  Somebody noticed that the Lisp manual hasn't been
> updated properly to take account of the new compiler warning.
>
> The attached patch/bundle is an attempt to explain the situation as
> well as to provide guidance on how to use these forms correctly.
> Please let me have any comments.

Thank you.  I've committed a patch, along the same lines but using
different wording, to the branch.