From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.bugs Subject: bug#36370: 27.0.50; XFIXNAT called on negative numbers Date: Thu, 27 Jun 2019 12:38:10 -0700 Organization: UCLA Computer Science Department Message-ID: <7ef599ae-0a1d-e86f-2bed-a1503455833f@cs.ucla.edu> References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------DCEB8734CF7935EB7423EB85" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="129921"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 Cc: 36370-done@debbugs.gnu.org To: Pip Cet Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu Jun 27 21:39:16 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1hgaEl-000XgU-O0 for geb-bug-gnu-emacs@m.gmane.org; Thu, 27 Jun 2019 21:39:15 +0200 Original-Received: from localhost ([::1]:53920 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hgaEk-0006wY-PS for geb-bug-gnu-emacs@m.gmane.org; Thu, 27 Jun 2019 15:39:14 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:34843) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hgaEe-0006wN-8q for bug-gnu-emacs@gnu.org; Thu, 27 Jun 2019 15:39:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hgaEc-0003wv-T4 for bug-gnu-emacs@gnu.org; Thu, 27 Jun 2019 15:39:08 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:54462) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hgaEc-0003wh-Ls for bug-gnu-emacs@gnu.org; Thu, 27 Jun 2019 15:39:06 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hgaEc-0001fZ-H4 for bug-gnu-emacs@gnu.org; Thu, 27 Jun 2019 15:39:06 -0400 Resent-From: Paul Eggert Original-Sender: "Debbugs-submit" Resent-To: bug-gnu-emacs@gnu.org Resent-Date: Thu, 27 Jun 2019 19:39:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: cc-closed 36370 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Mail-Followup-To: 36370@debbugs.gnu.org, eggert@cs.ucla.edu, pipcet@gmail.com Original-Received: via spool by 36370-done@debbugs.gnu.org id=D36370.15616643046328 (code D ref 36370); Thu, 27 Jun 2019 19:39:02 +0000 Original-Received: (at 36370-done) by debbugs.gnu.org; 27 Jun 2019 19:38:24 +0000 Original-Received: from localhost ([127.0.0.1]:39767 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hgaDv-0001dz-My for submit@debbugs.gnu.org; Thu, 27 Jun 2019 15:38:24 -0400 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:35762) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hgaDt-0001dl-E7 for 36370-done@debbugs.gnu.org; Thu, 27 Jun 2019 15:38:22 -0400 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 44694161D0F; Thu, 27 Jun 2019 12:38:15 -0700 (PDT) Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id XKkj5ZAfPbXi; Thu, 27 Jun 2019 12:38:14 -0700 (PDT) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 45638161D54; Thu, 27 Jun 2019 12:38:14 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Original-Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id AUf5GYxEpo9H; Thu, 27 Jun 2019 12:38:14 -0700 (PDT) Original-Received: from Penguin.CS.UCLA.EDU (Penguin.CS.UCLA.EDU [131.179.64.200]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 24E49161D0F; Thu, 27 Jun 2019 12:38:14 -0700 (PDT) Openpgp: preference=signencrypt Autocrypt: addr=eggert@cs.ucla.edu; prefer-encrypt=mutual; keydata= xsFNBEyAcmQBEADAAyH2xoTu7ppG5D3a8FMZEon74dCvc4+q1XA2J2tBy2pwaTqfhpxxdGA9 Jj50UJ3PD4bSUEgN8tLZ0san47l5XTAFLi2456ciSl5m8sKaHlGdt9XmAAtmXqeZVIYX/UFS 96fDzf4xhEmm/y7LbYEPQdUdxu47xA5KhTYp5bltF3WYDz1Ygd7gx07Auwp7iw7eNvnoDTAl KAl8KYDZzbDNCQGEbpY3efZIvPdeI+FWQN4W+kghy+P6au6PrIIhYraeua7XDdb2LS1en3Ss mE3QjqfRqI/A2ue8JMwsvXe/WK38Ezs6x74iTaqI3AFH6ilAhDqpMnd/msSESNFt76DiO1ZK QMr9amVPknjfPmJISqdhgB1DlEdw34sROf6V8mZw0xfqT6PKE46LcFefzs0kbg4GORf8vjG2 Sf1tk5eU8MBiyN/bZ03bKNjNYMpODDQQwuP84kYLkX2wBxxMAhBxwbDVZudzxDZJ1C2VXujC OJVxq2kljBM9ETYuUGqd75AW2LXrLw6+MuIsHFAYAgRr7+KcwDgBAfwhPBYX34nSSiHlmLC+ KaHLeCLF5ZI2vKm3HEeCTtlOg7xZEONgwzL+fdKo+D6SoC8RRxJKs8a3sVfI4t6CnrQzvJbB n6gxdgCu5i29J1QCYrCYvql2UyFPAK+do99/1jOXT4m2836j1wARAQABzSBQYXVsIEVnZ2Vy dCA8ZWdnZXJ0QGNzLnVjbGEuZWR1PsLBfgQTAQIAKAUCTIByZAIbAwUJEswDAAYLCQgHAwIG FQgCCQoLBBYCAwECH In-Reply-To: Content-Language: en-US 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: 209.51.188.43 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.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:161650 Archived-At: This is a multi-part message in MIME format. --------------DCEB8734CF7935EB7423EB85 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit >> if it were up to me we'd get rid of XFIXNAT entirely, >> and just use XFIXNUM. But old habits die hard.... > > I actually think that would be best, so we're only disagreeing about > what the second-best solution is :-) Removing XFIXNAT would be outside the scope of this patch. However, if we're already fixing the code for some other reason and if the XFIXNATs are confusing that code, we might as well replace them with XFIXNUMs. The attached patch does that. > valid_image_p only returns true if ->valid_p returns true, which only > happens when parse_image_spec returns true, which only happens if > :ascent is not present, Qcenter, or a fixnum in the 0..100 range. Thanks for explaining. The attached patch removes the unnecessary range checks that I proposed. > My idea is to define eassume as follows: > > #define eassume(cond) (__builtin_constant_p (!(cond) == !(cond)) ? > ((cond) ? 0 : __builtin_unreachable ()) : 0) That would generate worse code in some cases, since after (say) "eassume (i >= 0); return i/2;" where i is a variable, GCC would not be able to optimize i/2 into i>>1 because GCC would not know that i is nonnegative. The main point of eassume (as opposed to eassert) is to enable optimizations like that. > think this code is a bit hard to read: The attached patch changes that to use XFIXNUM instead of XFIXNAT, along the lines discussed above. Thanks for the review. In addition to the already-mentioned patches, I installed the attached and am closing the bug report. --------------DCEB8734CF7935EB7423EB85 Content-Type: text/x-patch; name="0001-Omit-a-few-minor-unnecessary-range-checks.patch" Content-Disposition: attachment; filename="0001-Omit-a-few-minor-unnecessary-range-checks.patch" Content-Transfer-Encoding: quoted-printable >From 7219f991a94015e06851d54b77acb4b1161749a5 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 27 Jun 2019 12:31:27 -0700 Subject: [PATCH] Omit a few minor unnecessary range checks MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit Based on Pip Cet=E2=80=99s review (Bug#36370#19). * src/fileio.c (Fdo_auto_save): * src/image.c (lookup_image): * src/textprop.c (Fnext_single_char_property_change): Prefer XFIXNUM to XFIXNAT for clarity and consistency with neighboring code, and to avoid unnecessary range checks. * src/image.c (lookup_image): Omit unnecessary range checks. --- src/fileio.c | 2 +- src/image.c | 12 ++++++------ src/textprop.c | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/fileio.c b/src/fileio.c index 61e10dac47..ed1d2aedf3 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -5852,7 +5852,7 @@ DEFUN ("do-auto-save", Fdo_auto_save, Sdo_auto_save= , 0, 2, "", spare the user annoying messages. */ && XFIXNUM (BVAR (b, save_length)) > 5000 && (growth_factor * (BUF_Z (b) - BUF_BEG (b)) - < (growth_factor - 1) * XFIXNAT (BVAR (b, save_length))) + < (growth_factor - 1) * XFIXNUM (BVAR (b, save_length))) /* These messages are frequent and annoying for `*mail*'. */ && !NILP (BVAR (b, filename)) && NILP (no_message)) diff --git a/src/image.c b/src/image.c index bbf25b4d58..f3d6508f46 100644 --- a/src/image.c +++ b/src/image.c @@ -2385,18 +2385,18 @@ lookup_image (struct frame *f, Lisp_Object spec) #endif =20 ascent =3D image_spec_value (spec, QCascent, NULL); - if (RANGED_FIXNUMP (0, ascent, INT_MAX)) - img->ascent =3D XFIXNAT (ascent); + if (FIXNUMP (ascent)) + img->ascent =3D XFIXNUM (ascent); else if (EQ (ascent, Qcenter)) img->ascent =3D CENTERED_IMAGE_ASCENT; =20 margin =3D image_spec_value (spec, QCmargin, NULL); - if (RANGED_FIXNUMP (0, margin, INT_MAX)) - img->vmargin =3D img->hmargin =3D XFIXNAT (margin); + if (FIXNUMP (margin)) + img->vmargin =3D img->hmargin =3D XFIXNUM (margin); else if (CONSP (margin)) { - img->hmargin =3D XFIXNAT (XCAR (margin)); - img->vmargin =3D XFIXNAT (XCDR (margin)); + img->hmargin =3D XFIXNUM (XCAR (margin)); + img->vmargin =3D XFIXNUM (XCDR (margin)); } =20 relief =3D image_spec_value (spec, QCrelief, NULL); diff --git a/src/textprop.c b/src/textprop.c index 3026ec7e99..9023f4efa0 100644 --- a/src/textprop.c +++ b/src/textprop.c @@ -799,10 +799,10 @@ DEFUN ("next-single-char-property-change", Fnext_si= ngle_char_property_change, else CHECK_FIXNUM_COERCE_MARKER (limit); =20 - if (XFIXNAT (position) >=3D XFIXNUM (limit)) + if (XFIXNUM (position) >=3D XFIXNUM (limit)) { position =3D limit; - if (XFIXNAT (position) > ZV) + if (XFIXNUM (position) > ZV) XSETFASTINT (position, ZV); } else --=20 2.21.0 --------------DCEB8734CF7935EB7423EB85--