46

This is a real WTF for me, looks like a bug in GCC, but I'd like to have the community have a look and find a solution for me.

Here's the simplest program I could muster:

#include <stdio.h>
#include <stdint.h>

int main(void)
{
 uint16_t i = 1;
 uint16_t j = 2;
 i += j;
 return i;
}

I'm trying to compile this on GCC with -Werror=conversion flag, which I'm using for much of my code.

Here's the result:

.code.tio.c: In function ‘main’:
.code.tio.c:9:7: error: conversion to ‘uint16_t {aka short unsigned int}’ from ‘int’ may alter its value [-Werror=conversion]
  i += j;

Same error would happen for this code:

uint16_t i = 1;
i += ((uint16_t)3);

Error is

.code.tio.c: In function ‘main’:
.code.tio.c:7:7: error: conversion to ‘uint16_t {aka short unsigned int}’ from ‘int’ may alter its value [-Werror=conversion]
  i += ((uint16_t)3);
       ^

Just to be clear, the error here is on the += operator, NOT the cast.

It looks like the operator overloading for the += with uint16_t is messed up. Or am I missing something subtle here?

For your use: MCVE

Edit: Some more of the same:

.code.tio.c:8:6: error: conversion to ‘uint16_t {aka short unsigned int}’ from ‘int’ may alter its value [-Werror=conversion]
  i = i + ((uint16_t)3);

But i = (uint16_t)(i +3); at least works...

dbush
  • 205,898
  • 23
  • 218
  • 273
immortal
  • 3,118
  • 20
  • 38
  • 2
    C or C++? Operator overloading isn't C... – Joe Nov 21 '17 at 15:46
  • 2
    C, I meant the internal overloading in the compiler... Might've used wrong term? – immortal Nov 21 '17 at 15:46
  • @Michi. Could you explain why? – Mad Physicist Nov 21 '17 at 15:48
  • @Michi I thought you were right and the solution just ugly, but... `.code.tio.c:8:6: error: conversion to ‘uint16_t {aka short unsigned int}’ from ‘int’ may alter its value [-Werror=conversion] i = i + ((uint16_t)3);` – immortal Nov 21 '17 at 15:48
  • @immortal. You should probably add that to the question. Seems relevant. – Mad Physicist Nov 21 '17 at 15:49
  • 2
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=40752 seems releveant – Mat Nov 21 '17 at 15:50
  • I was thinking about something like [This](https://stackoverflow.com/questions/35439640/how-to-drop-the-warning-conversion-to-char-from-int-may-alter-its-value) see @chux answer. – Michi Nov 21 '17 at 15:50
  • @immortal What version of GCC are we playing with here? – Joe Nov 21 '17 at 15:52
  • @Joe Mine is 6.2, Not sure what tio.run uses... – immortal Nov 21 '17 at 15:52
  • Sepcifically, see [Comment 10](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=40752#c10) of the thread Mat posted. – ikegami Nov 21 '17 at 15:53
  • Also, `i++` seems to work. Odd. – Mad Physicist Nov 21 '17 at 15:53
  • The key here seems to be `cc1: some warnings being treated as errors`. There is a reason that this sum is normally only a warning. – Mad Physicist Nov 21 '17 at 15:55
  • Note that `-WConversion` is a very good flag to have. I would recommend to use all of `-std=c11 -pedantic-errors -Wall -Wextra -WConversion`. – Lundin Nov 21 '17 at 16:05
  • 2
    @Lundin This question demonstrates that `-Wconversion` is not a good flag to have because it generates a lot of useless warnings for perfectly good code. – interjay Nov 21 '17 at 16:44
  • I knew [this reminded me of something](https://stackoverflow.com/questions/12976343/c-how-to-get-rid-of-conversion-error). – Daniel Fischer Nov 21 '17 at 22:50
  • 2
    @Phil1970 What in this question made you think of C++? This is a C question... – immortal Nov 22 '17 at 07:31
  • @interjay And if the `int` contained a negative value or a value too large to fit in `uint16_t` that would be perfectly good code too? Or if the lvalue was a signed type and the assignment caused an overflow? The compiler can't always evaluate in compile-time what value a variable will hold. Better that it warns for potential severe bugs than let them slip through. When you start using static analysers, you'll learn about the world of "false positives", which is currently a necessary evil to weed out the greater evil: dangerous, subtle bugs caused by the programmer. – Lundin Nov 22 '17 at 07:50
  • @Lundin So are you saying that every operation that might overflow should cause a warning? Should the compiler issue a warning for `x = x + 1` (where `x` is an `int`) because that may overflow? If not, then why should `i+=j` issue a warning when `i` and `j` are `uint16_t` but not when they are `int`? False positives are fine when they are rare. When there are too many, they will just cause people to ignore or disable warnings, including other warnings which could have been useful. – interjay Nov 22 '17 at 12:38
  • @interjay The real reason why a warning against implicit type promotions is useful is because they are a common source of bugs. And C programmers often have poor knowledge about how they work. You _will_ get a warning for `x = x + 1;` if x is `uint16_t`, but also for `x = x + 33000;`. The compiler is not smart enough to separate these two cases. Swallow the false positive in the first case, because it will save you from the bug in the second case. Alternatively, write rugged, stable C code which does not contain any implicit promotions at all. – Lundin Nov 22 '17 at 12:48
  • they need to add a sufix for short unsigned int numbers. Similar things happens if you compare short unsigned int number with some constant (for example 1). – BЈовић Nov 22 '17 at 12:59
  • 1
    @immortal I didn’t realize it was C... so I delete my comment. – Phil1970 Nov 22 '17 at 17:06

5 Answers5

35

The reason for the implicit conversion is due to the equivalency of the += operator with = and +.

From section 6.5.16.2 of the C standard:

3 A compound assignment of the form E1 op= E2 is equivalent to the simple assignment expression E1 = E1 op (E2), except that the lvalue E1 is evaluated only once, and with respect to an indeterminately-sequenced function call, the operation of a compound assignment is a single evaluation

So this:

i += ((uint16_t)3);

Is equivalent to:

i = i + ((uint16_t)3);

In this expression, the operands of the + operator are promoted to int, and that int is assigned back to a uint16_t.

Section 6.3.1.1 details the reason for this:

2 The following may be used in an expression wherever an int or unsigned int may be used:

  • An object or expression with an integer type (other than int or unsigned int) whose integer conversion rank is less than or equal to the rank of int and unsigned int.
  • A bit-field of type _Bool, int, signed int, or unsigned int.

If an int can represent all values of the original type (as restricted by the width, for a bit-field), the value is converted to an int; otherwise, it is converted to an unsigned int. These are called the integer promotions. All other types are unchanged by the integer promotions.

Because a uint16_t (a.k.a. an unsigned short int) has lower rank than int, the values are promoted when used as operands to +.

You can get around this by breaking up the += operator and casting the right hand side. Also, because of the promotion, the cast on the value 3 has no effect so that can be removed:

i =  (uint16_t)(i + 3);

Note however that this operation is subject to overflow, which is one of the reasons a warning is given when there is no cast. For example, if i has value 65535, then i + 3 has type int and value 65538. When the result is cast back to uint16_t, the value 65536 is subtracted from this value yielding the value 2, which then gets assigned back to i.

This behavior is well defined in this case because the destination type is unsigned. If the destination type were signed, the result would be implementation defined.

dbush
  • 205,898
  • 23
  • 218
  • 273
  • Yes this is what I meant too in my comment – Michi Nov 21 '17 at 15:54
  • 2
    Thanks, I figured this myself, I'm just trying to understand where's the compiler coming from and what causes this error? Why is my uint16_t promoted to int prior to assignment? – immortal Nov 21 '17 at 15:54
  • @immortal see chux answer from my Comment – Michi Nov 21 '17 at 15:55
  • `In this expression, the operands of the + operator are promoted to int` This make sense to me. – Michi Nov 21 '17 at 16:08
  • Thank you all for the answers, can't award the selection to you all so awarding to the 1st, and contains the most info. – immortal Nov 21 '17 at 16:13
  • Notably, the integer promotion rule 6.3.1.1 applies because it is part of the usual arithmetic conversions, which are in turn invoked from the binary + operator. – Lundin Nov 21 '17 at 16:15
  • `(uint16_t)3` Is there any purpose for this? It will still be through integer promotion as an operand to `+` and then the `int` result will be casted to `uint16_t` which will satisfy the compiler - that user is aware of the possible occurrence of spillage and that the user knows what is being done. Won't `uint16_t(i+3)` so the same purpose here? – user2736738 Nov 21 '17 at 16:52
  • @coderredoc Correct, that argument will still be promoted to `int`, so the cast is superfluous. I'll update to reflect. – dbush Nov 21 '17 at 16:54
  • The fact that overflow can occur should be explained. – Eric Postpischil Nov 21 '17 at 17:54
  • @EricPostpischil Good point. Added explanation of overflow. – dbush Nov 21 '17 at 18:02
  • Actually, wrapping in `uint16_t` is not what I meant. That is not overflow in the terms of C standard. I was thinking of the possibility that, because the addition is done with the promoted type, ’int‘, it could overflow and have undefined behavior. However, on second thought, the promotion to ’int` should have only occur if it can represent all `uint16_t`, which leaves enough margin for this arithmetic, unless a bizarre `int` type is possible. – Eric Postpischil Nov 21 '17 at 18:45
  • 3
    Who do I need to smack to get this behavior to go away. It's just plain old wrong. – Joshua Nov 21 '17 at 22:12
  • I still wonder why GCC decided to warn on this. Overflows happen left and right (`int + int` can overflow too), so it's a weak motivation, and the only conversion is "hidden" from the user anyway... seems like it warrants a special case. – Matthieu M. Nov 22 '17 at 08:15
  • 1
    @Joshua I'd say it's probably the most dangerous and broken thing with C. Seemingly "random" integer promotions, sometimes from unsigned to signed.. – pipe Nov 22 '17 at 08:35
14

The argument to any arithmetic operator is subject to the usual arithmetic conversions described in N1570 (latest C11 draft), §6.3.1.8. The passage relevant to this question is the following:

[some rules about floating point types]

Otherwise, the integer promotions are performed on both operands.

So, looking further how the integer promotions are defined, we find the relevant text in §6.3.1.1 p2:

If an int can represent all values of the original type (as restricted by the width, for a bit-field), the value is converted to an int; otherwise, it is converted to an unsigned int. These are called the integer promotions.

So, even with this code:

i += ((uint16_t)3);

the presence of an arithmetic operator causes the operand to be converted back to int. As the assignment is part of the operation, it assigns an int to i.

This is indeed relevant because i + 3 could actually overflow an uint16_t.

  • i + 3 can overflow an int too. – Joshua Nov 21 '17 at 22:13
  • @Joshua Not in this case, because `i` is a 16 bit int, so `i+3` is not going to be very large. – tomsmeding Nov 21 '17 at 23:14
  • @Joshua of course, but this wouldn't be because of *conversion* –  Nov 22 '17 at 06:44
  • @Joshua Well yes then of course, it can overflow, but that is not relevant here. :) – tomsmeding Nov 22 '17 at 07:42
  • But the overflow could be desired behaviour because unsigned value are defined as wrapping around. – meneldal Nov 24 '17 at 05:43
  • So? The OP asked about an option that warns about *conversions*. In this expression, the arithmetic operation is done as `int`, so there's a conversion happening when assigning the result to `i`. End of story... –  Nov 24 '17 at 06:14
12
i += ((uint16_t)3);

is equal to(1)

i = i + ((uint16_t)3);

The right-most operand is explicitly converted from int (the type of the integer constant 3) to uint16_t by the cast. After that, the usual arithmetic conversions(2) are applied on both operands of +, after which both operands are implicitly converted to int. The result of the + operation is of type int.

You then try to store an int in a uint16_t which correctly results in a warning from -Wconversion.

A possible work-around if you wish to avoid assigning an int to a uint16_t would be something like this (MISRA-C compliant etc):

i = (uint16_t)(i + 3u);

(1) This is mandated for all compound assignment operators, C11 6.5.16.2:

A compound assignment of the form E1 op= E2 is equivalent to the simple assignment expression E1 = E1 op (E2), except that the lvalue E1 is evaluated only once,

(2) See Implicit type promotion rules for more info about implicit type promotions.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • `The right-most operand is converted from int to uint16_t.` you mean `3`, which is an `int`? – Michi Nov 21 '17 at 15:59
  • @Michi Updated with clarification. – Lundin Nov 21 '17 at 16:01
  • yes, because [this](https://ideone.com/PLDKap) still results [in the same warning](https://pastebin.com/raw/kGkXEiC8) – Michi Nov 21 '17 at 16:02
  • Here also the `3u` portion is not needed. Because anyway it will go through an integer promotion (be the `u` there or not). The cast to the result only matters. – user2736738 Nov 21 '17 at 16:58
  • @coderredoc The 3u is a good habit and needed to prevent an implicit signed to unsigned conversion. It shows that the programmer is aware of how implicit type promotions work, rather than writing their code in the dark. It doesn't matter in this specific case, but it will matter in other cases. There is always code maintenance to consider: having your variables silently change signedness two times on a single line is not how you write rugged C code. – Lundin Nov 22 '17 at 07:56
3

An explanation is found here:

joseph[at]codesourcery.com 2009-07-15 14:15:38 UTC
Subject: Re: -Wconversion: do not warn for operands not larger than target type

On Wed, 15 Jul 2009, ian at airs dot com wrote:

> Sure, it can wrap, but -Wconversion is not for wrapping warnings.

It's for warnings about implicit conversions changing a value; the arithmetic, in a wider type (deliberately or otherwise), does not wrap, but the value gets changed by the implicit conversion back to char. If the user had explicit casts to int in their arithmetic, there could be no doubt that the warning is appropriate.

The warning occurs because the compiler has the computer performs the arithmetic using a larger type than uint16_t (an int, through integer promotion), and placing the value back into a uint16_t could truncate it. For example,

uint16_t i = 0xFFFF;
i += (uint16_t)3;     /* Truncated as per the warning */

The very same applies to separate assignment and addition operators.

uint16_t i = 0xFFFF;
i = i + (uint16_t)3;  /* Truncated as per the warning */
Community
  • 1
  • 1
ikegami
  • 367,544
  • 15
  • 269
  • 518
  • Shouldn' the same logic apply to `int` though? `uint32_t i = MAX_UINT; i += 3;` Should cause the same behavior. Indeed, any += operation could cause an overflow... Why is the += operator for uint_16 promoting the type? Or is it a single generic implementation for all int types? – immortal Nov 21 '17 at 16:04
  • @immortal See my last comment from Lundin's answer. – Michi Nov 21 '17 at 16:06
  • 2
    @immortal, No. There's no conversion there. The warning is not about the overflow; the warning is about storing an `int` in a `uint16_t`. – ikegami Nov 21 '17 at 16:09
  • @immortal The key is to understand this is that the `+=` operator expands into `i = i + ...`, which in turn invokes implicit type promotions. Simple assignment `i = x;` does not promote the right operand, but rather forces a "lvalue conversion" on it, so that it becomes the same type as the left operand. – Lundin Nov 21 '17 at 16:21
2

There are many cases where it would be useful to perform integer operations directly on small unsigned integer types. Because the behavior of ushort1 = ushort1+intVal; will in all defined cases be equivalent to coercing intVal to the type of ushort1 and then performing the addition directly on that type, however, the authors of the Standard saw no need to write special rules for that situation. I think they clearly recognized that such behavior was useful, but that they expected that implementations would generally behave in such fashion whether or not the Standard mandated it.

Incidentally, gcc sometimes processes arithmetic on values of type uint16_t in different ways when the result is coerced to uint16_t than in cases where it isn't. For example, given

uint32_t multest1(uint16_t x, uint16_t y)
{
  x*=y;
  return x;
}

uint32_t multest2(uint16_t x, uint16_t y)
{
  return (x*y) & 65535u;
}

The function multest1() appears to consistently perform the multiplication mod 65536 in all cases, but the function multest2 does not. For example, the function:

void tester(uint16_t n, uint16_t *p)
{
  n|=0x8000;
  for (uint16_t i=0x8000; i<n; i++)
    *p++= multest2(65535,i);
  return 0;
}

will be optimized equivalent to:

void tester(uint16_t n, uint16_t *p)
{
  n|=0x8000;
  if (n != 0x8000)
    *p++= 0x8000;
  return 0;
}

but such simplification won't occur when using multest1. I would not consider gcc's mod-65536 behavior as reliable, but the difference in code generation shows that:

  1. Some compilers, including gcc, perform mod 65536 arithmetic directly when a result will be coerced directly to uint16_t, but...

  2. Some compilers process integer overflow in ways that may cause erroneous behavior even if code completely ignores all the upper bits of the result, so a compiler that's trying to warn of all possible UB should flag constructs which compilers would be allowed to process in silly fashion as portability violations.

While there are many statements of the form ushort1+=intval that could not possibly cause overflow, it's easier to squawk at all such statements than identify only those which could actually cause erroneous behavior.

supercat
  • 77,689
  • 9
  • 166
  • 211