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

<list>: Remove an incorrect /* strengthened */ comment from the move assignment operator #5020

Closed
frederick-vs-ja opened this issue Oct 17, 2024 · 0 comments · Fixed by #5024
Labels
documentation Related to documentation or comments fixed Something works now, yay! good first issue Good for newcomers

Comments

@frederick-vs-ja
Copy link
Contributor

frederick-vs-ja commented Oct 17, 2024

Recently, when investigating stuffs in the standard library affected by WG21-N4258, I found that this exception specification isn't really strengthened:

STL/stl/inc/list

Lines 923 to 924 in 926d458

list& operator=(list&& _Right)
noexcept(_Choose_pocma_v<_Alnode> == _Pocma_values::_Equal_allocators) /* strengthened */ {

The standard requirement (in [list.overview]/2) is saying noexcept(allocator_traits<Allocator>::is_always_equal::value), which hasn't changed since the adoption of N4258.

Looking at the definition of _Choose_pocma_v, it can be inferred that _Choose_pocma_v<_Alnode> == _Pocma_values::_Equal_allocators and allocator_traits<_Alnode>::is_always_equal::value are actually equal.

STL/stl/inc/xmemory

Lines 735 to 746 in 926d458

enum class _Pocma_values {
_Equal_allocators, // usually allows contents to be stolen (e.g. with swap)
_Propagate_allocators, // usually allows the allocator to be propagated, and then contents stolen
_No_propagate_allocators, // usually turns moves into copies
};
template <class _Alloc>
constexpr _Pocma_values _Choose_pocma_v = allocator_traits<_Alloc>::is_always_equal::value
? _Pocma_values::_Equal_allocators
: (allocator_traits<_Alloc>::propagate_on_container_move_assignment::value
? _Pocma_values::_Propagate_allocators
: _Pocma_values::_No_propagate_allocators);

It's not very clear whether we're allowed use this value, see LWG-3267. But it is clear that this is not strengthening!

I believe we should remove the misleading /* strengthened */ comment.

Moreover, the exception specification of the move assignment operators of deque and (unordered) associative containers are spelt differently and more close to the forms specified in the standard. So perhaps we should spell the one of list as noexcept(_Alnode_traits::is_always_equal::value).


There's another _Choose_pocma_v<_Alnode> == _Pocma_values::_Equal_allocators in <xtree>. I guess it would be more consistent to write _Alnode_traits::is_always_equal::value instead.

STL/stl/inc/xtree

Lines 943 to 944 in 926d458

_Tree& operator=(_Tree&& _Right) noexcept(
_Choose_pocma_v<_Alnode> == _Pocma_values::_Equal_allocators && is_nothrow_move_assignable_v<key_compare>) {


This issue is intended for a new contributor (especially one new to GitHub) to get started with the simplest possible change.

Please feel free to submit a pull request if there isn't one already linked here - no need to ask for permission! 😸

You can (and should) link your pull request to this issue using GitHub's close/fix/resolve syntax.
(in the PR description not the commit message)

@CaseyCarter CaseyCarter added enhancement Something can be improved good first issue Good for newcomers labels Oct 17, 2024
5AIPAVAN added a commit to 5AIPAVAN/STL that referenced this issue Oct 17, 2024
@StephanTLavavej StephanTLavavej added documentation Related to documentation or comments and removed enhancement Something can be improved labels Oct 17, 2024
@StephanTLavavej StephanTLavavej added the fixed Something works now, yay! label Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Related to documentation or comments fixed Something works now, yay! good first issue Good for newcomers
Projects
None yet
3 participants