Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions stl/inc/algorithm
Original file line number Diff line number Diff line change
Expand Up @@ -3993,7 +3993,7 @@ namespace ranges {
#endif // __cpp_lib_is_constant_evaluated
{
const auto _Distance = static_cast<size_t>(_ULast - _UFirst);
_CSTD memset(_UFirst, static_cast<unsigned char>(_Value), _Distance);
_Fill_memset(_UFirst, _Value, _Distance);
_UFirst += _Distance;
_Seek_wrapped(_First, _UFirst);
return _First;
Expand Down Expand Up @@ -4032,7 +4032,7 @@ namespace ranges {
if (!_STD is_constant_evaluated())
#endif // __cpp_lib_is_constant_evaluated
{
_CSTD memset(_UFirst, static_cast<unsigned char>(_Value), static_cast<size_t>(_Count));
_Fill_memset(_UFirst, _Value, static_cast<size_t>(_Count));
_UFirst += _Count;
_Seek_wrapped(_First, _UFirst); // no need to move since _UFirst is a pointer
return _First;
Expand Down
8 changes: 4 additions & 4 deletions stl/inc/memory
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ _NoThrowFwdIt uninitialized_fill_n(_NoThrowFwdIt _First, const _Diff _Count_raw,
if (0 < _Count) {
auto _UFirst = _Get_unwrapped_n(_First, _Count);
if constexpr (_Fill_memset_is_safe<_Unwrapped_n_t<const _NoThrowFwdIt&>, _Tval>) {
_CSTD memset(_UFirst, static_cast<unsigned char>(_Val), _Count);
_Fill_memset(_UFirst, _Val, _Count);
_UFirst += _Count;
} else {
_Uninitialized_backout<_Unwrapped_n_t<const _NoThrowFwdIt&>> _Backout{_UFirst};
Expand Down Expand Up @@ -167,7 +167,7 @@ template <class _NoThrowFwdIt, class _Diff, class _Tval>
_NoThrowFwdIt _Uninitialized_fill_n_unchecked1(
const _NoThrowFwdIt _First, const _Diff _Count, const _Tval& _Val, true_type) {
// copy _Count copies of _Val to raw _First, memset optimization
_CSTD memset(_First, static_cast<unsigned char>(_Val), _Count);
_Fill_memset(_First, _Val, _Count);
return _First + _Count;
}

Expand Down Expand Up @@ -1653,7 +1653,7 @@ void _Uninitialized_fill_multidimensional_n(_Ty* const _Out, const size_t _Size,
}
_Guard._Target = nullptr;
} else if constexpr (_Fill_memset_is_safe<_Ty*, _Ty>) {
_CSTD memset(_Out, static_cast<unsigned char>(_Val), _Size);
_Fill_memset(_Out, _Val, _Size);
} else {
_Uninitialized_rev_destroying_backout _Backout{_Out};
for (size_t _Idx = 0; _Idx < _Size; ++_Idx) {
Expand Down Expand Up @@ -1976,7 +1976,7 @@ void _Uninitialized_fill_multidimensional_n_al(_Ty* const _Out, const size_t _Si
}
_Guard._Target = nullptr;
} else if constexpr (_Fill_memset_is_safe<_Ty*, _Ty> && _Uses_default_construct<_Alloc, _Ty*, const _Ty&>::value) {
_CSTD memset(_Out, static_cast<unsigned char>(_Val), _Size);
_Fill_memset(_Out, _Val, _Size);
} else {
_Uninitialized_rev_destroying_backout_al _Backout{_Out, _Al};
for (size_t _Idx = 0; _Idx < _Size; ++_Idx) {
Expand Down
8 changes: 4 additions & 4 deletions stl/inc/xmemory
Original file line number Diff line number Diff line change
Expand Up @@ -1705,7 +1705,7 @@ _Alloc_ptr_t<_Alloc> _Uninitialized_fill_n(
// copy _Count copies of _Val to raw _First, using _Al
using _Ty = typename _Alloc::value_type;
if constexpr (_Fill_memset_is_safe<_Ty*, _Ty> && _Uses_default_construct<_Alloc, _Ty*, _Ty>::value) {
_CSTD memset(_Unfancy(_First), static_cast<unsigned char>(_Val), static_cast<size_t>(_Count));
_Fill_memset(_Unfancy(_First), _Val, static_cast<size_t>(_Count));
return _First + _Count;
} else {
_Uninitialized_backout_al<_Alloc> _Backout{_First, _Al};
Expand Down Expand Up @@ -1733,7 +1733,7 @@ template <class _Alloc>
_Alloc_ptr_t<_Alloc> _Uninit_alloc_fill_n1(_Alloc_ptr_t<_Alloc> _First, _Alloc_size_t<_Alloc> _Count,
const typename _Alloc::value_type& _Val, _Alloc&, true_type) {
// copy _Count copies of _Val to raw _First, using default _Alloc construct, memset optimization
_CSTD memset(_Unfancy(_First), static_cast<unsigned char>(_Val), _Count);
_Fill_memset(_Unfancy(_First), _Val, _Count);
return _First + _Count;
}

Expand All @@ -1756,7 +1756,7 @@ void uninitialized_fill(const _NoThrowFwdIt _First, const _NoThrowFwdIt _Last, c
auto _UFirst = _Get_unwrapped(_First);
const auto _ULast = _Get_unwrapped(_Last);
if constexpr (_Fill_memset_is_safe<_Unwrapped_t<const _NoThrowFwdIt&>, _Tval>) {
_CSTD memset(_UFirst, static_cast<unsigned char>(_Val), static_cast<size_t>(_ULast - _UFirst));
_Fill_memset(_UFirst, _Val, static_cast<size_t>(_ULast - _UFirst));
} else {
_Uninitialized_backout<_Unwrapped_t<const _NoThrowFwdIt&>> _Backout{_UFirst};
while (_Backout._Last != _ULast) {
Expand All @@ -1783,7 +1783,7 @@ template <class _NoThrowFwdIt, class _Tval>
void _Uninitialized_fill_unchecked(
const _NoThrowFwdIt _First, const _NoThrowFwdIt _Last, const _Tval& _Val, true_type) {
// copy _Val throughout raw [_First, _Last), memset optimization
_CSTD memset(_First, static_cast<unsigned char>(_Val), static_cast<size_t>(_Last - _First));
_Fill_memset(_First, _Val, static_cast<size_t>(_Last - _First));
}

template <class _NoThrowFwdIt, class _Tval>
Expand Down
38 changes: 24 additions & 14 deletions stl/inc/xutility
Original file line number Diff line number Diff line change
Expand Up @@ -4732,24 +4732,31 @@ struct _Is_character<char8_t> : true_type {}; // UTF-8 code units are sort-of ch
#endif // __cpp_char8_t

template <class _Ty>
struct _Is_character_or_byte : _Is_character<_Ty>::type {};
struct _Is_character_or_byte_or_bool : _Is_character<_Ty>::type {};

#ifdef __cpp_lib_byte
template <>
struct _Is_character_or_byte<byte> : true_type {};
struct _Is_character_or_byte_or_bool<byte> : true_type {};
#endif // __cpp_lib_byte

template <>
struct _Is_character_or_byte_or_bool<bool> : true_type {};

// _Fill_memset_is_safe determines if _FwdIt and _Ty are eligible for memset optimization in fill
template <class _FwdIt, class _Ty, bool = is_pointer_v<_FwdIt>>
_INLINE_VAR constexpr bool _Fill_memset_is_safe = conjunction_v<
disjunction<conjunction<_Is_character_or_byte<_Unwrap_enum_t<_Ty>>,
_Is_character_or_byte<_Unwrap_enum_t<_Iter_value_t<_FwdIt>>>>,
conjunction<is_same<bool, _Unwrap_enum_t<_Ty>>, is_same<bool, _Unwrap_enum_t<_Iter_value_t<_FwdIt>>>>>,
_INLINE_VAR constexpr bool _Fill_memset_is_safe = conjunction_v<is_scalar<_Ty>,
_Is_character_or_byte_or_bool<_Unwrap_enum_t<remove_reference_t<_Iter_ref_t<_FwdIt>>>>,
is_assignable<_Iter_ref_t<_FwdIt>, const _Ty&>>;

template <class _FwdIt, class _Ty>
_INLINE_VAR constexpr bool _Fill_memset_is_safe<_FwdIt, _Ty, false> = false;

template <class _DestTy, class _Ty>
void _Fill_memset(_DestTy* const _Dest, const _Ty _Val, const size_t _Count) {
_DestTy _Dest_val = _Val; // implicitly convert (a cast would suppress warnings); also handles _DestTy being bool
_CSTD memset(_Dest, static_cast<unsigned char>(_Dest_val), _Count);
}

#if _HAS_IF_CONSTEXPR
template <class _FwdIt, class _Ty>
_CONSTEXPR20 void fill(const _FwdIt _First, const _FwdIt _Last, const _Ty& _Val) {
Expand All @@ -4765,7 +4772,7 @@ _CONSTEXPR20 void fill(const _FwdIt _First, const _FwdIt _Last, const _Ty& _Val)
if (!_STD is_constant_evaluated())
#endif // __cpp_lib_is_constant_evaluated
{
_CSTD memset(_UFirst, static_cast<unsigned char>(_Val), static_cast<size_t>(_ULast - _UFirst));
_Fill_memset(_UFirst, _Val, static_cast<size_t>(_ULast - _UFirst));
return;
}
}
Expand All @@ -4787,7 +4794,7 @@ void _Fill_unchecked1(_FwdIt _First, _FwdIt _Last, const _Ty& _Val, false_type)
template <class _FwdIt, class _Ty>
void _Fill_unchecked1(_FwdIt _First, _FwdIt _Last, const _Ty& _Val, true_type) {
// copy _Val through [_First, _Last), memset optimization
_CSTD memset(_First, static_cast<unsigned char>(_Val), static_cast<size_t>(_Last - _First));
_Fill_memset(_First, _Val, static_cast<size_t>(_Last - _First));
}

template <class _FwdIt, class _Ty>
Expand Down Expand Up @@ -4826,7 +4833,7 @@ _CONSTEXPR20 _OutIt fill_n(_OutIt _Dest, const _Diff _Count_raw, const _Ty& _Val
if (!_STD is_constant_evaluated())
#endif // __cpp_lib_is_constant_evaluated
{
_CSTD memset(_UDest, static_cast<unsigned char>(_Val), static_cast<size_t>(_Count));
_Fill_memset(_UDest, _Val, static_cast<size_t>(_Count));
_UDest += _Count;
_Seek_wrapped(_Dest, _UDest);
return _Dest;
Expand Down Expand Up @@ -4857,7 +4864,7 @@ _OutIt _Fill_n_unchecked2(_OutIt _Dest, _Diff _Count, const _Ty& _Val, false_typ
template <class _OutIt, class _Diff, class _Ty>
_OutIt _Fill_n_unchecked2(_OutIt _Dest, _Diff _Count, const _Ty& _Val, true_type) {
// copy _Val _Count times through [_Dest, ...), memset optimization
_CSTD memset(_Dest, static_cast<unsigned char>(_Val), static_cast<size_t>(_Count));
_Fill_memset(_Dest, _Val, static_cast<size_t>(_Count));
return _Dest + _Count;
}

Expand Down Expand Up @@ -4892,15 +4899,18 @@ _FwdIt fill_n(_ExPo&&, _FwdIt _Dest, _Diff _Count_raw, const _Ty& _Val) noexcept

// Integral types are eligible for memcmp in very specific cases.
// * They must be the same size. (`int == long` is eligible; `int == long long` isn't.)
// * They can't be bool. (Not even `bool == bool`; we're concerned about representations other than 0 and 1.)
// * They can't be volatile.
// * Finally, the usual arithmetic conversions must preserve bit patterns. (This is true for `int == unsigned int`,
// but false for `short == unsigned short`.)
#pragma warning(push)
#pragma warning(disable : 4806) // no value of type 'bool' promoted to type 'char' can equal the given constant
template <class _Elem1, class _Elem2,
bool = sizeof(_Elem1) == sizeof(_Elem2) //
&& _Is_nonbool_integral<_Elem1> && !is_volatile_v<_Elem1> //
&& _Is_nonbool_integral<_Elem2> && !is_volatile_v<_Elem2>>
_INLINE_VAR constexpr bool _Can_memcmp_elements = static_cast<_Elem1>(-1) == static_cast<_Elem2>(-1);
&& is_integral_v<_Elem1> && !is_volatile_v<_Elem1> //
&& is_integral_v<_Elem2> && !is_volatile_v<_Elem2>>
_INLINE_VAR constexpr bool _Can_memcmp_elements =
is_same_v<_Elem1, bool> || is_same_v<_Elem2, bool> || static_cast<_Elem1>(-1) == static_cast<_Elem2>(-1);
#pragma warning(pop)

#ifdef __cpp_lib_byte
// Allow memcmping std::byte.
Expand Down
7 changes: 7 additions & 0 deletions tests/std/tests/P0896R4_ranges_alg_fill/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ struct instantiator {
}
assert(result == ranges::end(output));
}
{ // Validate int is properly converted to bool
bool output[] = {false, true, false};
fill(ranges::begin(output), ranges::end(output), 5);
for (const bool& elem : output) {
assert(elem == true);
}
}
}
};

Expand Down
7 changes: 7 additions & 0 deletions tests/std/tests/P0896R4_ranges_alg_fill_n/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ struct instantiator {
assert(ranges::equal(output, expected_output));
assert(result == ranges::begin(output));
}
{ // Validate int is properly converted to bool
bool output[] = {false, true, false};
fill_n(ranges::begin(output), ranges::distance(output), 5);
for (const bool& elem : output) {
assert(elem == true);
}
}
}
};

Expand Down
36 changes: 34 additions & 2 deletions tests/std/tests/VSO_0180469_fill_family/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ using namespace std;

// This thing is a workaround for C4309 "truncation of constant value"
template <typename T, typename U>
T cast(U i) {
return static_cast<T>(i);
remove_volatile_t<T> cast(U i) {
return static_cast<remove_volatile_t<T>>(i);
}

// Tests that `fillCall`(buffer, value, startIndex, endIndex) fills [startIndex, endIndex) with `value`
Expand Down Expand Up @@ -131,9 +131,41 @@ int main() {
test_fill<int, char>();
test_fill<char, int>();

test_fill<volatile char, char>(); // Test GH-1183

test_uninitialized_fill(
[](count_copies* buff, size_t n, const count_copies& src) { uninitialized_fill(buff, buff + n, src); });

test_uninitialized_fill(
[](count_copies* buff, size_t n, const count_copies& src) { uninitialized_fill_n(buff, n, src); });

// Validate int is properly converted to bool
{
bool output[] = {false, true, false};
fill(output, output + 3, 5);
for (const bool& elem : output) {
assert(elem == true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests will likely pass even if int is not properly converted to bool. I suggest

for (const bool& elem : output) {
    assert(memcmp(&elem, &true_value, sizeof(bool)) == 0);
}

Copy link
Contributor Author

@AdamBucior AdamBucior Aug 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On my PC the assertion fails when I run:

#include <cstring>
#include <cassert>

using namespace std;

int main() {
    bool b;
    memset(&b, 5, 1);
    assert(b == true);
}

And I can't use memcmp because it's not constexpr

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it depends on debug (/Od) vs release (/O2).
Maybe in debug == true is literally comparison against true, and in release it is just test and branch on flags.

If that's right, then since tests run in debug, this looks enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this code in release width /O2 (only replaced assert(b == true); with if (b != true) abort(); because assert does nothing in release) and it still fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It mustn't be UB because this code compiles (unless it's a compiler bug):

#include <bit>
#include <cassert>

using namespace std;

constexpr bool f() {
    char i = 5;
    return bit_cast<bool>(i);
}

int main() {
    constexpr bool b = f();
    static_assert(b != true);
    static_assert(b != false);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I found that this code compiles:

#include <bit>
#include <algorithm>

using namespace std;

constexpr bool f() {
    char i = 2;

    bool arr[]  = {false, true, true};
    bool arr2[] = {false, true, bit_cast<bool>(i)};
    return !equal(begin(arr), end(arr), begin(arr2));
}

int main() {
    static_assert(f());
}

// * They can't be bool. (Not even `bool == bool`; we're concerned about representations other than 0 and 1.)

Which means that equal should be able to use memcmp with bool.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting.
I think it is still UB (expected behavior is a possible UB manifestation)
Usually wrong bools act sometimes as true, but apparently constexpr evaluation does not trigger that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe assert(bit_cast<char>(elem) == bit_cast<char>(bool(true))) ?

Copy link
Contributor Author

@AdamBucior AdamBucior Aug 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe assert(bit_cast<char>(elem) == bit_cast<char>(bool(true))) ?

These tests are not C++20 only so bit_cast is not always available.

I think it is still UB (expected behavior is a possible UB manifestation)

And even if it's UB it means that there's no reason for equal not to use memcmp for bool - not using memcmp in this case would only make sense if bit_cast<bool>((char)2) == true was always true and we know now that at least sometimes it's not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also has_unique_object_representations_v<bool> is true which (I think) means that if two bools have different representations they must compare unequal.

}
}
{
bool output[] = {false, true, false};
fill_n(output, 3, 5);
for (const bool& elem : output) {
assert(elem == true);
}
}
{
bool output[] = {false, true, false};
uninitialized_fill(output, output + 3, 5);
for (const bool& elem : output) {
assert(elem == true);
}
}
{
bool output[] = {false, true, false};
uninitialized_fill_n(output, 3, 5);
for (const bool& elem : output) {
assert(elem == true);
}
}
}
45 changes: 28 additions & 17 deletions tests/std/tests/VSO_0180469_ptr_cat/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,23 +257,33 @@ void test_case_Equal_memcmp_is_safe_comparator() {

#ifdef __cpp_lib_concepts
// contiguous iterators should not change the answer
STATIC_ASSERT(
_Equal_memcmp_is_safe<typename vector<Elem1>::iterator, typename vector<Elem2>::iterator, Pr> == Expected);
STATIC_ASSERT(_Equal_memcmp_is_safe<typename vector<Elem1>::const_iterator, typename vector<Elem2>::const_iterator,
Pr> == Expected);
if constexpr (!is_same_v<Elem1, bool> && !is_same_v<Elem2, bool>) { // vector<bool>::iterator is not contiguous
STATIC_ASSERT(
_Equal_memcmp_is_safe<typename vector<Elem1>::iterator, typename vector<Elem2>::iterator, Pr> == Expected);
STATIC_ASSERT(_Equal_memcmp_is_safe<typename vector<Elem1>::const_iterator,
typename vector<Elem2>::const_iterator, Pr> == Expected);
}
STATIC_ASSERT(
_Equal_memcmp_is_safe<typename array<Elem1, 1>::iterator, typename array<Elem2, 1>::iterator, Pr> == Expected);
STATIC_ASSERT(_Equal_memcmp_is_safe<typename array<Elem1, 1>::const_iterator,
typename array<Elem2, 1>::const_iterator, Pr> == Expected);
// Mixing contiguous iterators should not change the answer
STATIC_ASSERT(_Equal_memcmp_is_safe<typename vector<Elem1>::iterator, typename vector<Elem2>::const_iterator,
Pr> == Expected);
STATIC_ASSERT(_Equal_memcmp_is_safe<typename array<Elem1, 1>::const_iterator,
typename vector<Elem2>::const_iterator, Pr> == Expected);
STATIC_ASSERT(
_Equal_memcmp_is_safe<typename vector<Elem1>::iterator, typename array<Elem2, 1>::iterator, Pr> == Expected);
STATIC_ASSERT(_Equal_memcmp_is_safe<typename vector<Elem1>::iterator, typename array<Elem2, 1>::const_iterator,
Pr> == Expected);
if constexpr (!is_same_v<Elem1, bool> && !is_same_v<Elem2, bool>) {
STATIC_ASSERT(_Equal_memcmp_is_safe<typename vector<Elem1>::iterator, typename vector<Elem2>::const_iterator,
Pr> == Expected);
}

if constexpr (!is_same_v<Elem2, bool>) {
STATIC_ASSERT(_Equal_memcmp_is_safe<typename array<Elem1, 1>::const_iterator,
typename vector<Elem2>::const_iterator, Pr> == Expected);
}

if constexpr (!is_same_v<Elem1, bool>) {
STATIC_ASSERT(_Equal_memcmp_is_safe<typename vector<Elem1>::iterator, typename array<Elem2, 1>::iterator,
Pr> == Expected);
STATIC_ASSERT(_Equal_memcmp_is_safe<typename vector<Elem1>::iterator, typename array<Elem2, 1>::const_iterator,
Pr> == Expected);
}
// span iterators are contiguous
STATIC_ASSERT(
_Equal_memcmp_is_safe<typename span<Elem1>::iterator, typename span<Elem2>::iterator, Pr> == Expected);
Expand Down Expand Up @@ -335,7 +345,8 @@ void test_case_Equal_memcmp_is_safe() {
}

void equal_safe_test_cases() {
// memcmp is safe for non-bool integral types
// memcmp is safe for integral types
test_case_Equal_memcmp_is_safe<true, bool, bool>();
test_case_Equal_memcmp_is_safe<true, char, char>();
test_case_Equal_memcmp_is_safe<true, signed char, signed char>();
test_case_Equal_memcmp_is_safe<true, unsigned char, unsigned char>();
Expand Down Expand Up @@ -377,10 +388,10 @@ void equal_safe_test_cases() {
test_case_Equal_memcmp_is_safe<sizeof(int) == sizeof(long), unsigned long, unsigned int>();
test_case_Equal_memcmp_is_safe<true, long long, unsigned long long>();
test_case_Equal_memcmp_is_safe<true, unsigned long long, long long>();
// memcmp is not safe for bool types
test_case_Equal_memcmp_is_safe<false, bool, bool>();
test_case_Equal_memcmp_is_safe<false, bool, char>();
test_case_Equal_memcmp_is_safe<false, char, bool>();
// memcmp is safe between bool and other integral types with the same size because we don't care about
// representations other than 0 and 1
test_case_Equal_memcmp_is_safe<true, bool, char>();
test_case_Equal_memcmp_is_safe<true, char, bool>();
// No enums
test_case_Equal_memcmp_is_safe<false, bool_enum, bool>();
test_case_Equal_memcmp_is_safe<false, bool, bool_enum>();
Expand Down