3

I am trying to set a color to either foreground or background (fg / bg) according to a mask.

uint8_t bitmap, mask, bg, fg = <whatever>;

This code works:

uint8_t color = bg;
if (bitmap & mask)
{
    color = fg;
}

This code works too but throws a warning: (373) implicit signed to unsigned conversion

uint8_t color = (bitmap & mask) ? fg : bg;

Can anyone explain why? I am using Microchip XC8 2.30.

Kay
  • 123
  • 8
  • 1
    @chux-ReinstateMonica When making an MRE I realised a silly mistake on my part that rendered the question inaccurate. – Kay Apr 16 '21 at 00:11

2 Answers2

2

The compiler warns you that the type of the ternary expression is int, a signed type, whereas the type of the assigned object is uint8_t, an unsigned type. Mixing signed and unsigned types in expressions can lead to counter intuitive behavior, such as sizeof(int) > -1 being false.

It could also warn you that storing an int value into a unt8_t variable may cause an implicit conversion that would change the value. But in this particular case, range analysis can easily prove that any possible outcome of this expression is in the range of type uint8_t so none of the above cases require a warning. This compiler is obnoxious and not smart enough.

The warning can be silenced with an extra cast:

uint8_t color = (uint8_t)((bitmap & mask) ? fg : bg);

but such useless casts obfuscate the code and confuse the reader. Cast should be avoided in most cases as they can lead to spurious bugs.

Your best solution seems to keep the if statement. The compiler might actually generate faster code for this solution as it is a good candidate for a conditional store instruction, producing efficient branchless code.

chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • 3
    The compiler is being properly obnoxious IMO. Per [**6.5.15 Conditional operator**, p5](https://port70.net/~nsz/c/c11/n1570.html#6.5.15p5): "If both the second and third operands have arithmetic type, the result type that would be determined by the usual arithmetic conversions, were they applied to those two operands, is the type of the result. ..." Implicitly cramming an `int` into a `uint8_t` deserves a warning, and making the compiler smart enough to correctly handle all possible cases where no issues could ever arise is probably difficult. – Andrew Henle Apr 01 '21 at 09:54
  • @AndrewHenle: range analysis is not difficult to perform: both expressions have a value in the range [0..255] hence the whole expression value is in the same range so there is no possible data loss in storing it into a `uint8_t` variable. The warning is not unfounded but the cast required to silence it is nefarious in general. Also note that the compiler is not warning about *Implicitly cramming an`int` into a `uint8_t`* but rather about *implicit signed to unsigned conversion* – chqrlie Apr 01 '21 at 19:17
  • @chqrlie: The `if` statement does indeed generate nicer code but is IMO less readable, and certainly I am hoping to avoid unnecessary casts. I do agree with @AndrewHenle though that "the result type that would be determined by the usual arithmetic conversions" - hence this does not make sense since all operands are unsigned. – Kay Apr 06 '21 at 00:38
1

The conditional operator triggers the usual arithmetic conversions over its last two elements (in order to find an appropriate type for the expression as a whole). This in turns implies integer promotions for integral types of rank less than int, which the case here. Oddly, when all the values of the original type fits into an int, the expression gets promoted to int regardless of whether the original type was signed or not. Hence, your conditional expression ends up with type int, and back to uint8_t for the assignment.

This does not happen with the if statement, as in that case you have simple assignments in which both sides have the same uint8_t type.

Virgile
  • 9,724
  • 18
  • 42
  • it's not odd at all [Why must a short be converted to an int before arithmetic operations in C and C++?](https://stackoverflow.com/q/24371868/995714) – phuclv Apr 01 '21 at 08:29
  • @phuclv what kind of artithmetics operations might be applied here? – Gerhardh Apr 01 '21 at 09:16
  • This compiler is obnoxious. The warning can be silenced with an extra cast as in `uint8_t color = (uint8_t)((bitmap & mask) ? fg : bg);` but such useless casts obfuscate the code and confuse the reader. Cast should be avoided in most cases as they can lead to spurious bugs. – chqrlie Apr 01 '21 at 09:17
  • 1
    @Gerhardh see the bitwise `&` there? – phuclv Apr 01 '21 at 09:20
  • 1
    @phuclv true, but that is in the condition only. Why would that bei converted to an unsigned value? The result of that expression ist either fg or bg without any extra arithmetic operation. – Gerhardh Apr 01 '21 at 09:22
  • @Gerhardh I'm not talking about the warning. I'm addressing the `Oddly, when all the values of the original type fits into an int, the expression gets promoted to int regardless of whether the original type was signed or not` sentence – phuclv Apr 01 '21 at 09:25
  • @phuclv But the answer (including the part about being odd) only addresses handling of the last two elements. – Gerhardh Apr 01 '21 at 09:28
  • 1
    @phuclv I've absolutely no problem with the implicit conversion from `short` to `int`. My point is that implicitly converting an _unsigned_ type to a _signed_ one can be a bit confusing. – Virgile Apr 01 '21 at 12:33
  • @Virgile C only types that doesn't change the value after promotion will be promoted. If `unsigned char` has the same size to `int` then it won't be promoted to int. But yes in some cases it's more intuitive to promote to an unsigned type – phuclv Apr 01 '21 at 13:06