From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Tomas Nordin Newsgroups: gmane.emacs.bugs Subject: bug#45226: Avoid redundant calls in narrow-to-defun Date: Sun, 13 Dec 2020 19:16:24 +0100 Message-ID: <87mtyhwozr.fsf@posteo.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="23226"; mail-complaints-to="usenet@ciao.gmane.io" To: 45226@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Dec 13 19:17:18 2020 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1koVvq-0005w0-AM for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 13 Dec 2020 19:17:18 +0100 Original-Received: from localhost ([::1]:33116 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1koVvp-0007Bz-2l for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 13 Dec 2020 13:17:17 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:36130) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1koVvb-0007BR-Sb for bug-gnu-emacs@gnu.org; Sun, 13 Dec 2020 13:17:04 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:38557) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1koVva-0008E7-G4 for bug-gnu-emacs@gnu.org; Sun, 13 Dec 2020 13:17:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1koVva-0002vs-CU for bug-gnu-emacs@gnu.org; Sun, 13 Dec 2020 13:17:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Tomas Nordin Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 13 Dec 2020 18:17:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 45226 X-GNU-PR-Package: emacs X-Debbugs-Original-To: bug-gnu-emacs@gnu.org Original-Received: via spool by submit@debbugs.gnu.org id=B.160788339411232 (code B ref -1); Sun, 13 Dec 2020 18:17:02 +0000 Original-Received: (at submit) by debbugs.gnu.org; 13 Dec 2020 18:16:34 +0000 Original-Received: from localhost ([127.0.0.1]:50103 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1koVv8-0002v6-01 for submit@debbugs.gnu.org; Sun, 13 Dec 2020 13:16:34 -0500 Original-Received: from lists.gnu.org ([209.51.188.17]:55466) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1koVv6-0002uy-9d for submit@debbugs.gnu.org; Sun, 13 Dec 2020 13:16:32 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:35938) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1koVv6-00078E-4O for bug-gnu-emacs@gnu.org; Sun, 13 Dec 2020 13:16:32 -0500 Original-Received: from mout02.posteo.de ([185.67.36.66]:52035) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1koVv3-0008AE-G3 for bug-gnu-emacs@gnu.org; Sun, 13 Dec 2020 13:16:31 -0500 Original-Received: from submission (posteo.de [89.146.220.130]) by mout02.posteo.de (Postfix) with ESMTPS id 1E2C8240100 for ; Sun, 13 Dec 2020 19:16:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1607883387; bh=13xHk6RFOWtmRipV43jho9V7LORN8iGHuIRswr2egkI=; h=From:To:Subject:Date:From; b=nbcYINvE5w3Ll0NDyaP9PLQT3YTRS8piE8ktPUGYLVtXsyBIYZa6HoMRL3g1scdda ufJnYWXXG7Y2eoSGk67sTJKjJKVGd6W7Dlu4xTKOMEikUpohvaqCeMHYoxPSD62Ye3 HL+kIELM/WcflMzwiWaPpDTE2adZqM08YnA0TQj+J7ndLIfkaWqA/9qIKOoYpd6KNX 8NhPoHuM8Pp9p/E5hYRUn/FnL3rI0Z5l4A/0we86MMMxqvtSPlSI27otm1gWSsl0hN 0OqkzlCi/Gdw9e06FJTCZrqyJcOBlXMzjU0J0gmpkmH4T9BYQXAd6iM7828aj5eAJH Qk5en3H+aIRyw== Original-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4CvCQL3l19z9rxR for ; Sun, 13 Dec 2020 19:16:26 +0100 (CET) Received-SPF: pass client-ip=185.67.36.66; envelope-from=tomasn@posteo.net; helo=mout02.posteo.de X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:195990 Archived-At: --=-=-= Content-Type: text/plain Hello Emacs Working with bug 40563 I had reason "demystify" the narrow-to-defun function and I think one can see a redundant check and call to beginning-of-defun there. Here is a walk-through of the beginning of the narrow-to-defun function to try and explain what I mean. ... (let ((opoint (point)) beg end) ;; Try first in this order for the sake of languages with nested ;; functions where several can end at the same place as with ;; the offside rule, e.g. Python. ;; Finding the start of the function is a bit problematic since ;; `beginning-of-defun' when we are on the first character of ;; the function might go to the previous function. ;; ;; Therefore we first move one character forward and then call ;; `beginning-of-defun'. However now we must check that we did ;; not move into the next function. (let ((here (point))) (unless (eolp) (forward-char)) forward-char to allow narrowing with point on the entry of a defun (allow the beginning-of-defun function to find the beginning of /this/ defun). (beginning-of-defun) (when (< (point) here) This test pass in all cases but the case when point was on the entry of a defun at calling narrow-to-defun. (goto-char here) (beginning-of-defun))) And then beginning-of-defun function is called again, with point on 'here', from where we started. (setq beg (point)) beg is now possibly the result from the second call to beginning-of-defun. In the case of lisp defuns (defuns starting at beginning-of-line) this is no problem, only redundant I would claim (the test for (< (point) here) and the additional call to beginning-of-defun). With a language with the mentioned "offside rule", like Python, it might be a problem. If the intention was to narrow a nested function with point on the the first character of the defun, the following happens, (| is point) class A(): |def a1(self): pass The forward-char call allow beginning-of-defun function to find the beginning of a1. If not elsewhere, the beginning-of-defun function in lisp.el finalize with beginning-of-line, so we get class A(): | def a1(self): pass and that means (< (point) here) and beginning-of-defun is called again, this time from the beginning of def ('here') and no forward-char support, ending up at the beginning of class A. Other funny things can happen but I stop there. It is not a problem with lisp because a defun start at beginning-of-line. In any case, I cannot see what problem the forward-char could cause, and the protection for it. The nested let with the forward-char service was introduced with commit 050cc68b402f5998193a6026d0eeeecb9d2cb9c4 Author: Lennart Borgman Date: Wed Apr 11 04:12:20 2012 +0200 `narrow-to-defun' fixup * emacs-lisp/lisp.el (narrow-to-defun): `beginning-of-defun' goes to previous function when point is on the first character of a function. Take care of that in `narrow-to-defun'. Fixes: debbugs:6157 and in the discussion about it (bug 6157) I cannot see an example of where it would be a problem. The comment is saying we must check we didn't end up in the next function, but that's not what is checked in that 'when' test as far as I understand. With the attached patch I suggest a partial revert of the above commit, keeping only the forward-char service. I have been experimenting with it a lot and see no problem. I made 'make check' and see no failures FWIW. As a bonus, in combination with a patch I submitted with bug#40563, python nested defuns can be narrowed. What do you think, maybe I am missing something. Or do you agree the check and the additional call is redundant? GNU Emacs 28.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.5, cairo version 1.16.0) of 2020-12-12 lisp.el(c) commit 8ace7700b9 Best regards -- Tomas --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=narrow-to-defun.patch diff --git a/lisp/emacs-lisp/lisp.el b/lisp/emacs-lisp/lisp.el index 124900168c..83c20933b3 100644 --- a/lisp/emacs-lisp/lisp.el +++ b/lisp/emacs-lisp/lisp.el @@ -659,15 +659,10 @@ narrow-to-defun ;; the function might go to the previous function. ;; ;; Therefore we first move one character forward and then call - ;; `beginning-of-defun'. However now we must check that we did - ;; not move into the next function. - (let ((here (point))) - (unless (eolp) - (forward-char)) - (beginning-of-defun) - (when (< (point) here) - (goto-char here) - (beginning-of-defun))) + ;; `beginning-of-defun'. + (unless (eolp) + (forward-char)) + (beginning-of-defun) (setq beg (point)) (end-of-defun) (setq end (point)) --=-=-=--