0

I have a code with very simple logical arithmetic that involves values returned from std::atomic_bool.

#include <iostream>
#include <atomic>

int main() {
    uint16_t v1 = 0x1122;
    uint16_t v2 = 0xaaff;
    std::atomic_bool flag1(false);

    uint16_t r1 = v1 | v2;
    std::cout << std::hex << r1 << std::endl;

    uint16_t r2 = static_cast<uint16_t>(flag1.load()) | static_cast<uint16_t>(0xaaff);
    std::cout << std::hex << r2 << std::endl;

    std::cout << __VERSION__ << std::endl;
}

Code example is here. Compile line: g++ -std=c++17 -O3 -Wall -pedantic -Wconversion -pthread main.cpp && ./a.out.

Based on the STD API, load() should return the underlined type stored in the atomic. So the flag1.load() should be returning bool. However, the compiler send a warning that it is asked to convert an int to uint16_t:

main.cpp:13:55: warning: conversion from 'int' to 'uint16_t' {aka 'short unsigned int'} may change value [-Wconversion]
     uint16_t r2 = static_cast<uint16_t>(flag1.load()) | static_cast<uint16_t>(0xaaff);
                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Where exactly does it do this conversion? Both sides of the | are converted to uint16_t. Why is it still printing a warning?

Peter Cordes
  • 328,167
  • 45
  • 605
  • 847
ilya1725
  • 4,496
  • 7
  • 43
  • 68

1 Answers1

1

Usual arithmetic conversions (as specified at §5 of ISO standard) define that any operands for most or all arithmetic and binary operators undergo integer promotion before the operation itself.

This means that both uint16_t operands are first promoted to int to compute the bitwise | and then truncated back to uint16_t to store in r2.

Indeed that's what the warning is about: there's an implicit truncation of an int to an uint16_t.

These conversions also define that a bool will always evaluate to 1 or 0, so the first cast is useless, but since the second operand will be promoted to an int, then also the second cast is useless, you could go with

uint16_t r2 = flag.load() | 0xaaff;

and possibly silence the warning by explicitly casting to a narrower type, which makes you aware of the fact that this is happening:

uint16_t r2 = static_cast<uint16_t>(flag.load() | 0xaaff);
Jack
  • 131,802
  • 30
  • 241
  • 343
  • *are first promoted to `uint32_t`*: unfortunately no. If `int` can represent all values of `uint16_t`, narrow unsigned values are converted to *signed* `int` not `unsigned int`. This can lead to unexpected signed-overflow UB for `a * b` with `uint16_t a,b; a=b=0xFFFF;` on a machine with 32-bit 2's complement `int` (result is `0xfffe0001`). We can see from the OP's warning that it's `int`, not `unsigned int`. – Peter Cordes May 15 '18 at 23:18
  • @PeterCordes: yes, by using `` types I got confused myself while answering, I edited the answer. It's actually safe to assume that if both values can fit in an `uint16_t` then no unexpected behavior will arise in this situation (as both will fit an `int32_t` and we're dealing with a bitwise operator) but in different situations this could be a problem. – Jack May 15 '18 at 23:22
  • Now your answer is correct for machines where `int` is `int32_t`. The standard doesn't guarantee this, but it's probably the case on every machine that has `int32_t` at all and where `int` is wider than 16 bits. (A C++ implementation on m68k or AVR might have 16-bit `int` = `int16_t`, for example.) I'm not sure it's a good idea to use stdint.h types in this answer; the OP probably knows how wide `int` is on their machine. – Peter Cordes May 15 '18 at 23:25
  • @PeterCordes: it's better to be safe than sorry although we're dealing with a really corner case here – Jack May 15 '18 at 23:27