Skip to content

Folding algorithms - #3099

Merged
Stephan T. Lavavej (StephanTLavavej) merged 43 commits into
microsoft:mainfrom
JMazurkiewicz:fold
Oct 24, 2022
Merged

Stephan T. Lavavej (StephanTLavavej) merged 43 commits into
microsoft:mainfrom
JMazurkiewicz:fold

Conversation

@JMazurkiewicz

@JMazurkiewicz Jakub Mazurkiewicz (JMazurkiewicz) commented Sep 12, 2022 •

Copy link
Copy Markdown
Contributor

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

Changes:

  • The header includes 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 Stephan T. Lavavej (StephanTLavavej) added ranges C++20/23 ranges cxx23 C++23 feature labels Sep 12, 2022
@JMazurkiewicz
Jakub Mazurkiewicz (JMazurkiewicz) marked this pull request as ready for review September 12, 2022 22:52
Comment thread stl/inc/yvals_core.h Outdated

Copy link
Copy Markdown
Contributor

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!'

Comment thread stl/inc/yvals_core.h Outdated
Comment thread stl/inc/algorithm Outdated
Comment thread stl/inc/algorithm Outdated
Comment thread stl/inc/algorithm Outdated
Comment thread stl/inc/algorithm Outdated
@StephanTLavavej

This comment was marked as resolved.

Comment thread stl/inc/algorithm Outdated
Comment thread stl/inc/algorithm Outdated
Comment thread stl/inc/algorithm Outdated
Comment thread tests/std/tests/P2322R6_ranges_alg_fold/test.cpp Outdated
Comment thread tests/std/tests/P2322R6_ranges_alg_fold/test.cpp Outdated
Comment thread tests/std/tests/P2322R6_ranges_alg_fold/test.cpp
@StephanTLavavej

Copy link
Copy Markdown
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 nicole mazzuca (@strega-nil-ms) as I pushed changes after you approved.

Comment thread stl/inc/algorithm

template > _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
Copy Markdown
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
Copy Markdown
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

Copy link
Copy Markdown
Member

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

@StephanTLavavej

Copy link
Copy Markdown
Member

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

@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit bbc5d9b into microsoft:main Oct 24, 2022
@StephanTLavavej

Copy link
Copy Markdown
Member

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

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.

7 participants