[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: `save-excursion' defeated by `set-buffer'
From: |
Drew Adams |
Subject: |
RE: `save-excursion' defeated by `set-buffer' |
Date: |
Mon, 14 Mar 2011 01:47:45 -0700 |
> It takes quite a bit of insight into the code to understand that it is
> actually *harmful* redundancy.
What redundancy? What harm?
> The unintentional uses of save-excursion cover up potential
> bugs inside the code which don't get noticed because of this
> redundant saving.
Redundant saving? What redundant saving? Each use of `save-excursion' saves
and restores the buffer that was originally current along with its point and
mark. Where's the redundancy? Where's the harm?
> So, my test is pure and simple. Convert all the save-excursion's that
> the compiler complains about to save-current-buffer.
OMG.
> If your code continues to run perfectly, you are my hero.
> If your code falls over, then you should accept that your code is
> buggy
If it wasn't before, it probably is after your near-global search-and-replace.
And what if your code doesn't seem to fall over immediately, and it takes you 2
years to discover that it was misguided after all not to save and restore point
in that particular case?
> and it is only able to stand up on the crutches of what you thought
> was "harmless redundancy".
Please tell us more about the harmless redundancy and the harmful redundancy.
Or even just about the redundancy.
> Then it is up to you whether you want to fix the code or
> continue to live with it. But there are many of us who won't touch
> such code with a tadpole because we regard it as buggy.
>
> Unfortunately, it is more than a question of clarity. It is a
> question of correctness. Let me restate my example from Friday a bit
> more explicitly
>
> (defun my-beautiful-function ()
> (save-excursion
> (set-buffer B)
> (goto-char x)
> (setq a (find-something))
> ...))
>
> (defun find-something ()
> (....
> (set-buffer A)
> (goto-char y)
> ...))
>
> find-something is *buggy* here. It is moving point in buffer A though
> it wasn't supposed to.
Why wasn't it supposed to? How can you know from this skeleton whether it was
supposed to move point in A or not?
Or are you just telling us that it is _hypothetically_ buggy, that
_hypothetically_ it is not supposed to move point in A, but it does?
Tell us more. What buffer is `m-b-f' called in? Presumably, if it is intended
to be called in buffer A, then it wants to restore point in A when it is done
(assuming the `save-excursion' is the last thing it does).
Presumably, if it is intended instead to be called in buffer C then it wants to
restore point in C when it is done. In this case, presumably (if we don't just
assume that it is buggy) `find-something' wants to leave point in A at Y
(assuming it doesn't move it in the last `...').
There is no way to know from your skeleton what the purpose or the behavior is.
The devil is in the details...which are missing.
There is nothing intrinsically wrong with a function that moves point to some
position in some buffer and leaves it there. I probably wouldn't name such a
function just `find-something', however, and I would document that part of its
purpose is to move point in buffer A or whatever. Assuming that is the case.
> However, my-beautiful-function works fine as
> long as I call it in buffer A because the "harmlessly redundant"
> save-excursion at its top-level restores A's point.
Oh, so you were presuming that `m-b-f' wants to restore A's point. OK. Where's
the problem? (So far nothing is _redundant_, however.)
If `m-b-f' intends to always do things in buffer A and afterward restore point
in A to what it was initially, and if it doesn't know which buffer it will be
called from, then it should wrap (with-current-buffer A ...) around its
`save-excursion'.
> Tomorrow, you decide to call my-beautiful-function from some other
> buffer C, and all hell breaks loose. You suddenly find that the point
> is moving in the unrelated buffer A and you have no idea why.
Well you should know. You should know from the name of `find-something' (which
needs a better name) and from its doc that it moves point in A.
Or if by hypothesis `f-s' is buggy and should not in fact move point in A, then
you should be able to see by debugging that it in fact does move point in A,
even when `m-b-f' is called in A. And you should be able to tell just by
looking at the code that it moves point in A.
I don't deny that if the overall code is complex such a bug might not be
obvious, but I submit that this is a general problem that has nothing special to
do with `save-excursion' or `set-buffer'. It is simply about keeping track of
the side effects performed by your code.
`find-something' as written has the side effect of moving point in buffer A -
always. If that is not the intention, then yes, its bug will not surface as
long as it is called inside a `save-excursion' for buffer A.
The same problem would exist for a function that increments a variable X by 10
and is called by a function within a `let' that binds it to 42. You might never
notice the incrementing as long as that `let' binding is there. Call the
incrementing function from another context and you might suddenly discover the
incrementing.
There are a zillion cases of such things in a language like Lisp. Given both
its side effects and dynamic binding there are all kinds of possible pitfalls in
Lisp.
It should be very clear that a construct that restores point for the current
buffer will restore point for the current buffer (!). Restoring point for a
given buffer hides point movements that took place between saving and restoring
- of course. The same would be true if you instead used your proposed
`save-point-and-mark' to do the saving and restoring. Nothing remarkable here
at all.
Nothing mysterious. Except for the various `...' and the context of use
(invocation). Without knowing those things, without knowing the intended
behavior, the behavior could be nearly anything.
> So, rewrite the code as follows:
>
> (defun my-beautiful-function ()
> (save-current-buffer
> (set-buffer B)
> (goto-char x)
> ...
> (setq a (find-something))
> ...))
>
> (defun find-something ()
> (....
> (save-current-buffer
> (set-buffer A)
> (save-excursion
> (goto-char y)
> ...)))
>
> Not only would you have mollified the compiler, but you will have much
> more robust code that will continue to function when used in
> unforeseen ways.
Oh, so you meant that `m-b-f' doesn't need to do anything specifically in A, and
it need not be called from A. Such details were missing. And `f-s' should not
move point in A (the buggy hypothesis). In that case, sure, that's a perfectly
fine way to do what you apparently want.
It all depends on what the intended behavior is. You might well want `f-s' to
move point in A. You might, as part of its contract, know that `m-b-f' will
always be called from A, or from C, or whatever.
- Re: `save-excursion' defeated by `set-buffer', (continued)
- Message not available
- Re: `save-excursion' defeated by `set-buffer', Tim X, 2011/03/12
- Re: `save-excursion' defeated by `set-buffer', Andreas Röhler, 2011/03/13
- RE: `save-excursion' defeated by `set-buffer', Drew Adams, 2011/03/13
- Message not available
- Re: `save-excursion' defeated by `set-buffer', Uday Reddy, 2011/03/13
- RE: `save-excursion' defeated by `set-buffer', Drew Adams, 2011/03/13
- Message not available
- Re: `save-excursion' defeated by `set-buffer', Uday Reddy, 2011/03/13
- RE: `save-excursion' defeated by `set-buffer', Drew Adams, 2011/03/13
- Re: `save-excursion' defeated by `set-buffer', Uday S Reddy, 2011/03/13
- RE: `save-excursion' defeated by `set-buffer',
Drew Adams <=
- Message not available
- Re: `save-excursion' defeated by `set-buffer', Uday Reddy, 2011/03/14
- Re: `save-excursion' defeated by `set-buffer', Andreas Röhler, 2011/03/14
- Message not available
- RE: `save-excursion' defeated by `set-buffer', Uday S Reddy, 2011/03/14
- RE: `save-excursion' defeated by `set-buffer', Drew Adams, 2011/03/14
- RE: `save-excursion' defeated by `set-buffer', Uday S Reddy, 2011/03/14
- RE: `save-excursion' defeated by `set-buffer', Drew Adams, 2011/03/14
- Message not available
- Re: `save-excursion' defeated by `set-buffer', Stefan Monnier, 2011/03/15
- RE: `save-excursion' defeated by `set-buffer', Drew Adams, 2011/03/15
- Message not available
- Re: `save-excursion' defeated by `set-buffer', Stefan Monnier, 2011/03/15
- RE: `save-excursion' defeated by `set-buffer', Drew Adams, 2011/03/15