From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: David Kastrup Newsgroups: gmane.lisp.guile.bugs Subject: bug#22630: Bad comment Date: Tue, 11 Oct 2016 11:03:11 +0200 Message-ID: <87pon79s1c.fsf@fencepost.gnu.org> References: <1455190308-16788-1-git-send-email-dak@gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: blaine.gmane.org 1476176666 13156 195.159.176.226 (11 Oct 2016 09:04:26 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Tue, 11 Oct 2016 09:04:26 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) To: 22630@debbugs.gnu.org Original-X-From: bug-guile-bounces+guile-bugs=m.gmane.org@gnu.org Tue Oct 11 11:04:19 2016 Return-path: Envelope-to: guile-bugs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1btsyq-0001p0-Ab for guile-bugs@m.gmane.org; Tue, 11 Oct 2016 11:04:12 +0200 Original-Received: from localhost ([::1]:54477 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btsyo-0004Td-VZ for guile-bugs@m.gmane.org; Tue, 11 Oct 2016 05:04:11 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:56669) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btsyi-0004TM-N8 for bug-guile@gnu.org; Tue, 11 Oct 2016 05:04:05 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1btsyg-0001rE-K6 for bug-guile@gnu.org; Tue, 11 Oct 2016 05:04:03 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:45522) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btsyg-0001r9-Gg for bug-guile@gnu.org; Tue, 11 Oct 2016 05:04:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1btsyg-0000he-A7 for bug-guile@gnu.org; Tue, 11 Oct 2016 05:04:02 -0400 X-Loop: help-debbugs@gnu.org In-Reply-To: <1455190308-16788-1-git-send-email-dak@gnu.org> Resent-From: David Kastrup Original-Sender: "Debbugs-submit" Resent-CC: bug-guile@gnu.org Resent-Date: Tue, 11 Oct 2016 09:04:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 22630 X-GNU-PR-Package: guile X-GNU-PR-Keywords: patch Original-Received: via spool by 22630-submit@debbugs.gnu.org id=B22630.14761766012652 (code B ref 22630); Tue, 11 Oct 2016 09:04:02 +0000 Original-Received: (at 22630) by debbugs.gnu.org; 11 Oct 2016 09:03:21 +0000 Original-Received: from localhost ([127.0.0.1]:51712 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1btsy0-0000gg-Sz for submit@debbugs.gnu.org; Tue, 11 Oct 2016 05:03:21 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:37322) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1btsxy-0000gU-UZ for 22630@debbugs.gnu.org; Tue, 11 Oct 2016 05:03:19 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1btsxs-0001Tr-VE for 22630@debbugs.gnu.org; Tue, 11 Oct 2016 05:03:13 -0400 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:34298 helo=lola.localdomain) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btsxs-0001TJ-Kx for 22630@debbugs.gnu.org; Tue, 11 Oct 2016 05:03:12 -0400 Original-Received: by lola.localdomain (Postfix, from userid 1000) id EE629E112C; Tue, 11 Oct 2016 11:03:11 +0200 (CEST) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-guile@gnu.org List-Id: "Bug reports for GUILE, GNU's Ubiquitous Extension Language" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guile-bounces+guile-bugs=m.gmane.org@gnu.org Original-Sender: "bug-guile" Xref: news.gmane.org gmane.lisp.guile.bugs:8424 Archived-At: I routinely rebase before abandoning my patches so it only now caught my attention that this bug fix was committed with changed comments. Unfortunately, one of those comments was changed for the worse. Here is the respective merge conflict: <<<<<<< 4f324684cc7672b39800bce0a373da10057dc3c6 /* In Guile, `assv' is the same as `assq' for keys of all types except numbers. */ ======= /* Non-immediate numbers are the only keys we need to check * other than with eq */ >>>>>>> Let assv/assoc shortcircuit to assq where feasible if (!SCM_NUMP (key)) return scm_sloppy_assq (key, alist); The first of the comments is the one actually in the repository. Apart from the difference in formatting (which is fine) it features a different in content that is extremely misleading. SCM_NUMP indeed checks for _nonimmediate_ numbers (as opposed to the more inclusive SCM_NUMBERP). Since this comment concerns a _very_ deliberate and nontrivial optimization and since the name of the macro SCM_NUMP is fabulously misleading (which apparently triggered the comment change in the first place), I'd consider it important to fix the comment. It might even be an idea to replace the condition with if (SCM_IMP (key) || !SCM_NUMBERP (key)) (which yields an ugly macro expansion but the compiler might optimize it) or if (SCM_IMP (key) || !SCM_NUMP (key)) both of which are sort of nonsensical to the compiler (the latter more so than the former) but might be less confusing to the human reader. Or find a better name for SCM_NUMP in the first place. At any rate, the minimally invasive option would be to restore the original comment albeit with typographic fixes, namely writing `eq?' instead of just eq. -- David Kastrup