Skip to content

Implement P3612R1 Harmonize Proxy-Reference Operations #5848

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+111 −7
Open

Implement P3612R1 Harmonize Proxy-Reference Operations #5848

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ec80052
Add tests for new vector<bool> and bitset reference swaps and assignm…
vmichal Nov 12, 2025
4bed2e4
Add swap to bitset::reference, add operator=(bool) const.
vmichal Nov 12, 2025
8c0a262
Add swap to vector<bool>::reference, make operator=(bool) const avail…
vmichal Nov 12, 2025
3ab0600
Mark libcxx test std/containers/sequences/vector.bool/reference.swap.…
vmichal Nov 12, 2025
8f21bc7
Mention the implemented paper P3612R1 in yvals_core.h
vmichal Nov 12, 2025
5c9abf0
Silence deprecation warning in tr1 vector test.
vmichal Nov 12, 2025
d81ed91
Remove meaningless argument name from defaulted copy ctor.
vmichal Nov 13, 2025
2418738
Deprecate std::vector<bool>::swap(reference, reference) only in C++26…
vmichal Nov 19, 2025
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
23 changes: 22 additions & 1 deletion stl/inc/bitset
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public:
friend bitset;

public:
_CONSTEXPR23 reference(const reference&) = default;
_CONSTEXPR23 reference(const reference&) noexcept = default;

_CONSTEXPR23 ~reference() noexcept {} // TRANSITION, ABI

Expand All @@ -99,6 +99,11 @@ public:
return *this;
}

_CONSTEXPR23 const reference& operator=(const bool _Val) const noexcept {
_Pbitset->_Set_unchecked(_Mypos, _Val);
return *this;
}

_NODISCARD _CONSTEXPR23 bool operator~() const noexcept {
return !_Pbitset->_Subscript(_Mypos);
}
Expand All @@ -112,6 +117,22 @@ public:
return *this;
}

friend constexpr void swap(reference _Left, reference _Right) noexcept {
bool _Val = _Left; // NOT _STD swap
_Left = _Right;
_Right = _Val;
}

friend constexpr void swap(reference _Left, bool& _Right) noexcept {
bool _Val = _Left; // NOT _STD swap
_Left = _Right;
_Right = _Val;
}

friend constexpr void swap(bool& _Left, reference _Right) noexcept {
swap(_Right, _Left);
}

private:
_CONSTEXPR23 reference(bitset<_Bits>& _Bitset, const size_t _Pos) noexcept : _Pbitset(&_Bitset), _Mypos(_Pos) {}

Expand Down
17 changes: 13 additions & 4 deletions stl/inc/vector
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2461,7 +2461,7 @@ private:
using _Difference_type = typename _Mybase::_Difference_type;

public:
_CONSTEXPR20 _Vb_reference(const _Vb_reference&) = default;
_CONSTEXPR20 _Vb_reference(const _Vb_reference&) noexcept = default;

_CONSTEXPR20 _Vb_reference(const _Mybase& _Right) noexcept
: _Mybase(_Right._Myptr, _Right._Myoff, _Right._Getcont()) {}
Expand All @@ -2480,8 +2480,7 @@ public:
return *this;
}

#if _HAS_CXX23
constexpr const _Vb_reference& operator=(bool _Val) const noexcept {
_CONSTEXPR20 const _Vb_reference& operator=(bool _Val) const noexcept {
if (_Val) {
*const_cast<_Vbase*>(_Getptr()) |= _Mask();
} else {
Expand All @@ -2490,7 +2489,6 @@ public:

return *this;
}
#endif // _HAS_CXX23

_CONSTEXPR20 void flip() noexcept {
*const_cast<_Vbase*>(_Getptr()) ^= _Mask();
Expand All @@ -2517,6 +2515,16 @@ public:
_Right = _Val;
}

friend _CONSTEXPR20 void swap(_Vb_reference _Left, bool& _Right) noexcept {
bool _Val = _Left; // NOT _STD swap
_Left = _Right;
_Right = _Val;
}

friend _CONSTEXPR20 void swap(bool& _Left, _Vb_reference _Right) noexcept {
swap(_Right, _Left);
}

protected:
_CONSTEXPR20 _Vbase _Mask() const noexcept {
return static_cast<_Vbase>(1) << this->_Myoff;
Expand Down Expand Up @@ -3509,6 +3517,7 @@ public:
}
}

_DEPRECATE_VECTOR_BOOL_STATIC_REFERENCE_SWAP
static _CONSTEXPR20 void swap(reference _Left, reference _Right) noexcept {
bool _Val = _Left; // NOT _STD swap
_Left = _Right;
Expand Down
18 changes: 17 additions & 1 deletion stl/inc/yvals_core.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,10 @@
// P2614R2 Deprecating float_denorm_style, numeric_limits::has_denorm, numeric_limits::has_denorm_loss
// Other C++23 deprecation warnings

// _HAS_CXX26 and _SILENCE_ALL_CXX26_DEPRECATION_WARNINGS control:
// P3612R1 Harmonize Proxy-Reference Operations
// Other C++26 deprecation warnings

// Parallel Algorithms Notes
// C++ allows an implementation to implement parallel algorithms as calls to the serial algorithms.
// This implementation parallelizes several common algorithm calls, but not all.
Expand Down Expand Up @@ -1512,7 +1516,19 @@ _EMIT_STL_ERROR(STL1004, "C++98 unexpected() is incompatible with C++23 unexpect
#define _DEPRECATE_LOCALE_EMPTY
#endif // ^^^ warning disabled ^^^

// next warning number: STL4049

#if _HAS_CXX26 && !defined(_SILENCE_VECTOR_BOOL_STATIC_REFERENCE_SWAP_DEPRECATION_WARNING) \
&& !defined(_SILENCE_ALL_CXX26_DEPRECATION_WARNINGS)
#define _DEPRECATE_VECTOR_BOOL_STATIC_REFERENCE_SWAP \
[[deprecated("warning STL4049: Static std::vector<bool>::swap(reference, reference) is deprecated by C++26 (see " \
"LWG-3638 and P3612R1). Use non-member function swap(reference, reference) instead. You can define " \
"_SILENCE_VECTOR_BOOL_STATIC_REFERENCE_SWAP_DEPRECATION_WARNING or " \
"_SILENCE_ALL_CXX26_DEPRECATION_WARNINGS to suppress this warning.")]]
#else // ^^^ warning enabled / warning disabled vvv
#define _DEPRECATE_VECTOR_BOOL_STATIC_REFERENCE_SWAP
#endif // ^^^ warning disabled ^^^

// next warning number: STL4050

// next error number: STL1013

Expand Down
3 changes: 3 additions & 0 deletions tests/libcxx/expected_results.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ std/atomics/atomics.ref/member_types.compile.pass.cpp FAIL
std/thread/futures/futures.promise/uses_allocator.pass.cpp FAIL
std/thread/futures/futures.task/futures.task.members/ctor2.compile.pass.cpp FAIL

# libc++ has not implemented P3612R1 "Harmonize Proxy-Reference Operations"
std/containers/sequences/vector.bool/reference.swap.pass.cpp FAIL

# Various bogosity (LLVM-D141004)
std/utilities/utility/mem.res/mem.res.pool/mem.res.pool.ctor/ctor_does_not_allocate.pass.cpp FAIL
std/utilities/utility/mem.res/mem.res.pool/mem.res.pool.ctor/sync_with_default_resource.pass.cpp FAIL
Expand Down
54 changes: 53 additions & 1 deletion tests/std/tests/Dev10_816787_swap_vector_bool_elements/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
// Copyright (c) Microsoft Corporation.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

// This test was extended with functionality needed for
// P3612R1: Harmonize Proxy-Reference Operations

#include <algorithm>
#include <bitset>
#include <cassert>
#include <type_traits>
#include <vector>

using namespace std;

static const auto is_true = [](bool b) { return b; };
static const auto is_false = [](bool b) { return !b; };

int main() {
void check_values_match() {
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja Nov 12, 2025

Choose a reason for hiding this comment

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

I think we should also test the functionality in constant evaluation since C++20.

E.g. we can mark this function CONSTEXPR20.

#if _HAS_CXX20
#define CONSTEXPR20
#else // ^^^ _HAS_CXX20 / !_HAS_CXX20
#define CONSEXPR20 inline
#endif // ^^^ !_HAS_CXX20 ^^^

And then write

check_values_match();
#if _HAS_CXX20
static_assert(check_values_match());
#endif // _HAS_CXX20

Same for some other functions.

Copy link
Author

@vmichal vmichal Nov 13, 2025

Choose a reason for hiding this comment

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

Understood; I tried to fix your comments, and a new issue surfaced: bitset<N>::reference is not (yet) a literal type in C++20 (it uses _CONSTEXPR23), while vector::reference is.

This could be solved naturally by adding #if _HAS_CXX23 to the tests, but is that the correct solution, considering the intent of the paper that both types shall have similar (or even the same?) semantics?
Making the bitset::reference _CONSTEXPR20 would require making relevant portions of bitset _CONSTEXPR20 as well though, so those changes accumulate...

I can look into it, but I need some guidance or precedent for adding constexpr to declarations that are not yet required to be constexpr.

Copy link
Contributor

@frederick-vs-ja frederick-vs-ja Nov 13, 2025

Choose a reason for hiding this comment

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

bitset<N>::reference is not (yet) a literal type in C++20 (it uses _CONSTEXPR23), while vector::reference is.

We can use

#if _HAS_CXX20 && !_HAS_CXX23
if (!is_constant_evaluated())
#endif // _HAS_CXX20 && !_HAS_CXX23
{
    /* test code for bitset<N>::reference */
}

Copy link
Author

@vmichal vmichal Nov 13, 2025

Choose a reason for hiding this comment

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

I see your point.

Nevertheless, could you please clarify what the issue would be with my suggestion (replacing about a dozen instances of _CONSTEXPR23 in <bitset> with _CONSTEXPR20)? I have done this experimentally on my local branch, and things seem to work fine (complete test suite is still running, I have hand-picked several tests touching the std::bitset).

Is it purely for maximal standards conformance? Or is constexpr part of ABI?

Copy link
Contributor

@NylteJ NylteJ Nov 13, 2025

Choose a reason for hiding this comment

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

replacing about a dozen instances of _CONSTEXPR23 in <bitset> with _CONSTEXPR20

I guess the issue is that the standard explicitly forbids adding extra constexpr to functions ([constexpr.functions]).

Copy link
Author

@vmichal vmichal Nov 14, 2025

Choose a reason for hiding this comment

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

Well, other changes introduced by this PR violate [constexpr.functions] as well, don't they? e.g. the change to operator=(bool) const in std::vector<bool>::reference that @StephanTLavavej approved (see #5848 (comment))

Some degree of standards violation seems unavoidable when changes are backported... I don't mean to disagree, I'm just trying to get the correct mental model to simplify future contributions.

Copy link
Contributor

@NylteJ NylteJ Nov 15, 2025

Choose a reason for hiding this comment

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

Well, other changes introduced by this PR violate [constexpr.functions] as well, don't they? e.g. the change to operator=(bool) const in std::vector<bool>::reference that @StephanTLavavej approved (see #5848 (comment))

I'm not a language lawyer, but operator=(bool) const was already constexpr when it was first introduced by P2321R2, so I’d guess it doesn’t violate [constexpr.functions] (we’re just backporting the complete future functionality to an older version).

Some degree of standards violation seems unavoidable when changes are backported...

I agree, but about a dozen explicit standard violations seems a bit excessive to me.

Copy link
Author

@vmichal vmichal Nov 16, 2025

Choose a reason for hiding this comment

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

Understood.
To clarify, do you expect me to implement @frederick-vs-ja's suggestion (#5848 (comment)) or postpone the resolution? Mr. Lavavej mentioned in #5847 that he will review these two PRs later. Do we (I) want his opinion on this matter, or get only approval of the final version?

Copy link
Contributor

@NylteJ NylteJ Nov 16, 2025

Choose a reason for hiding this comment

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

Implementing @frederick-vs-ja’s suggestion seems correct to me, but I’m not a maintainer.

Copy link
Contributor

@frederick-vs-ja frederick-vs-ja Nov 19, 2025

Choose a reason for hiding this comment

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

Well, other changes introduced by this PR violate [constexpr.functions] as well, don't they? e.g. the change to operator=(bool) const in std::vector<bool>::reference that @StephanTLavavej approved (see #5848 (comment))

I personally want to leave addition of operator=(bool) const in C++23 and later. Because it doesn't seem intended for WG21-P3612R1 to backport changes of vector<bool>::reference, and it is intentional that bitset::reference should behave consistently with vector<bool>::reference (this part is also LWG-4187). IMO, as a result, operator=(bool) const of both should be restricted in C++23 and later. But maintainer(s) may have different opinions.

vector<bool> x(100, false);
vector<bool> y(100, true);

Expand All @@ -27,3 +32,50 @@ int main() {
assert(!y[34]);
assert(all_of(y.begin() + 35, y.end(), is_true));
}

template <typename T>
void check_P3612(T& collection) {

auto ref0 = collection[0];
auto const ref1 = collection[1];

// assignments from bool
ref0 = true;
ref1 = false;
// assignments from reference
ref0 = ref1;
ref1 = ref0;

collection[0] = true;
collection[1] = false;

swap(collection[0], collection[1]); // swap(reference, reference)
assert(!collection[0]);

bool b = true;
swap(collection[0], b); // swap(reference, bool)
assert(collection[0]);

swap(b, collection[0]); // swap(bool, reference)
assert(!collection[0]);
}

template <typename T>
constexpr bool has_noexcept_copy_ctor = noexcept(T(std::declval<const T&>()));

int main() {
check_values_match();


constexpr size_t N = 10;
vector<bool> vector(N);
bitset<N> bitset(0);

check_P3612(vector);
check_P3612(bitset);

static_assert(std::is_nothrow_copy_constructible_v<decltype(vector)::reference>, "");
static_assert(std::is_nothrow_copy_constructible_v<decltype(bitset)::reference>, "");
static_assert(has_noexcept_copy_ctor<decltype(vector)::reference>, "");
static_assert(has_noexcept_copy_ctor<decltype(bitset)::reference>, "");
}
3 changes: 3 additions & 0 deletions tests/tr1/tests/vector/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
// test <vector>
#define TEST_NAME "<vector>"

// Since P3612R1 the static vector<bool>::swap(reference, reference) is deprecated
#define _SILENCE_VECTOR_BOOL_STATIC_REFERENCE_SWAP_DEPRECATION_WARNING

#include "tdefs.h"
#include <stddef.h>
#include <vector>
Expand Down