From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: "Drew Adams" Newsgroups: gmane.emacs.help Subject: RE: `save-excursion' defeated by `set-buffer' Date: Mon, 14 Mar 2011 10:36:40 -0700 Message-ID: <5F174275B43A467AA8FA2A1928774136@us.oracle.com> References: <4D792D16.1080900@easy-emacs.de><83y64ksoik.fsf@gnu.org><87aah0j7lf.fsf@rapttech.com.au><19836.48264.656000.373465@gargle.gargle.HOWL> <19838.9290.109000.165849@gargle.gargle.HOWL> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Trace: dough.gmane.org 1300124280 13086 80.91.229.12 (14 Mar 2011 17:38:00 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Mon, 14 Mar 2011 17:38:00 +0000 (UTC) Cc: 'Tim X' , help-gnu-emacs@gnu.org To: "'Uday S Reddy'" Original-X-From: help-gnu-emacs-bounces+geh-help-gnu-emacs=m.gmane.org@gnu.org Mon Mar 14 18:37:53 2011 Return-path: Envelope-to: geh-help-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1PzBiK-0002Hr-Bq for geh-help-gnu-emacs@m.gmane.org; Mon, 14 Mar 2011 18:37:53 +0100 Original-Received: from localhost ([127.0.0.1]:46737 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PzBiJ-0005m4-PA for geh-help-gnu-emacs@m.gmane.org; Mon, 14 Mar 2011 13:37:51 -0400 Original-Received: from [140.186.70.92] (port=46363 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PzBhW-0005jk-Di for help-gnu-emacs@gnu.org; Mon, 14 Mar 2011 13:37:04 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PzBhS-0004Ag-JW for help-gnu-emacs@gnu.org; Mon, 14 Mar 2011 13:37:02 -0400 Original-Received: from rcsinet10.oracle.com ([148.87.113.121]:39106) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PzBhR-0004AN-Te for help-gnu-emacs@gnu.org; Mon, 14 Mar 2011 13:36:58 -0400 Original-Received: from rcsinet13.oracle.com (rcsinet13.oracle.com [148.87.113.125]) by rcsinet10.oracle.com (Switch-3.4.2/Switch-3.4.2) with ESMTP id p2EHahLl032114 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 14 Mar 2011 17:36:45 GMT Original-Received: from acsmt356.oracle.com (acsmt356.oracle.com [141.146.40.156]) by rcsinet13.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id p2EHagV0011365 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 14 Mar 2011 17:36:43 GMT Original-Received: from abhmt014.oracle.com (abhmt014.oracle.com [141.146.116.23]) by acsmt356.oracle.com (8.12.11.20060308/8.12.11) with ESMTP id p2EHagEX026573; Mon, 14 Mar 2011 12:36:42 -0500 Original-Received: from dradamslap1 (/130.35.178.194) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Mon, 14 Mar 2011 10:36:41 -0700 X-Mailer: Microsoft Office Outlook 11 In-Reply-To: <19838.9290.109000.165849@gargle.gargle.HOWL> Thread-Index: AcviUxCrKFf76wGXQ0O1eV/kQM/rDQADyl4Q X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.5994 X-Source-IP: acsmt356.oracle.com [141.146.40.156] X-Auth-Type: Internal IP X-CT-RefId: str=0001.0A090209.4D7E522B.021A,ss=1,fgs=0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 148.87.113.121 X-BeenThere: help-gnu-emacs@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Users list for the GNU Emacs text editor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: help-gnu-emacs-bounces+geh-help-gnu-emacs=m.gmane.org@gnu.org Errors-To: help-gnu-emacs-bounces+geh-help-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.help:80101 Archived-At: You repeated a lot Uday, but that's OK if it helps you understand or express yourself. > I am rewriting the text surrounding the example, based on your questions. > This `my-beautiful-function' was designed to be called in a buffer A. > > (defun my-beautiful-function () > (save-excursion > (set-buffer B) > (goto-char x) > (setq a (find-something)) > ...)) > > (defun find-something () > (.... > (save-excursion > (set-buffer A) > (goto-char y) > ...))) > > The assumption is that find-something is *buggy* here. It is moving > point in buffer A though it wasn't supposed to. Though you did not state this as part of your scenario, since you have mentioned it otherwise may I add the assumption that this hypothetical code came from an existing application written by a good programmer, and that that code generally works well and has done so for quite some time? IOW, I think you are trying to point out that code that has worked well can nevertheless be fragile. So let's assume that with no changes the code works, and the person who wrote it was no fool - s?he wrote what s?he intended and needed. Let's imagine further that this code was inherited by someone else, and that s?he is not necessarily familiar with all of the details. > The `save-excursion' at the outer level in `my-beautiful-function' was > presumably placed there to restore the current buffer. Given the assumptions that I added (fairly, I hope), I would say that presumably the `save-excursion' is there NOT ONLY to restore which buffer was current but also restore point and mark in that buffer. That's what `save-excursion' DOES, and there is no reason not to suppose that the original author did not intend that. Even if the inheritor is not sure of things, we must give the benefit of the doubt to the original designer/programmer, if for no other reason than the code works. IOW, given inherited code that seems to work, assume as a start that it works _because_ of the code that is there. Do not assume that the code can just be altered willy-nilly, replacing some functionality by a subset that does not do the whole job. That might be the case, and perhaps the inheritor can improve the code, but it would be a mistake to just assume that things can be changed willy-nilly. > Just looking at its body, we don't see any point movements in buffer A. > So, we might feel justified in thinking that the `save-excursion' can be > replaced by `save-current-buffer'. But let us leave that aside for > the moment. I think that would be a big mistake - a very rash assumption. A wiser and more conservative assumption would be that even if you see no A-buffer point movements directly in `m-b-f', somewhere in some code invoked within the `save-excursion' there are or probably are A-buffer movements. In fact, precisely because you do not see point-A movements in `m-b-f' itself, you can be almost sure there could be such in code called withing the `save-excursion'. Why? Because the original programmer used `save-excursion'. IOW, again, do NOT assume that the `save-excursion' is intending to only `save-current-buffer'. Such an assumption is unwarranted based only on a glance at the `m-b-f' code - after all, it does call other code. > As long as you use `my-beautiful-function' in buffer A, it will appear > to work fine, because the unprotected point movement in > `find-something' is cancelled by the `save-excursion' at the > top-level. If you call it in some other buffer C, it will > unexpectedly move point in buffer A. Correct. > My position is that `find-something' is *inherently buggy* because of > its unprotected point movement in buffer A. Well of course that's your position - and mine too, since you _posited_ that as your hypothesis! You seem to be trying to conclude what you hypothesized. (??) > The fact that it appears to work fine as long as it is called > from `my-beautiful-function' invoked in buffer A, doesn't make it right. Dunno about "right". But it works (by hypothesis). Again, I think you are saying that code that works can nevertheless be fragile. I might add: especially if the code is maintained by someone other than the original author - or even, as sometimes happens to me, if the original author (me) has simply forgotten some of the logic/details. ;-) > The `save-excursion' at the top-level of `my-beautiful-function' > serves to cover up the bug, whether intentionally or unintentionally. Correct. The original programmer's use of `save-excursion' does what s?he wants: ensure that buffer A and its point and mark are restored, regardless of what the code in the body of the `save-excursion' might do. IOW, the `m-b-f' code _ensures_ against any A-buffer point movements, whether from bugs or otherwise. The code in `f-s' is buggy by hypothesis, since you said that it was not intended that it move point in A, but even if it were not buggy and that movement were intentional, if the intent of using the `save-excursion' is what we must assume it is (restore point as well as the buffer), then things work as they should. Call it a "cover up" if you want, but that's the intention behind and the documented behavior of `save-excursion'. > The solution is to change the `save-excursion' to > `save-current-buffer', exposing the bug, and then to track it down. That would not be my solution, but feel free. Rather, that might be one way to help you find the bug, if it really need be found. But it is also problematic - be sure to remember to return to using `save-excursion' if needed. I would probably let the sleeping dog lie, and wait until the `f-s' bug actually manifested itself. After all, the `save-excursion' code does just what you want here (by hypothesis): it ensures that _even if_ some code in the body (rightly or wrongly) moves point in A, point in A will get restored. If you later call `m-b-f' from some other buffer (e.g. Z) than A, then hopefully you would ask yourself this immediately: "Hey, this `save-excursion' restores things for the buffer that was current before. What buffer was that? A. So if I now call it from buffer Z then I probably need to wrap things in `with-current-buffer' or some such. I certainly cannot just assume that now I should restore things in Z instead of A." Or else, if you do not want to just wrap that way, you will want to analyze things in detail to find out just what point movements are involved, why the code restored point in A, etc. Or if you do not reflect this way, hopefully you will see some change in behavior and track down the bug in `f-s'. The point is that you should _not_ just _assume_ that `save-excursion' is used only in a way that treats it as `save-current-buffer'. Assume instead that all of its behavior is needed, then work from there. You can, as you suggest, try substituting `save-current-buffer' to see what happens (i.e. as a diagnostic exercise), but be prepared for (look out for) behavior changes. > > 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). > > Why do you think m-b-f "wants" to restore point in A, and not in other > buffers? Because the original programmer said so, a priori, by choosing to use `save-excursion' in buffer A. That's what `save-excursion' does. We must presume that the programmer knew what s?he was doing and knew that point in A needed to be restored. On what basis could you presume anything different? You seem to be so convinced (unlike Stefan, BTW) that `save-excursion' is evil that you assume that its point-saving behavior is not needed (in code that works). > What makes the buffer A special? Why is it that you should > always restore the point in the current buffer and leave all the other > buffers to their fate? That is what `save-excursion' does. If you want something different, then code something different. By hypothesis, the original programmer used `save-excursion', and knew what s?he was doing. Don't ask me why s?he felt that A needed restoring and not other buffers. But s?he did (by hypothesis), or s?he wouldn't have used `save-excursion'. > > 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 `...'). > > It may not may have been intended to be called in buffer C, but > nothing in the design might indicate that it is intended to be called > only in buffer A. So, you would be justified in calling it in > another buffer. Ooops. No, but thanks for playing. Something in the (inherited) implementation is crying loud and clear that it is intended to be called from A and it is intended to restore A. What is that? `save-excursion' was called from buffer A, and its job is to restore the current buffer. This couldn't be clearer. You ignore such a clear indication of programmer intention (implementation and probably design) at your own peril. Let me repeat: `save-excursion' is _not_ `save-current-buffer'. You substitute the latter for the former willy-nilly at your own peril. > > 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. > > So, how much detail would one need to see before one can infer basic > facts about point movements and buffer changes? See above. Infer from the presence of `save-excursion' that there is a _need_ to restore the original buffer, which was A, as well as its point and mark. That is your best assumption, since the code has been working and the programmer was no idiot. Assume anything else by way of shortcut at your own peril. Tread carefully and analyze closely. Again, it's fine to try substituting `save-current-buffer' as a probing operation, to try to see what will happen - what might break, but be aware that such a switch automatically changes the behavior: you are no longer saving and restoring point (and mark). > > > 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.) > > I have no idea what it means for m-b-f "wanting" to restore A's point. The presence of `save-excursion' indicates that the current buffer's point will be restored. By hypothesis the code reflects the programmer's intention (it works and has been used for a long, long time). `save-excursion' "wants" to restore point, and it does so. > I don't see why it should care about the buffer A at all. The way I > view things, m-b-f should restore whatever temporary point movements > it makes. Well you are jumping to a wild conclusion then, one not based on the existing code. The existing code says only that `m-b-f' should restore point for the original buffer (which is A). If the intent had been to restore point movements in all buffers then a very different program would have to have been written. `save-excursion' does NOT do that. One of Stefan's points (and a/the reason for the warning) was that some people can get fooled into thinking it does, but at this point at least _you_ should not be so fooled. Nothing in that code indicates any intention to restore point movements in all buffers. Absolutely nothing. Quite the contrary. The code is telling us loud and clear that the intention is to restore point _only_ in the original buffer (e.g., A). > It shouldn't be covering up the bugs in other functions > like `find-something'. But the way the save-excursion/set-buffer > combination works, everything is covering up everything else's bugs. > The code might be completely infested with bugs. But we will never > know, because it is also constantly covering them up! Dunno what to say, Uday, without repeating myself more. `save-excursion' saves and restores point in only one buffer. If that is not the intention then don't use it. What you call "covering up" is exactly the ignoring of temporary point movements (for _one_ buffer). > > `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. > > Great, I seem to have hit a home run. `find-something' bug will not > surface as long as it is called inside a `save-excursion' for buffer > A. You said it! Glad you got it. Whether buggy or intended, any A-buffer point movements in `f-s' are ultimately lost if it is called within a `save-excursion' invoked from A. That's the intention behind `save-excursion'. > Now, what plausible reason is there for m-b-f to use `save-excursion' > if not for covering up the `find-something' bug? Uh, to do what it does? To ignore point movements in the buffer it's called from? We have to presume that it _means_ to do that, unless you have some reason not to. > If the bug wasn't there, we can simply convert `save-excursion' > to `save-current-buffer' and we will be fine. If `save-current-buffer' were all that was needed here to start with, that is, if the programmer did _not_ intend/need to restore point for buffer A, then `save-excursion' would not have been used. Even in code written long, long ago the programmer would have simply saved and restored the buffer and not also the values of point and mark. > My belief is that all save-excursions used with set-buffer are > precisely there to cover up bugs. What can I say, Uday? > If there is any other conceivable reason why `save-excursion' should > be used with `set-buffer', then please show me! Please read the original thread if you don't get it by now. What if you need to save and later restore point in a buffer, Uday, what do _you_ do? You can use `save-excursion' with `save-current-buffer' or with `with-current-buffer' instead of with `set-buffer', but you still need to use `save-excursion' (or roll your own point-saving/restoring code). Your beef seems to be with `save-excursion' and the fact that it saves and restores point for a buffer. Your beef _should_ be with `set-buffer', I would think. I would think you would simply be arguing for using 1 instead of 2, in general: 1. (save-excursion (with-current-buffer Z...)) 2. (save-excursion (set-buffer Z)...) But #1, like #2 has all of the bad "cover-up" behavior you decry. `save-excursion' saves point for the buffer in which it is invoked. Invoke it from a different buffer with no other changes and you are likely to get some behavior you don't want.