Warnings when using % operator

M

Martin

Below is some sample code to illustrate two warnings I get using a Hitachi
compiler. When I use the GCC compiler I get a clean compile. I'd be grateful
if someone could explain what the warnings actually mean as they are rather
crytic, to me anyway.

When I change the type of LOOKUP_MAX from size_t to int, I get a clean
compile in the Hitachi compiler.

#include <stdio.h>
#define NELEMENTS(a) (sizeof(a) / sizeof(a[0]))

/* macro for portable modulo operations */
#if (-1 % 2 > 0)
#define IMOD(i, j) ((i) % (j))
#else
#define IMOD(i, j) (((i) % (j)) < 0 ? ((i) % (j)) + j : ((i) % (j)))
#endif

char *lookup[] =
{
"ABC",
"DEF",
"GHI",
"JKL"
};

const size_t LOOKUP_MAX = NELEMENTS(lookup);

#include <stdio.h>

int main( void )
{
int val;
/* code where user decides whether to increment or decrement value
removed */
/* ... */

/* User wants to increment value, keep in range [0, LOOKUP_MAX] */
val = IMOD(val+1, LOOKUP_MAX);

/* The above line of code generates two warnings:
* Unsigned compare always true/false
* Conditional expression always true/false.
*/
return 0; /* success */
}

Martin
http://martinobrien.co.uk/
 
B

Ben Pfaff

Martin said:
#define IMOD(i, j) (((i) % (j)) < 0 ? ((i) % (j)) + j : ((i) % (j)))
[...]

val = IMOD(val+1, LOOKUP_MAX);

/* The above line of code generates two warnings:
* Unsigned compare always true/false
* Conditional expression always true/false.
*/

When `a' and `b' are unsigned, `a % b' is always nonnegative.
Thus, ((i) % (j)) < 0 is always false in this case, hence the
first warning. That subexpression is the first part of a ?:
expression, hence the second warning.

This kind of situation is why GCC doesn't generate these warnings
by default. (You can enable the former with -W.)
 
T

Tim Rentsch

Martin said:
[...]

/* macro for portable modulo operations */
#if (-1 % 2 > 0)
#define IMOD(i, j) ((i) % (j))
#else
#define IMOD(i, j) (((i) % (j)) < 0 ? ((i) % (j)) + j : ((i) % (j)))
#endif

As other people have explained, the difficulty arose because the
modulo operation was done unsigned because the second operand was
unsigned.

This brings up an interesting challenge - how can IMOD be written
so that (1) it handles any combination of signed and unsigned
operands, and (2) doesn't generate compiler warnings like the
one mentioned in the earlier posting caused by comparing an
unsigned value to be less than zero? The desired semantics
are to return the smallest non-negative 'r' such that

i == m * j + r

with 'm' and 'r' being integers. For simplicity feel free to
assume that j != 0 and the value of 'r' can be represented as
a value of the type of 'i % j'.

By the way, am I right in thinking that in a standard-compliant
compilation the expression '-1 % 2 > 0' will always be zero (that
is, false)?
 
D

Derrick Coetzee

Tim said:
This brings up an interesting challenge - how can IMOD be written
so that (1) it handles any combination of signed and unsigned
operands, and (2) doesn't generate compiler warnings like the
one mentioned in the earlier posting caused by comparing an
unsigned value to be less than zero? The desired semantics
are to return the smallest non-negative 'r' such that

i == m * j + r

with 'm' and 'r' being integers. For simplicity feel free to
assume that j != 0 and the value of 'r' can be represented as
a value of the type of 'i % j'.

Neither assumption is needed: 0 <= r < |j|, and % takes the type of j.
The answer seems simple to me - transform before, not afterwards:

#define IMOD(i,j) ((i) <= 0 \
? ((j) <= 0 ? -j - ((-i) % (-j)) : j - ((-i) % (j))) \
: ((j) <= 0 ? (i) % (-j) : (i) % (j)))

Then, if i == m*j + r, then all the following also hold:
i == (-m)*(-j) + r // i pos, j neg
-i == (m-1)*(-j) - j - r // i neg, j neg, -j > r
-i == (m-1)*j + j - r // i neg, j pos, j > r
The remainder parts are all the smallest positive possible, since
they're all positive and all < j.

I'm avoiding the compiler warning for unsigned i and j by using "i <= 0"
and "j <= 0" rather than "i < 0" and "j < 0" (it doesn't matter which
branch we use for zero). Here's a complete sample that runs and produces
correct answers with no warnings with gcc -Wall:

#include <stdio.h>

#define IMOD(i,j) ((i) <= 0 \
? ((j) <= 0 ? -j - ((-i) % (-j)) : j - ((-i) % (j))) \
: ((j) <= 0 ? (i) % (-j) : (i) % (j)))

int main() {
printf("%d %d %d %d %d\n", IMOD(5u,3u), IMOD(5,3), IMOD(-5,3),
IMOD(5,-3), IMOD(-5,-3));
return 0;
}
By the way, am I right in thinking that in a standard-compliant
compilation the expression '-1 % 2 > 0' will always be zero (that
is, false)?

Nope. "If both operands are nonnegative then the remainder is
nonnegative; if not, the sign of the remainder is
implementation-defined." - ISO C++ Standard, 5.6

However, there is an implicit constraint in this promise:
"(a/b)*b + a%b is equal to a"
But we know how multiplication and integer division behave; if you work
it out, (a/b)*b must have the sign of a, and be no further from zero
than a. Consequently, if a is negative, a%b must be nonpositive, in
order to "get back" to a. Similarly, if a is positive, a%b must be
nonnegative, regardless of the sign of b, to satisfy this constraint. I
think pretty much all implementations overlooked this, though.
 
D

Derrick Coetzee

Derrick said:
Nope. "If both operands are nonnegative then the remainder is
nonnegative; if not, the sign of the remainder is
implementation-defined." - ISO C++ Standard, 5.6
"(a/b)*b + a%b is equal to a"

Oops, damnit. I thought I was in the C++ group. Very sorry. Although the
C standard says very similar things in section 6.3.5, it also specifies
that dividing can round in either direction (implementation-defined) if
either operand is negative. This ruins my "implicit constraint"
argument. However, the implementation's choice of behaviours for / and %
are closely interlinked by the statement above.
 
C

CBFalconer

Derrick said:
Oops, damnit. I thought I was in the C++ group. Very sorry.
Although the C standard says very similar things in section 6.3.5,
it also specifies that dividing can round in either direction
(implementation-defined) if either operand is negative. This ruins
my "implicit constraint" argument. However, the implementation's
choice of behaviours for / and % are closely interlinked by the
statement above.

I believe the standard for % changed between C90 and C99. So it
might be wide to make the macro depend on the standard version.
This won't help with non-standard systems, such as many embedded
compilers.

My preference is that the modulo operation adjust the signs so
that the denominator is positive, and then return a value with the
sign of the numerator. I don't think the standards agree with me.
 
D

Derrick Coetzee

CBFalconer said:
I believe the standard for % changed between C90 and C99. So it
might be wide to make the macro depend on the standard version.
This won't help with non-standard systems, such as many embedded
compilers.

It's changed significantly actually, but only in the last clause:

C90, 6.3.5: "When integers are divided and the division is inexact, if
both operands are positive the result of the / operator is the largest
integer less than the algebraic quotient and the result of the %
operator is positive. If either operand is negative, whether the result
of the / operator is the largest integer less than or equal to the
algebraic quotient or the smallest integer greater than or equal to the
algebraic quotient is implementation-defined, as is the sign of the
result of the % operator. If the quotient a/b is representable, the
expression (a/b)*b + a%b."

C99, 6.5.5: "When integers are divided, the result of the / operator is
the algebraic quotient with any fractional part discarded. [Footnote:
This is often called "truncation towards zero."] If the quotient a/b is
representable, the expression (a/b)*b + a%b shall equal a."

Although much shorter, this is a significantly stronger restriction on
both / and %. They've mandated that division is now truncation, even for
negative arguments, for example:

(-5)/(-3) = 1 or 2 in C90, but 1 in C99
(-5)/3 = -1 or -2 in C90, but -1 in C99

Among other things, this means that this identity now holds:

a/b == (-a)/(-b), for all values a,b of integral types where -a and -b

are representable

As a consequence, my previous argument applies in C99, and ensures that
a%b has the sign of a (considering zero to have both signs). Although
the sign of % appears to now be unspecified, in reality it's fully
specified, but in a very subtle way.
 
T

Tim Rentsch

Derrick Coetzee said:
Neither assumption is needed: 0 <= r < |j|, and % takes the type of j.
The answer seems simple to me - transform before, not afterwards:

Certainly there can't be an 'r' with 0 <= r < |j| when j is zero.
A literal reading of the constraint in my earlier message would mean
that j == 0 would mean r == i if i >= 0, and no possible value for
r if i < 0. But let's ignore the j == 0 case for now.

#define IMOD(i,j) ((i) <= 0 \
? ((j) <= 0 ? -j - ((-i) % (-j)) : j - ((-i) % (j))) \
: ((j) <= 0 ? (i) % (-j) : (i) % (j)))

I like the approach. Unfortunately it doesn't quite work if i has a
value that is the most negative value for the type (on most machines
with 2's complement representation anyway). (See I wasn't asking a
question that's completely trivial. :) If j's value is the most
negative value for its (signed) type, the tests I did show the right
value, but it seems like that depends on how signed overflow behaves,
which can't be relied on standard-wise (at least, I believe it can't).

By the way, am I right in thinking that in a standard-compliant
compilation the expression '-1 % 2 > 0' will always be zero (that
is, false)?

[...]

However, there is an implicit constraint in this promise:
"(a/b)*b + a%b is equal to a"
But we know how multiplication and integer division behave; if you work
it out, (a/b)*b must have the sign of a, and be no further from zero
than a. Consequently, if a is negative, a%b must be nonpositive, in
order to "get back" to a. Similarly, if a is positive, a%b must be
nonnegative, regardless of the sign of b, to satisfy this constraint.

Right. I was thinking that the combination of the constraint relating
the '/' and '%' operators plus knowing that divide rounds towards zero
together would imply what the sign of -1 % 2 would be. I suppose
the implementation might be non-compliant, however.
 
T

Tim Rentsch

Derrick Coetzee said:
Oops, damnit. I thought I was in the C++ group. Very sorry. Although the
C standard says very similar things in section 6.3.5, it also specifies
that dividing can round in either direction (implementation-defined) if
either operand is negative.

Oh, I thought C99 said (in effect) that division would round toward
zero. Maybe I'm misremembering.
 
B

Ben Pfaff

Tim Rentsch said:
Oh, I thought C99 said (in effect) that division would round toward
zero. Maybe I'm misremembering.

C99 6.5.5 says this:

When integers are divided, the result of the / operator is
the algebraic quotient with any fractional part
discarded.87) If the quotient a/b is representable, the
expression (a/b)*b + a%b shall equal a.

87) This is often called ``truncation toward zero''.
 
M

Martin

Thank you Ben, Tim, Derrick, and CBFalconer for your replies.

It is a real nuisance that I get that warning, I have now had to change my
initial strategy of creating variables using this method:

const size_t LOOKUP_MAX = NELEMENTS(lookup);

to

const int LOOKUP_MAX = NELEMENTS(lookup);

Obviously I prefer the former as NELEMENTS (the macro I use and shown in my
initial post), uses the sizeof operator which returns size_t and there is
the potential for the wrong value to be placed in LOOKUP_MAX if it's greater
than INT_MAX. This is unlikely to happen in the code I'm writing, but alert!
That's famous last words isn't it? And is how bugs are conceived.

I know that Ben said GCC doesn't generate those warning by default, is it
just those specific type of warnings? I'd have to check in the Hitachi
compiler whether that level of fine-tuning is available.

Martin
 
E

Eric Sosman

Martin said:
Thank you Ben, Tim, Derrick, and CBFalconer for your replies.

It is a real nuisance that I get that warning, I have now had to change my
initial strategy of creating variables using this method:

const size_t LOOKUP_MAX = NELEMENTS(lookup);

to

const int LOOKUP_MAX = NELEMENTS(lookup);

Obviously I prefer the former as NELEMENTS (the macro I use and shown in my
initial post), uses the sizeof operator which returns size_t and there is
the potential for the wrong value to be placed in LOOKUP_MAX if it's greater
than INT_MAX. This is unlikely to happen in the code I'm writing, but alert!
That's famous last words isn't it? And is how bugs are conceived.

Opening the gates to potential bugs in order to
shut out warnings seems a poor trade-off. Why not

/* macro for portable modulo operations */
#if (-1 % 2 > 0)
#define IMOD(i, j) ((i) % (j))
#else
#define IMOD(i, j) (((i) % (j) + (j)) % (j))
#endif

By the way, neither this nor the original handles
the case of j<0 -- but if j is a size_t, that's not
too much of a problem ...
 
M

Martin

Eric Sosman said:
Opening the gates to potential bugs in order to
shut out warnings seems a poor trade-off. Why not

/* macro for portable modulo operations */
#if (-1 % 2 > 0)
#define IMOD(i, j) ((i) % (j))
#else
#define IMOD(i, j) (((i) % (j) + (j)) % (j))
#endif

By the way, neither this nor the original handles
the case of j<0 -- but if j is a size_t, that's not
too much of a problem ...

Have you read my original post, or this thread, Eric?

Martin
 
T

Tim Rentsch

Martin said:
Thank you Ben, Tim, Derrick, and CBFalconer for your replies.

It is a real nuisance that I get that warning, I have now had to change my
initial strategy of creating variables using this method:

const size_t LOOKUP_MAX = NELEMENTS(lookup);

to

const int LOOKUP_MAX = NELEMENTS(lookup);

Obviously I prefer the former as NELEMENTS (the macro I use and shown in my
initial post), uses the sizeof operator which returns size_t and there is
the potential for the wrong value to be placed in LOOKUP_MAX if it's greater
than INT_MAX. This is unlikely to happen in the code I'm writing, but alert!
That's famous last words isn't it? And is how bugs are conceived.

If I understand the problem you're dealing with, it's that the first
form (where LOOKUP_MAX is unsigned) causes problems because some
signed variable (probably 'int i') is compared to it. So if
there is code like

int i;

for( i = 0; i < LOOKUP_MAX; i++ ){
... something with lookup ...
}

The signed/unsigned comparison in the 'for' control clause gets flagged
with a warning. Am I on track?

Assuming that's the kind of problem you're facing, another approach is
to use unsigned types for variables used for indexing. I've taken to
using the type name 'Index' for this:

Index i;

for( i = 0; i < LOOKUP_MAX; i++ ){
... something with lookup ...
}

where 'Index' has been typedef'ed to 'unsigned int' or 'unsigned long'
(or conceivably some other unsigned integer type). Of course, if
this is done, other warnings will come up when 'i' is compared to
signed quantities. Sometimes those quantities can also be changed
to unsigned, but when they can't, it's nice to be able to write
warning free, and safe, comparisons:

#define NONNEGATIVE(x) (assert((x)>=0),(x))

Index i;
int dollars;

if( i > (Index) NONNEGATIVE(dollars) ){
...
}

I have to admit, getting around the signed/unsigned comparison
warnings is kind of a pain in the a**, and it takes some getting used
to. Now that I've been doing it for a while though, it does seem like
the benefits outweigh the costs. And, I'm happy to report, the effort
required to avoid the warnings has gone down monotonically as I've
gone up the experience curve.

By the way, note that the way NONNEGATIVE is written, many compilers
will give a warning message if the argument is already an unsigned
type (as might happen if the type of 'dollars' in the code above
changed, for example). I consider that warning a plus rather than a
minus - it means NONNEGATIVE won't be used unnecessarily. I'm
confident that there are other ways to write the assertion so that no
warning will be given for already unsigned arguments, if someone were
to desire that.
 
M

Martin

Tim said:
... The signed/unsigned comparison in the
'for' control clause gets flagged with a
warning. Am I on track?

Sort of. In fact, Ben explained the problem in his response to me:

"When `a' and `b' are unsigned, `a % b'
is always nonnegative. Thus, ((i) % (j)) < 0
is always false in this case, hence the
first warning. That subexpression is the
first part of a ?: expression, hence the
second warning." -- Ben, 15 Sep 2004

So the problem arises when the IMOD macro is used. Regarding your
suggestion, I note that Plauger says:

"You should make a point of using type
size_t *anywhere* your program
performs array subscripting or address
arithmetic."
-- P.J. Plauger, The Standard C Library

Thanks for your post.

Martin
http://martinobrien.co.uk/
 
E

Eric Sosman

Martin said:
Eric, I was puzzled by the sudden break in sentence, you wrote "why not" and
then quoted my IMOD definition.

Ah, now I see. If you'll re-read my post you'll find
that one of the six lines is not a quote, but differs from
your original version. The changed line is my contribution
(FWIW); the others were just for context. Sorry if they
misled you.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,769
Messages
2,569,579
Members
45,053
Latest member
BrodieSola

Latest Threads

Top