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

Folding algorithms #3099

Merged
merged 43 commits into from
Oct 24, 2022
Merged

Folding algorithms #3099

merged 43 commits into from
Oct 24, 2022

Conversation

JMazurkiewicz
Copy link
Contributor

@JMazurkiewicz JMazurkiewicz commented Sep 12, 2022

This PR implements P2322R6 (ranges::fold) and closes #2922.

Changes:

  • The <algorithm> header includes <optional> in C++23 mode,
  • Added folding functions:
    • ranges::fold_left and ranges::fold_left_with_iter - both invoke _Fold_left_with_iter_fn::_Fold_left_with_iter_impl,
    • ranges::fold_left_first and ranges::fold_left_first_with_iter - both invoke _Fold_left_first_with_iter_fn::_Fold_left_first_with_iter_impl,
    • ranges::fold_right - invokes _Fold_right_unchecked
    • ranges::fold_right_last - invokes private function _Fold_right_last_unchecked

@StephanTLavavej StephanTLavavej added ranges C++20/23 ranges cxx23 C++23 feature labels Sep 12, 2022
@JMazurkiewicz JMazurkiewicz marked this pull request as ready for review September 12, 2022 22:52
@JMazurkiewicz JMazurkiewicz requested a review from a team as a code owner September 12, 2022 22:52
stl/inc/yvals_core.h Outdated Show resolved Hide resolved
@strega-nil-ms strega-nil-ms self-assigned this Sep 14, 2022
Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

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

Minor comments for the algorithm, but otherwise this looks fantastic!
Kiki looking excited like 'alright! thanks!'

stl/inc/yvals_core.h Outdated Show resolved Hide resolved
stl/inc/algorithm Outdated Show resolved Hide resolved
stl/inc/algorithm Outdated Show resolved Hide resolved
stl/inc/algorithm Outdated Show resolved Hide resolved
stl/inc/algorithm Outdated Show resolved Hide resolved
@StephanTLavavej

This comment was marked as resolved.

@StephanTLavavej StephanTLavavej self-assigned this Oct 12, 2022
stl/inc/algorithm Outdated Show resolved Hide resolved
stl/inc/algorithm Outdated Show resolved Hide resolved
stl/inc/algorithm Outdated Show resolved Hide resolved
tests/std/tests/P2322R6_ranges_alg_fold/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P2322R6_ranges_alg_fold/test.cpp Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Oct 20, 2022
@StephanTLavavej
Copy link
Member

Thanks, this looks great! 😻 I exhaustively compared this to the Standard, and checked all of the _EXPORT_STD markings. All I found were some very minor nitpicks and a couple of test typos, so I validated and pushed changes. FYI @strega-nil-ms as I pushed changes after you approved.


template <input_range _Rng, class _Ty, _Indirectly_binary_left_foldable<_Ty, iterator_t<_Rng>> _Fn>
_NODISCARD constexpr auto operator()(_Rng&& _Range, _Ty _Init, _Fn _Func) const {
return _RANGES fold_left_with_iter(_STD forward<_Rng>(_Range), _STD move(_Init), _Pass_fn(_Func)).value;
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally we'd want this to avoid the extra move into the in_value_result and back out. (This is why the wording uses Returns: instead of "Effects: Equivalent to return ...;")

Copy link
Member

Choose a reason for hiding this comment

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

Do RVO or deferred temporary materialization help with that?

I think this would be reasonable to address in a followup PR.

@StephanTLavavej StephanTLavavej self-assigned this Oct 21, 2022
@StephanTLavavej
Copy link
Member

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

@StephanTLavavej
Copy link
Member

I've pushed a merge with main to resolve trivial adjacent-add conflicts in yvals_core.h and tests/std/test.lst.

@StephanTLavavej StephanTLavavej merged commit bbc5d9b into microsoft:main Oct 24, 2022
@StephanTLavavej
Copy link
Member

Thanks for implementing this C++23 feature! ⚙️ 🎉 😻

@JMazurkiewicz JMazurkiewicz deleted the fold branch October 24, 2022 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cxx23 C++23 feature ranges C++20/23 ranges
Projects
None yet
Development

Successfully merging this pull request may close these issues.

P2322R6 ranges::fold_left, ranges::fold_right, etc.
6 participants