From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: Re: Preview: portable dumper Date: Fri, 16 Feb 2018 13:49:43 -0800 Organization: UCLA Computer Science Department Message-ID: <9a292a77-f443-3db1-3388-3716a34b4816@cs.ucla.edu> References: <128341f5-4bc1-4c03-980c-c7b2b7e7776e@email.android.com> <2a161cd5-4293-1ccb-9fa8-ba604b83ba4a@cs.ucla.edu> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------461B3484712FAEFA2658CD40" X-Trace: blaine.gmane.org 1518817689 29354 195.159.176.226 (16 Feb 2018 21:48:09 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Fri, 16 Feb 2018 21:48:09 +0000 (UTC) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 Cc: Andy Moreton , emacs-devel@gnu.org To: Daniel Colascione , Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Feb 16 22:48:04 2018 Return-path: Envelope-to: ged-emacs-devel@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 1emnrH-0006i4-10 for ged-emacs-devel@m.gmane.org; Fri, 16 Feb 2018 22:47:55 +0100 Original-Received: from localhost ([::1]:36394 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1emntH-0003Cv-B5 for ged-emacs-devel@m.gmane.org; Fri, 16 Feb 2018 16:49:59 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:39357) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1emnt9-0003BO-0d for emacs-devel@gnu.org; Fri, 16 Feb 2018 16:49:52 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1emnt7-0003st-WA for emacs-devel@gnu.org; Fri, 16 Feb 2018 16:49:51 -0500 Original-Received: from zimbra.cs.ucla.edu ([131.179.128.68]:56430) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1emnt4-0003rT-5X; Fri, 16 Feb 2018 16:49:46 -0500 Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id B7D051615AF; Fri, 16 Feb 2018 13:49:44 -0800 (PST) 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 ho0TSXE4h7Wh; Fri, 16 Feb 2018 13:49:43 -0800 (PST) Original-Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id B86DE1615B2; Fri, 16 Feb 2018 13:49:43 -0800 (PST) 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 XWkTn4cM8B87; Fri, 16 Feb 2018 13:49:43 -0800 (PST) Original-Received: from Penguin.CS.UCLA.EDU (Penguin.CS.UCLA.EDU [131.179.64.200]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 977771615AC; Fri, 16 Feb 2018 13:49:43 -0800 (PST) In-Reply-To: Content-Language: en-US X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 131.179.128.68 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:222849 Archived-At: This is a multi-part message in MIME format. --------------461B3484712FAEFA2658CD40 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable On 02/16/2018 01:23 PM, Daniel Colascione wrote: > Outside of certain specialized casting facilities DEFINE_TOLISP_FUNC is exactly the sort of specialized casting facility=20 where -Wconversion is more likely to mess up than usual, which is why I=20 suggested disabling -Wconversion there in pdumper.c. -Wconversion is not=20 as useful elsewhere, which is why I suggested leaving INTEGER_TO_CONS alo= ne. > , -Wconversion ought to warn only on things that are actually=20 > problems. Have a counter-example Sure, revert the pdumper change to INTEGER_TO_CONS but leave pdumper.c=20 alone, by applying the attached patch to pdumper. The compile on a=20 platform where EMACS_INT is 32 bits, using GCC 7.3.1 20180130 (Red Hat=20 7.3.1-2). The resulting diagnostic is a false alarm: pdumper.c: In function =E2=80=98intmax_t_to_lisp=E2=80=99: pdumper.c:694:29: error: conversion to =E2=80=98EMACS_INT {aka int}=E2=80= =99 from=20 =E2=80=98intmax_t {aka long long int}=E2=80=99 may alter its value [-Werr= or=3Dconversion] =C2=A0=C2=A0=C2=A0=C2=A0 return INTEGER_TO_CONS (value);=C2=A0 \ =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0 ^ lisp.h:3572:19: note: in definition of macro =E2=80=98INTEGER_TO_CONS=E2=80= =99 =C2=A0=C2=A0=C2=A0 ? make_number (i)=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 \ =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ^ pdumper.c:698:1: note: in expansion of macro =E2=80=98DEFINE_TOLISP_FUNC=E2= =80=99 =C2=A0DEFINE_TOLISP_FUNC (intmax_t_to_lisp, intmax_t); =C2=A0^~~~~~~~~~~~~~~~~~ This false alarm is due to a GCC bug. GCC is supposed to take=20 expressions like this one (adapted from INTEGER_TO_CONS): =C2=A0 (MOST_NEGATIVE_FIXNUM <=3D i && i <=3D MOST_POSITIVE_FIXNUM =C2=A0=C2=A0 ? make_number (i) =C2=A0=C2=A0 : something_else (i)) and evaluate make_number (i) in the context of i being in range for=20 fixnums (which means i is in range for EMACS_INT). In this particular=20 case, GCC has a bug where it forgets i's range, and thus the false alarm. I'd file a bug report with the GCC folks, but in the past my experience=20 is that they don't take -Wconversion bug reports all that seriously. In=20 practice, -Wconversion isn't good enough for high-quality code like what=20 Emacs should be. > -Wconversion helped me find real bugs and otherwise mysterious bugs in=20 > pdumper. Yes, -Wconversion can find bugs, particularly the first time one writes=20 a program when the program has lots of bugs that need finding. But its=20 false-alarm rate is too high to be useful in high-quality code. Although=20 it's OK to use -Wconversion temporarily in new or otherwise-buggy areas,=20 we shouldn't encourage its use in the part of Emacs that is intended to=20 be stable and high-quality. --------------461B3484712FAEFA2658CD40 Content-Type: text/x-patch; name="pdumper.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="pdumper.diff" diff --git a/src/lisp.h b/src/lisp.h index 45e8a0d791..6d4635f2a7 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -3568,9 +3568,9 @@ extern Lisp_Object arithcompare (Lisp_Object num1, Lisp_Object num2, itself, or a cons of two or three integers, or if all else fails a float. I should not have side effects. */ #define INTEGER_TO_CONS(i) \ - (! FIXNUM_OVERFLOW_P ((i)) \ - ? make_number ((EMACS_INT) (i)) \ - : EXPR_SIGNED ((i)) ? intbig_to_lisp ((i)) : uintbig_to_lisp ((i))) + (! FIXNUM_OVERFLOW_P (i) \ + ? make_number (i) \ + : EXPR_SIGNED (i) ? intbig_to_lisp (i) : uintbig_to_lisp (i)) extern Lisp_Object intbig_to_lisp (intmax_t); extern Lisp_Object uintbig_to_lisp (uintmax_t); --------------461B3484712FAEFA2658CD40--