Implement LWG-3561 Issue with internal counter in `discard_block_engi…

…ne` (#4066)

Co-authored-by: Stephan T. Lavavej <[email protected]>

stl/inc/random

@@ -1437,14 +1437,100 @@ private:
     int _Nx;
 };
 
+template <class _Engine, size_t _Px, size_t _Rx>
+class _Discard_block_base { // TRANSITION, ABI, should be merged into discard_block_engine
+public:
+    using base_type   = _Engine;
+    using result_type = typename _Engine::result_type;
+
+    _Discard_block_base() : _Eng(), _Nx(0) {}
+
+    explicit _Discard_block_base(const _Engine& _Ex) : _Eng(_Ex), _Nx(0) {}
+
+    explicit _Discard_block_base(result_type _Seed) : _Eng(_Seed), _Nx(0) {}
+
+    template <class _Seed_seq, _Enable_if_seed_seq_t<_Seed_seq, _Discard_block_base, _Engine> = 0>
+    explicit _Discard_block_base(_Seed_seq& _Seq) : _Eng(_Seq), _Nx(0) {}
+
+    void seed() { // seed engine from default value
+        _Eng.seed();
+        _Nx = 0;
+    }
+
+    void seed(result_type _Xx0) { // seed engine from specified value
+        _Eng.seed(_Xx0);
+        _Nx = 0;
+    }
+
+    template <class _Seed_seq, _Enable_if_seed_seq_t<_Seed_seq, _Discard_block_base> = 0>
+    void seed(_Seed_seq& _Seq) { // seed engine from seed sequence
+        _Eng.seed(_Seq);
+        _Nx = 0;
+    }
+
+    _NODISCARD const base_type& base() const noexcept {
+        return _Eng;
+    }
+
+    _NODISCARD result_type operator()() {
+        if (_Rx <= _Nx) { // discard values
+            while (_Nx++ < _Px) {
+                (void) _Eng();
+            }
+
+            _Nx = 0;
+        }
+        ++_Nx;
+        return _Eng();
+    }
+
+    void discard(unsigned long long _Nskip) { // discard _Nskip elements
+        for (; 0 < _Nskip; --_Nskip) {
+            (void) (*this)();
+        }
+    }
+
+    _NODISCARD_FRIEND bool operator==(const _Discard_block_base& _Left, const _Discard_block_base& _Right) {
+        return _Left._Eng == _Right._Eng && _Left._Nx == _Right._Nx;
+    }
+
+#if !_HAS_CXX20
+    _NODISCARD_FRIEND bool operator!=(const _Discard_block_base& _Left, const _Discard_block_base& _Right) {
+        return !(_Left == _Right);
+    }
+#endif // !_HAS_CXX20
+
+    template <class _Elem, class _Traits>
+    friend basic_istream<_Elem, _Traits>& operator>>(
+        basic_istream<_Elem, _Traits>& _Istr, _Discard_block_base& _Eng) { // read state from _Istr
+        return _Istr >> _Eng._Eng >> _Eng._Nx;
+    }
+
+    template <class _Elem, class _Traits>
+    friend basic_ostream<_Elem, _Traits>& operator<<(
+        basic_ostream<_Elem, _Traits>& _Ostr, const _Discard_block_base& _Eng) { // write state to _Ostr
+        return _Ostr << _Eng._Eng << ' ' << _Eng._Nx;
+    }
+
+private:
+    base_type _Eng;
+    size_t _Nx;
+};
+
 _EXPORT_STD template <class _Engine, size_t _Px, size_t _Rx>
-class discard_block_engine : public discard_block<_Engine, _Px, _Rx> { // discard_block_engine compound engine
+class discard_block_engine // discard_block_engine compound engine
+    : public conditional_t<_Px <= INT_MAX, discard_block<_Engine, static_cast<int>(_Px), static_cast<int>(_Rx)>,
+          _Discard_block_base<_Engine, _Px, _Rx>> {
 public:
     static_assert(0 < _Rx && _Rx <= _Px, "invalid template argument for discard_block_engine");
 
-    using _Mybase     = discard_block<_Engine, _Px, _Rx>;
+    using _Mybase = conditional_t<_Px <= INT_MAX, discard_block<_Engine, static_cast<int>(_Px), static_cast<int>(_Rx)>,
+        _Discard_block_base<_Engine, _Px, _Rx>>;
     using result_type = typename _Engine::result_type;
 
+    static constexpr size_t block_size = _Px;
+    static constexpr size_t used_block = _Rx;
+
     discard_block_engine() : _Mybase() {}
 
     explicit discard_block_engine(const _Engine& _Ex) : _Mybase(_Ex) {}

tests/std/test.lst

@@ -242,6 +242,7 @@ tests\LWG3234_math_special_overloads
 tests\LWG3422_seed_seq_ctors
 tests\LWG3480_directory_iterator_range
 tests\LWG3545_pointer_traits_sfinae
+tests\LWG3561_discard_block_engine_counter
 tests\LWG3610_iota_view_size_and_integer_class
 tests\P0009R18_mdspan_default_accessor
 tests\P0009R18_mdspan_extents

tests/std/tests/LWG3561_discard_block_engine_counter/env.lst

@@ -0,0 +1,4 @@
+# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+RUNALL_INCLUDE ..\usual_matrix.lst

tests/std/tests/LWG3561_discard_block_engine_counter/test.cpp

@@ -0,0 +1,83 @@
+// Copyright (c) Microsoft Corporation.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include <cassert>
+#include <climits>
+#include <cstddef>
+#include <iosfwd>
+#include <random>
+#include <sstream>
+#include <string>
+#include <type_traits>
+
+#define STATIC_ASSERT(...) static_assert(__VA_ARGS__, #__VA_ARGS__)
+
+using namespace std;
+
+constexpr size_t large_block_size = UINT_MAX - 16;
+constexpr size_t large_used_block = UINT_MAX - 20;
+
+struct trivial_engine {
+    using result_type = size_t;
+
+    static constexpr size_t min() {
+        return 0;
+    }
+    static constexpr size_t max() {
+        return SIZE_MAX;
+    }
+
+    size_t operator()() noexcept {
+        return counter_++;
+    }
+
+#if _HAS_CXX20
+    friend bool operator==(const trivial_engine&, const trivial_engine&) = default;
+#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv
+    friend bool operator==(const trivial_engine& lhs, const trivial_engine& rhs) noexcept {
+        return lhs.counter_ == rhs.counter_;
+    }
+    friend bool operator!=(const trivial_engine& lhs, const trivial_engine& rhs) noexcept {
+        return lhs.counter_ != rhs.counter_;
+    }
+#endif // ^^^ !_HAS_CXX20 ^^^
+
+    template <class CharT, class Traits>
+    friend basic_istream<CharT, Traits>& operator>>(basic_istream<CharT, Traits>& is, trivial_engine& eng) {
+        return is >> eng.counter_;
+    }
+
+    template <class CharT, class Traits>
+    friend basic_ostream<CharT, Traits>& operator<<(basic_ostream<CharT, Traits>& os, const trivial_engine& eng) {
+        return os << eng.counter_;
+    }
+
+    size_t counter_ = 0;
+};
+
+using trivial_discard_block = discard_block_engine<trivial_engine, large_block_size, large_used_block>;
+
+void test_lwg_3561() {
+    trivial_discard_block e{};
+    e.discard(168);
+    auto rep = (ostringstream{} << e).str();
+
+    assert(e == e);
+    assert(!(e != e));
+    assert(rep == "168 168"); // relies on the implementation-specific details of operator<<
+}
+
+// Also tests the type correctness of discard_block_engine::block_size/used_block
+
+STATIC_ASSERT(is_same_v<const size_t, decltype(trivial_discard_block::block_size)>);
+STATIC_ASSERT(is_same_v<const size_t, decltype(trivial_discard_block::used_block)>);
+
+STATIC_ASSERT(is_same_v<const size_t, decltype(ranlux24::block_size)>);
+STATIC_ASSERT(is_same_v<const size_t, decltype(ranlux24::used_block)>);
+
+STATIC_ASSERT(is_same_v<const size_t, decltype(ranlux48::block_size)>);
+STATIC_ASSERT(is_same_v<const size_t, decltype(ranlux48::used_block)>);
+
+int main() {
+    test_lwg_3561();
+}

tests/tr1/tests/random4/test.cpp

@@ -231,7 +231,7 @@ static void tsubtract() {
 }
 
 static void tdiscard() {
-    int i;
+    STD size_t i;
     typedef STD subtract_with_carry_engine<Uint32, 24, 10, 24> rng_base_t;
     typedef STD discard_block_engine<rng_base_t, 223, 24> rng_t;
     CHECK_INT(rng_t::block_size, 223);