Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GENERATE(random(...)) can't generate uint8_t values on Windows+MSVC #2433

Open
saxbophone opened this issue May 22, 2022 · 2 comments
Open

Comments

@saxbophone
Copy link

Describe the bug
Attempting to generate random values of std::uint8_t type (AKA unsigned char) fails at compile time with a static_assert() in standard library code.

Truncated error log sample:

example.cpp
C:/data/msvc/14.31.31104/include\random(1919): error C2338: invalid template argument for uniform_int_distribution: N4659 29.6.1.1 [rand.req.genl]/1e requires one of short, int, long, long long, unsigned short, unsigned int, unsigned long, or unsigned long long
C:/data/libraries/installed/x64-windows/include\catch2/catch.hpp(4620): note: see reference to class template instantiation 'std::uniform_int_distribution<Integer>' being compiled
        with
        [
            Integer=uint8_t
        ]
C:/data/msvc/14.31.31104/include\type_traits(327): note: see reference to class template instantiation 'Catch::Generators::RandomIntegerGenerator<T>' being compiled
        with
        [
            T=uint8_t
        ]

Expected behavior
The code compiles and generates uniformly distributed random values of type uint8_t, just as it does for the other unsigned types that are 16 bits wide or greater.

Reproduction steps
Compile the following code with MSVC on Windows (Godbolt link: https://godbolt.org/z/xs97x5c8T)

#define CATCH_CONFIG_MAIN
#include <catch.hpp>

#include <cstdint>

TEST_CASE("Catch2 fails to generate random uint8_t on MSVC") {
    auto input = GENERATE(random((std::uint8_t)0x00, (std::uint8_t)0xff));

    SUCCEED();
}

Platform information:

  • OS: Windows NT
  • Compiler+version: x64 MSVC v19.31
  • Catch version: v2.13.4..2.13.9 (other v2 versions also affected)

Additional context
By reading the spec for std::uniform_int_distribution<>, which I understand Catch uses for random(Integer, Integer), it's clear that this issue arises from attempting to instantiate that number distribution with unsigned char, which is undefined behaviour according to spec. Although this issue does not arise with Clang/GCC on Linux or macOS, it's clear that the Windows C++ stdlib interprets the standard more strictly.

I've previously worked around this Catch limitation by generating values of uint16_t in the range 0x00..0xFF and then downcasting them before use, however this becomes quite awkward in TEMPLATE_TEST_CASE()... It would be nice for Catch to wrap this itself, or outright refuse to generate random values of this type with its own compile time error.

saxbophone added a commit to saxbophone/arby that referenced this issue May 22, 2022
@horenmar
Copy link
Member

I would actually like to reimplement the distributions for Catch2 to provide reproducibility across platforms, but this is definitely a "when I have a lot of time" thing.

@saxbophone
Copy link
Author

I would actually like to reimplement the distributions for Catch2 to provide reproducibility across platforms, but this is definitely a "when I have a lot of time" thing.

That's great to hear. On that note, I've also found random(float, float) a bit awkward for some use-cases (not so much Catch's fault, more a limitation of the fact that uniform_real_distribution<>() is distributed numerically rather than "across all possible discrete values" —try using it with std::numeric_limits<float>::lowest(), std::numeric_limits<float>::max() and it tends to only produce values on the very high ends of the range for the type, not ideal for the use case where you need "pick random floats across the range of every possible floating point bit pattern" type testing.

I've actually rolled my own solution to this second problem, and would be happy to share my experiments with you.

horenmar added a commit that referenced this issue Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants