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.

Sign up for GitHub

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

Jump to bottom

iter_value_t<I> should always use direct-initialization #4233

Merged
merged 21 commits into from
Jan 24, 2024
Merged

iter_value_t<I> should always use direct-initialization #4233

merged 21 commits into from
Jan 24, 2024

Conversation

fsb4000
Copy link
Contributor

@fsb4000 fsb4000 commented Dec 3, 2023 •
edited
Loading

@fsb4000 fsb4000 requested a review from a team as a code owner December 3, 2023 07:39
@fsb4000 fsb4000 changed the title iter_value_t<I> should always use direct-initialization iter_value_t<I> should always use direct-initialization Dec 3, 2023
@fsb4000 @frederick-vs-ja
fix ranges::transform
b78ce8f 
Co-authored-by: A. Jiang <[email protected]>
@CaseyCarter CaseyCarter added bug Something isn't working ranges C++20/23 ranges labels Dec 5, 2023
@StephanTLavavej StephanTLavavej self-assigned this Dec 6, 2023
@StephanTLavavej

This comment was marked as resolved.

Sign in to view
StephanTLavavej
StephanTLavavej reviewed Jan 18, 2024
View reviewed changes
tests/std/tests/GH_004109_iter_value_t_direct_initialization/test.compile.pass.cpp Outdated Show resolved Hide resolved
tests/std/tests/GH_004109_iter_value_t_direct_initialization/test.compile.pass.cpp Outdated Show resolved Hide resolved
tests/std/tests/GH_004109_iter_value_t_direct_initialization/test.compile.pass.cpp Outdated Show resolved Hide resolved
tests/std/tests/GH_004109_iter_value_t_direct_initialization/test.compile.pass.cpp Outdated Show resolved Hide resolved
tests/std/tests/GH_004109_iter_value_t_direct_initialization/test.compile.pass.cpp Outdated Show resolved Hide resolved
tests/std/tests/GH_004109_iter_value_t_direct_initialization/test.compile.pass.cpp Outdated Show resolved Hide resolved
tests/std/tests/GH_004109_iter_value_t_direct_initialization/test.compile.pass.cpp Outdated Show resolved Hide resolved
tests/std/tests/GH_004109_iter_value_t_direct_initialization/test.compile.pass.cpp Outdated Show resolved Hide resolved
tests/std/tests/GH_004109_iter_value_t_direct_initialization/test.compile.pass.cpp Show resolved Hide resolved
tests/std/tests/GH_004109_iter_value_t_direct_initialization/test.compile.pass.cpp Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Jan 18, 2024
@StephanTLavavej
Copy link
Member

Thanks for fixing this bug and adding a comprehensive test! I pushed a merge with main to resolve a trivial conflict, then I pushed a bunch of changes to the test - please double-check my work.

@fsb4000
Copy link
Contributor Author

fsb4000 commented Jan 18, 2024

adding a comprehensive test

Thanks for adding all the missing tests to my comprehensive test 😻

@StephanTLavavej
Copy link
Member

I gave this some more thought and pushed more changes after I got greedy 😹

  • No need to test ranges::destroy, ranges::destroy_n.
  • Test more overloads of parallel inclusive_scan and transform_inclusive_scan.
    • Noticed while comparing serial to parallel.
  • Drop unnecessary argument variations - this is compile-only.
    • I verified that all of the observable iter_move fixes are still detected by the remaining test lines.

@StephanTLavavej StephanTLavavej self-assigned this Jan 24, 2024
@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 had to push an additional commit to work around a newly-discovered bug, VSO-1946395 "/clr /std:c++20 /Od backend ICE for parallel std::find_end". This repros with VS 2022 17.9 Preview 4 x86 (didn't check x64) without your PR's changes - it's simply being found by the enhanced test coverage you're adding.

@StephanTLavavej StephanTLavavej merged commit 2644138 into microsoft:main Jan 24, 2024
35 checks passed
@StephanTLavavej
Copy link
Member

Thanks for fixing this bug and adding comprehensive test coverage! 🐞 🛠️ 😸

@fsb4000 fsb4000 deleted the fix4109 branch January 25, 2024 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frederick-vs-ja frederick-vs-ja frederick-vs-ja left review comments

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

Assignees

@StephanTLavavej StephanTLavavej

Labels
bug Something isn't working ranges C++20/23 ranges
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

<algorithm>: iter_value_t<I> should always use direct-initialization
4 participants
@fsb4000 @StephanTLavavej @frederick-vs-ja @CaseyCarter