2

Problem

I am trying to create a map of std::uint8_t -> char, and initialise it with some values:

const std::map<std::uint8_t, char> ScenarioReader::alphabet = {
    { 0x12, 'b' },
    { 0x13, 'c' },
    { 0x15, 'a' },
    { 0x16, 'f' },
    ...
}

This generates a compiler warning because these integer literals (0x12, etc.) are recognised as unsigned ints, which are larger than a std::uint8_t:

1>d:\program files (x86)\microsoft visual studio\2017\community\vc\tools\msvc\14.16.27023\include\utility(172): warning C4244: 'initializing': conversion from '_Ty' to '_Ty1', possible loss of data
1>        with
1>        [
1>            _Ty=unsigned int
1>        ]
1>        and
1>        [
1>            _Ty1=uint8_t
1>        ]
1>d:\my-project\src\myfile.cpp(75): note: see reference to function template instantiation 'std::pair<const _Kty,_Ty>::pair<unsigned int,char,0>(_Other1 &&,_Other2 &&) noexcept' being compiled
1>        with
1>        [
1>            _Kty=uint8_t,
1>            _Ty=char,
1>            _Other1=unsigned int,
1>            _Other2=char
1>        ]
1>d:\my-project\src\myfile.cpp(12): note: see reference to function template instantiation 'std::pair<const _Kty,_Ty>::pair<unsigned int,char,0>(_Other1 &&,_Other2 &&) noexcept' being compiled
1>        with
1>        [
1>            _Kty=uint8_t,
1>            _Ty=char,
1>            _Other1=unsigned int,
1>            _Other2=char
1>        ]

Solutions

I am aware of 2 possible ways to fix this:

1) Disable warnings for this section

#pragma warning( push )
#pragma warning( disable : 4244 )
const std::map<std::uint8_t, char> ScenarioReader::alphabet = {
    { 0x12, 'b' },
    { 0x13, 'c' },
    { 0x15, 'a' },
    { 0x16, 'f' },
    ...
}
#pragma warning( pop)

2) Cast every key explicitly

const std::map<std::uint8_t, char> ScenarioReader::alphabet = {
    { static_cast<std::uint8_t>(0x12), 'b' },
    { static_cast<std::uint8_t>(0x13), 'c' },
    { static_cast<std::uint8_t>(0x15), 'a' },
    { static_cast<std::uint8_t>(0x16), 'f' },
    ...
}

I am not particularly happy about either approach, but the second is especially ugly to me.

Am I, perhaps, missing a simpler solution?

Dan
  • 1,198
  • 4
  • 17
  • 34
  • 1
    Aaaand [godbolt link](https://godbolt.org/z/fc5Yc5) – KamilCuk Sep 28 '20 at 07:52
  • Disabling warnings is seldom useful. The second approach is the better one imo. Allthough it's big and ugly it expresses clearly what you are doing and that you know the cast to be valid. – Lukas-T Sep 28 '20 at 07:58

3 Answers3

2

An integer literal can never be an std::uint8_t. Instead of using a static_cast, you can make an explicit cast of the literal:

const std::map<std::uint8_t, char> alphabet = {
    { std::uint8_t(0x12), 'b' },
    { std::uint8_t(0x13), 'c' },
    { std::uint8_t(0x15), 'a' },
    { std::uint8_t(0x16), 'f' },
};
Pierre
  • 1,942
  • 3
  • 23
  • 43
  • he could write a user-defined literal – JHBonarius Sep 28 '20 at 09:11
  • 1
    You might think you are calling a constructor but this is a C-style cast [...](https://stackoverflow.com/questions/10438249/in-c-what-are-the-differences-between-static-castdoublea-and-doublea) –  Sep 28 '20 at 09:19
  • @AryanParekh no, that would be `(std::uint8_t)0x12`. – JHBonarius Sep 28 '20 at 09:23
  • @AryanParekh Indeed, this is a cast and not a call to the constructor. I updated my answer accordingly. Finally, I think the use of a user-defined literal mentioned by JHBonarius seems to be the cleanest solution. – Pierre Sep 28 '20 at 10:11
  • @Pierre, is the `std::` part in `std::uint8_t` really necessary? If you `#include ` then you should have access to the type `uint8_t` without the namesame (I think behind the scenes there's a `typedef std::uint8_t uint8_t`). – Elliott Sep 28 '20 at 10:31
  • @Elliott You can indeed remove the `std::` part. – Pierre Sep 28 '20 at 11:13
  • @Pierre I also faced a situation where I needed to perform this. But I stuck to `static_cast` as it is safe –  Sep 28 '20 at 11:28
  • @Pierre I don't think it matters too much, but JHBonarius' method also teaches you some new things. Your method is just fine and that's why I upvoted –  Sep 28 '20 at 11:30
2

You can write a user-define literal

#include <cstdint>

constexpr std::uint8_t operator "" _ui8(unsigned long long value)
{
    return static_cast<std::uint8_t>(value);
}

#include <type_traits>

int main() {
    static_assert(std::is_same_v<decltype(0x12_ui8), std::uint8_t>);
}
JHBonarius
  • 10,824
  • 3
  • 22
  • 41
1

The warning is an erroneous one and it happens only on \W4 warning level.

A conversion from a literal to std::uint8_t should not generate any warnings, unless the literal is out of range of the converted to type, which is not the case here.

Any option you choose is OK in my opinion. It just boils down to a matter of personal preference. I personally would prefer the std::uint8_t{0x12} syntax.

Another option would be to go down one warning level to \W3 and then use a separate linter (\W4 is mostly used for lint-like warnings anyway).

bolov
  • 72,283
  • 15
  • 145
  • 224