Sequence container swap noexcept specifiers not-to-spec

I’ve noticed that our sequence containers’ noexcept specifiers for swap aren’t to spec. Is this intentional, or some gross oversight?

Example from

void swap(vector&)
#if _LIBCPP_STD_VER >= 14
_NOEXCEPT;
#else
NOEXCEPT(!__alloc_traits::propagate_on_container_swap::value ||
__is_nothrow_swappable<allocator_type>::value);
#endif

To me, this looks like it’s saying that swap is unconditionally noexcept in C++17 and C++20, and possibly the opposite of [vector.capacity]/12 otherwise?

[vector.capacity]/12 notes:

constexpr void swap(vector& x)
noexcept(allocator_traits::propagate_on_container_swap::value ||
allocator_traits::is_always_equal::value);

If this is an oversight, I’ll file a bug and patch ASAP. If it’s intentional, is it explicitly documented somewhere to explain the deviation? This was a surprise when writing tests for std::ranges::swap’s noexcept status.

Cheers,

Chris

P.S. I’ve scoped this to sequence containers for now, but it might be all allocator-aware containers.

Based on https://en.cppreference.com/w/cpp/container/vector/swap, it seems to me like what we want here is:

void swap(vector&)
#if _LIBCPP_STD_VER < 17
_NOEXCEPT
#else
NOEXCEPT((std::allocator_traits::propagate_on_container_swap::value ||
std::allocator_traits::is_always_equal::value))
#endif

In other words, mark it as unconditionally noexcept (as a conforming extension) pre-C++17, and implement the noexcept specification per the spec in C++17 and above. Do you agree? If so, I think it would be great to submit a patch and add tests for all the places where this is broken. I would be verrrrry grateful if you were willing to do that, I’ll review it.

Louis

I don't think this is an oversight. The conditional noexcept on swap
in the standard is entirely due to the Lakos rule (swapping containers
with non-propagating unequal allocators is undefined), so the
unconditional noexcept is a permissible strengthening. The
__is_nothrow_swappable check pre-17 is presumably to support
allocators with throwing swaps, which were banned by LWG2016.

If a change is desirable, I would suggest treating LWG2016 as a DR and
using unconditional noexcept across the board.

I’ve noticed that our sequence containers’ noexcept specifiers for swap aren’t to spec. Is this intentional, or some gross oversight?

Example from

void swap(vector&)
#if _LIBCPP_STD_VER >= 14
_NOEXCEPT;
#else
NOEXCEPT(!__alloc_traits::propagate_on_container_swap::value ||
__is_nothrow_swappable<allocator_type>::value);
#endif

To me, this looks like it’s saying that swap is unconditionally noexcept in C++17 and C++20, and possibly the opposite of [vector.capacity]/12 otherwise?

[vector.capacity]/12 notes:

constexpr void swap(vector& x)
noexcept(allocator_traits::propagate_on_container_swap::value ||
allocator_traits::is_always_equal::value);

If this is an oversight, I’ll file a bug and patch ASAP. If it’s intentional, is it explicitly documented somewhere to explain the deviation? This was a surprise when writing tests for std::ranges::swap’s noexcept status.

Based on https://en.cppreference.com/w/cpp/container/vector/swap, it seems to me like what we want here is:

void swap(vector&)
#if _LIBCPP_STD_VER < 17
_NOEXCEPT
#else
NOEXCEPT((std::allocator_traits::propagate_on_container_swap::value ||
std::allocator_traits::is_always_equal::value))
#endif

In other words, mark it as unconditionally noexcept (as a conforming extension) pre-C++17, and implement the noexcept specification per the spec in C++17 and above. Do you agree? If so, I think it would be great to submit a patch and add tests for all the places where this is broken. I would be verrrrry grateful if you were willing to do that, I’ll review it.

Louis

Yep, I’ll happily do the sequence containers between and .

Might need some more time getting an understanding of the (unordered)? associative containers before I’m comfortable touching them.

I don’t think this is an oversight. The conditional noexcept on swap
in the standard is entirely due to the Lakos rule (swapping containers
with non-propagating unequal allocators is undefined), so the
unconditional noexcept is a permissible strengthening. The
__is_nothrow_swappable check pre-17 is presumably to support
allocators with throwing swaps, which were banned by LWG2016.

If a change is desirable, I would suggest treating LWG2016 as a DR and
using unconditional noexcept across the board.

I see LWG2016 is P2 and hasn’t been looked at since 2017. What would be necessary to get LWG to review this again?

It's in C++17 already?

Ah, I was looking for a big “Resolved” status and missed that.

So IIUC you’re saying the conditional noexcept specifier is a relic required by the Lakos rule, but is always true thanks to LWG2016, and an allocator that throws in its swap in libc++ calls std::terminate, which is our resolution to UB, and so everything is already A-OK.

My interpretation is

- the _LIBCPP_STD_VER >= 14 unconditional noexcept is a permissible
strengthening relative to the standard's requirement. The conditional
noexcept in the standard is a Lakos rule relic because of the UB in
https://timsong-cpp.github.io/cppwp/container.requirements.general#9.sentence-4.
- the _LIBCPP_STD_VER < 14 conditional noexcept is an attempt to
support allocators that throw when swapped. Those allocators were
banned by LWG2016.

So everything is OK as is, but there may be a case for treating
LWG2016 as a DR and making this function unconditionally noexcept
across all standard versions.

Yeah, I’m fine with that.

Louis, I can add a comment explaining this instead of a technical patch. WDYT?

Thanks for disentangling this, Tim. For some reason, I constantly forget about the Lakos rule.

I actually think we’d be better off strengthening the noexcept to make it unconditional. It’s simpler that way and it’s a conforming extension.

Louis