emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Preview: portable dumper


From: Paul Eggert
Subject: Re: Preview: portable dumper
Date: Fri, 16 Feb 2018 13:49:43 -0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0

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 where -Wconversion is more likely to mess up than usual, which is why I suggested disabling -Wconversion there in pdumper.c. -Wconversion is not as useful elsewhere, which is why I suggested leaving INTEGER_TO_CONS alone.

, -Wconversion ought to warn only on things that are actually problems. Have a counter-example

Sure, revert the pdumper change to INTEGER_TO_CONS but leave pdumper.c alone, by applying the attached patch to pdumper. The compile on a platform where EMACS_INT is 32 bits, using GCC 7.3.1 20180130 (Red Hat 7.3.1-2). The resulting diagnostic is a false alarm:

pdumper.c: In function ‘intmax_t_to_lisp’:
pdumper.c:694:29: error: conversion to ‘EMACS_INT {aka int}’ from ‘intmax_t {aka long long int}’ may alter its value [-Werror=conversion]
     return INTEGER_TO_CONS (value);  \
                             ^
lisp.h:3572:19: note: in definition of macro ‘INTEGER_TO_CONS’
    ? make_number (i)          \
                   ^
pdumper.c:698:1: note: in expansion of macro ‘DEFINE_TOLISP_FUNC’
 DEFINE_TOLISP_FUNC (intmax_t_to_lisp, intmax_t);
 ^~~~~~~~~~~~~~~~~~

This false alarm is due to a GCC bug. GCC is supposed to take expressions like this one (adapted from INTEGER_TO_CONS):

  (MOST_NEGATIVE_FIXNUM <= i && i <= MOST_POSITIVE_FIXNUM
   ? make_number (i)
   : something_else (i))

and evaluate make_number (i) in the context of i being in range for fixnums (which means i is in range for EMACS_INT). In this particular 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 is that they don't take -Wconversion bug reports all that seriously. In practice, -Wconversion isn't good enough for high-quality code like what Emacs should be.


-Wconversion helped me find real bugs and otherwise mysterious bugs in pdumper.
Yes, -Wconversion can find bugs, particularly the first time one writes a program when the program has lots of bugs that need finding. But its false-alarm rate is too high to be useful in high-quality code. Although it's OK to use -Wconversion temporarily in new or otherwise-buggy areas, we shouldn't encourage its use in the part of Emacs that is intended to be stable and high-quality.

Attachment: pdumper.diff
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]