Preventing warnings in FIXNUM_OVERFLOW_P

Preventing warnings in FIXNUM_OVERFLOW_P

[Richard Stallman]

Ian Lance Taylor <iant@google.com> wrote:

    You can avoid it by using unsigned types.  I think that something like
    this will do the trick:

    #define FIXNUM_OVERFLOW_P(i)					\
      ((unsigned long long)(i) > MOST_POSITIVE_FIXNUM		\
       && (unsigned long long)(i) < MOST_NEGATIVE_FIXNUM)

Would someone please give that approach a try and see if it works?  I
am having too much trouble with concentration right now to see whether
that code is correct -- it might need somewhat more change than that
in order to get the comparisons right in an unsigned type.

Please ack with the results.

Re: Preventing warnings in FIXNUM_OVERFLOW_P

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

> Ian Lance Taylor <iant@google.com> wrote:
>
>     You can avoid it by using unsigned types.  I think that something like
>     this will do the trick:
>
>     #define FIXNUM_OVERFLOW_P(i)					\
>       ((unsigned long long)(i) > MOST_POSITIVE_FIXNUM		\
>        && (unsigned long long)(i) < MOST_NEGATIVE_FIXNUM)
>
> Would someone please give that approach a try and see if it works?  I
> am having too much trouble with concentration right now to see whether
> that code is correct -- it might need somewhat more change than that
> in order to get the comparisons right in an unsigned type.

unsigned long long is neither guaranteed to exist on all supported
architectures, nor guaranteed to be longer than long.

It seems to me like something along the lines of

#define FIXNUM_OVERFLOW_P(i) \
  (((unsigned long)(i)-(unsigned long)MOST_NEGATIVE_FIXNUM) > \
    (unsigned long)MOST_POSITIVE_FIXNUM)

should do the trick.  A few more casts than strictly necessary, but in
that manner there is a chance that the compiler will not complain.  I
am assuming here that the natural type of i is (long int), if it isn't
adapt the casts accordingly.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Preventing warnings in FIXNUM_OVERFLOW_P

[Stefan Monnier]

>> You can avoid it by using unsigned types.  I think that something like
>> this will do the trick:
>> 
>> #define FIXNUM_OVERFLOW_P(i)					\
>> ((unsigned long long)(i) > MOST_POSITIVE_FIXNUM		\
>> && (unsigned long long)(i) < MOST_NEGATIVE_FIXNUM)
>> 
>> Would someone please give that approach a try and see if it works?  I
>> am having too much trouble with concentration right now to see whether
>> that code is correct -- it might need somewhat more change than that
>> in order to get the comparisons right in an unsigned type.

> unsigned long long is neither guaranteed to exist on all supported
> architectures, nor guaranteed to be longer than long.

> It seems to me like something along the lines of

> #define FIXNUM_OVERFLOW_P(i) \
>   (((unsigned long)(i)-(unsigned long)MOST_NEGATIVE_FIXNUM) > \
>     (unsigned long)MOST_POSITIVE_FIXNUM)

That might be OK if we're trying to obfuscate the code.
Eli's current workaround is still the least bad I've seen.


        Stefan

Re: Preventing warnings in FIXNUM_OVERFLOW_P

[Richard Stallman]

    Eli's current workaround is still the least bad I've seen.

A GCC developer says that the warning does not happen for unsigned
types.  That means that even if GCC gets smarter, it won't give that
warning if we use an unsigned type.

Re: Preventing warnings in FIXNUM_OVERFLOW_P

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Date: Fri, 19 Jan 2007 05:42:53 -0500
> Cc: emacs-devel@gnu.org
> 
>     Eli's current workaround is still the least bad I've seen.
> 
> A GCC developer says that the warning does not happen for unsigned
> types.  That means that even if GCC gets smarter, it won't give that
> warning if we use an unsigned type.

But this modification of the macro causes it to look suspicious, as it
compares an unsigned variable with a signed (and negative) number.  I
don't think it's a good idea to modify our code to the point that an
experienced C programmer (such as me and you) can no longer be sure
that the code is correct.

By contrast, my workaround doesn't obfuscate the code, and the
extra assignment should be optimized out of existence under -O2.

I say let's leave my workaround alone until the GCC developers move
this warning into -Wextra.  When that version of GCC becomes
widespread, we can remove the workaround.

Re: Preventing warnings in FIXNUM_OVERFLOW_P

[Stefan Monnier]

> By contrast, my workaround doesn't obfuscate the code, and the
> extra assignment should be optimized out of existence under -O2.

Indeed.  That's why it's the least bad.  It makes the code slightly
heavier, but at least it's crystal clear that it doesn't change
the semantics.

> I say let's leave my workaround alone until the GCC developers move
> this warning into -Wextra.  When that version of GCC becomes
> widespread, we can remove the workaround.

Agreed,


        Stefan

Re: Preventing warnings in FIXNUM_OVERFLOW_P

[Richard Stallman]

    > I say let's leave my workaround alone until the GCC developers move
    > this warning into -Wextra.  When that version of GCC becomes
    > widespread, we can remove the workaround.

    Agreed,

That is a bad solution.  Turning off this warning globally risks
missing other places where there is a real problem.  The right
solution is a way to turn off the warning _only in this construct_.

Re: Preventing warnings in FIXNUM_OVERFLOW_P

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> CC: eliz@gnu.org, emacs-devel@gnu.org
> Date: Fri, 19 Jan 2007 21:10:44 -0500
> 
>     > I say let's leave my workaround alone until the GCC developers move
>     > this warning into -Wextra.  When that version of GCC becomes
>     > widespread, we can remove the workaround.
> 
>     Agreed,
> 
> That is a bad solution.  Turning off this warning globally risks
> missing other places where there is a real problem.  The right
> solution is a way to turn off the warning _only in this construct_.

But this argument is applicable to _every_ warning GCC is programmed
to emit! including those that are already in -Wall and -Wextra, about
which you yourself argued in the past that they should not be issued
by default.

Every warning can sometimes flag code that is a real problem, that's
the reason we have those warnings in the first place.  If we are to
accept your argument, why not use -Wall globally, and then go over the
warnings and turn each one only in those places where we know the code
is correct?

Re: Preventing warnings in FIXNUM_OVERFLOW_P

[Richard Stallman]

    > That is a bad solution.  Turning off this warning globally risks
    > missing other places where there is a real problem.  The right
    > solution is a way to turn off the warning _only in this construct_.

    But this argument is applicable to _every_ warning GCC is programmed
    to emit!

For most kinds of warnings, there is a way you can avoid the warning
by writing the code carefully.  I think there should be a convenient
way to avoid this warning too.

Re: Preventing warnings in FIXNUM_OVERFLOW_P

[Richard Stallman]

    >     #define FIXNUM_OVERFLOW_P(i)					\
    >       ((unsigned long long)(i) > MOST_POSITIVE_FIXNUM		\
    >        && (unsigned long long)(i) < MOST_NEGATIVE_FIXNUM)
    >
    > Would someone please give that approach a try and see if it works?  I
    > am having too much trouble with concentration right now to see whether
    > that code is correct -- it might need somewhat more change than that
    > in order to get the comparisons right in an unsigned type.

    unsigned long long is neither guaranteed to exist on all supported
    architectures, nor guaranteed to be longer than long.

`unsigned long long' does always exist, when you are using GCC.  The
definition could be conditional on __GNUC__.

As for the whether `unsigned long long' may not be longer than `long',
why does that matter?

Re: Preventing warnings in FIXNUM_OVERFLOW_P

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Date: Thu, 18 Jan 2007 12:27:57 -0500
> 
> Ian Lance Taylor <iant@google.com> wrote:
> 
>     You can avoid it by using unsigned types.  I think that something like
>     this will do the trick:
> 
>     #define FIXNUM_OVERFLOW_P(i)					\
>       ((unsigned long long)(i) > MOST_POSITIVE_FIXNUM		\
>        && (unsigned long long)(i) < MOST_NEGATIVE_FIXNUM)
> 
> Would someone please give that approach a try and see if it works?

It seems to work.

> I am having too much trouble with concentration right now to see
> whether that code is correct

Which is precisely the problem with this solution: it is no longer
clear that the code does what we want, since mixing signed with
unsigned produces confusing code and is generally asking for trouble.

And then there's what David said about long long: if we use the above,
we will need to have two versions of the macro: one for GCC, the other
for other compilers.

So on balance, I think this solution is worse than my workaround.

It sounds like GCC maintainers are working on fixing this in a future
version.  So I'd suggest to leave this alone until then.

Re: Preventing warnings in FIXNUM_OVERFLOW_P

[Richard Stallman]

    It sounds like GCC maintainers are working on fixing this in a future
    version.  So I'd suggest to leave this alone until then.

Could you explain what that refers to?  I do not know.

Re: Preventing warnings in FIXNUM_OVERFLOW_P

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> CC: emacs-devel@gnu.org
> Date: Fri, 19 Jan 2007 05:43:07 -0500
> 
>     It sounds like GCC maintainers are working on fixing this in a future
>     version.  So I'd suggest to leave this alone until then.
> 
> Could you explain what that refers to?  I do not know.

I was referring to this part of the response by Manuel López-Ibáñez in
the thread you started on the GCC mailing list:

> >  We are working on fixing those for GCC 4.3 :-)

See http://gcc.gnu.org/ml/gcc/2007-01/msg00808.html for the message
from which I quoted this.

In addition, an earlier message posted by Manuel:

  http://gcc.gnu.org/ml/gcc/2007-01/msg00480.html

seems to say that he is working on putting the warning that gave us
trouble into -Wextra, which is fine by me (since -Wextra groups
warnings that are _supposed_ to be nitpicking).

Re: Preventing warnings in FIXNUM_OVERFLOW_P

[Richard Stallman]

    I was referring to this part of the response by Manuel López-Ibáñez in
    the thread you started on the GCC mailing list:

    > >  We are working on fixing those for GCC 4.3 :-)

That message is so vague I had to respond by asking him what he means
by it.