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

<ranges>: Strengthen exception specification for _Defaultabox #3762

Merged
merged 3 commits into from
Jun 15, 2023

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Jun 11, 2023

One non-defaulted move constructor for _Defaultabox is currently never noexcept, which is probably a bug. So I decided not to add // strengthened to that move constructor. Some similar constructors are made conditionally noexcept, and I think // strengthened is suitable for them.

_Defaultabox is used for join_view::_Iterator and lazy_split_view, while the standard wording uses optional and non-propagating-cache respectively. And thus I'm afraid that the exception specifications are not yet strong enough for lazy_split_view. But further strengthening seems dangerous.

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner June 11, 2023 16:38
@achabense
Copy link
Contributor

achabense commented Jun 11, 2023

_Defaultabox is not in standard. Do we need // strengthened for the noexcept-ness of its methods?

@StephanTLavavej
Copy link
Member

@achabense is correct - we typically don't bother with "strengthened" comments for things that don't appear in the Standard. (For things that directly correspond to exposition-only functions, I think sometimes we do, but I forget specific examples.)

@StephanTLavavej StephanTLavavej added enhancement Something can be improved ranges C++20/23 ranges labels Jun 11, 2023
@StephanTLavavej StephanTLavavej self-assigned this Jun 11, 2023
@achabense
Copy link
Contributor

achabense commented Jun 12, 2023

There are also a lot of /*strengthened*/s for other non-standard classes in this header 👀.

stl/inc/ranges Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Jun 12, 2023
@StephanTLavavej StephanTLavavej self-assigned this Jun 14, 2023
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit c134021 into microsoft:main Jun 15, 2023
35 checks passed
@StephanTLavavej
Copy link
Member

🦾 ⚠️ 📦

@frederick-vs-ja frederick-vs-ja deleted the noexcept-_Defaultabox branch June 15, 2023 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved ranges C++20/23 ranges
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants