[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Suspicious warning in W64 build
From: |
Eli Zaretskii |
Subject: |
Re: Suspicious warning in W64 build |
Date: |
Sun, 17 Sep 2017 17:31:00 +0300 |
> Cc: address@hidden, address@hidden
> From: Paul Eggert <address@hidden>
> Date: Sun, 17 Sep 2017 00:01:09 -0700
>
> Why was the attached patch needed? What warning did it suppress?
I wrote about that in
http://lists.gnu.org/archive/html/emacs-devel/2017-09/msg00448.html
The warning is this:
../../emacs/src/data.c: In function 'minmax_driver':
../../emacs/src/data.c:3022:9: warning: 'accum.i' may be used uninitialized
in this function [-Wmaybe-uninitialized]
return accum;
^~~~~
Which seems to mean that even eassume is sometimes not enough to
convince GCC 7 that the code is correct:
static Lisp_Object
minmax_driver (ptrdiff_t nargs, Lisp_Object *args,
enum Arith_Comparison comparison)
{
eassume (0 < nargs); <<<<<<<<<<<<<<<<<<<<<<<
Lisp_Object accum;
for (ptrdiff_t argnum = 0; argnum < nargs; argnum++)
{
Lisp_Object val = args[argnum];
CHECK_NUMBER_OR_FLOAT_COERCE_MARKER (val);
if (argnum == 0 || !NILP (arithcompare (val, accum, comparison)))
accum = val;
else if (FLOATP (accum) && isnan (XFLOAT_DATA (accum)))
return accum;
}
return accum;
}
Since nargs > 0, the loop is always entered, but GCC seems to miss that.
> As you may recall, I prefer using UNINIT to suppress uninitialized-variable
> warnings because it is more automatable (e.g., it is easier to automate
> removing it later once GCC gets fixed). If UNINIT does not work I would like
> to know why.
UNINIT looked inappropriate to initialize a Lisp_Object which is
supposed to be a number. Even initializing with Qnil didn't look
right to me (it could raise some brows), and using UNINIT is only
equivalent to Qnil under the current representation of Qnil, and so is
insufficiently future-proof. E.g., it could become an invalid Lisp
object if we decide to change representation at some future point. By
contrast, args[0] was available, of the right data type, and the
comment I added makes it clear what was the reason for the
initialization.
So using args[0] sounds like a better solution here than UNINIT. In
any case, it's a valid solution.
- Re: Suspicious warning in W64 build, (continued)
- Re: Suspicious warning in W64 build, Andy Moreton, 2017/09/14
- Re: Suspicious warning in W64 build, Fabrice Popineau, 2017/09/15
- Re: Suspicious warning in W64 build, Eli Zaretskii, 2017/09/15
- Re: Suspicious warning in W64 build, Fabrice Popineau, 2017/09/15
- Re: Suspicious warning in W64 build, Eli Zaretskii, 2017/09/15
- Re: Suspicious warning in W64 build, Fabrice Popineau, 2017/09/15
- Re: Suspicious warning in W64 build, Eli Zaretskii, 2017/09/15
- Re: Suspicious warning in W64 build, Fabrice Popineau, 2017/09/15
- Re: Suspicious warning in W64 build, Eli Zaretskii, 2017/09/16
- Re: Suspicious warning in W64 build, Paul Eggert, 2017/09/17
- Re: Suspicious warning in W64 build,
Eli Zaretskii <=
- Re: Suspicious warning in W64 build, Philipp Stephani, 2017/09/17
- Re: Suspicious warning in W64 build, Paul Eggert, 2017/09/17
- Re: Suspicious warning in W64 build, Paul Eggert, 2017/09/17
- Re: Suspicious warning in W64 build, Eli Zaretskii, 2017/09/17
- Re: Suspicious warning in W64 build, Paul Eggert, 2017/09/17
- Re: Suspicious warning in W64 build, Eli Zaretskii, 2017/09/17
- Re: Suspicious warning in W64 build, Paul Eggert, 2017/09/17
- Re: Suspicious warning in W64 build, Eli Zaretskii, 2017/09/17
- Re: Suspicious warning in W64 build, Paul Eggert, 2017/09/18
- Re: Suspicious warning in W64 build, Eli Zaretskii, 2017/09/18