P2321R2: Added std::views::zip.

[tylerbrawl]

At long last, the moment we've all been waiting for has come: std::views::zip now finally has an implementation for the MSVC STL. This PR builds off of the efforts put forth in #2687. The following defect reports (DRs) and revisions have also been implemented, as described in #2252:

• LWG-3692
• The std::views::zip portion of P2165R4. A fallback mechanism is provided which continues to use std::pair as described in P2321R2 unless __cpp_lib_tuple_like is defined.

The following features have not been implemented as part of this PR:

• LWG-3702 - This describes modifications to std::views::zip_transform, which this PR does not implement.
• The rest of P2165R4. These changes are significant enough to warrant their own PR. To re-iterate, the provided std::views::zip implementation is compatible with both pre-P2165R4 and post-P2165R4 codebases.

The C++ working draft and the reference implementations from the proposed wording found in the aforementioned papers were the only references used in developing this implementation. Future work includes implementing the remaining portions of P2321R2, as well as those of P2165R4. I might be able to contribute more work towards implementing the remaining portions of P2321R2, but for now, only std::views::zip is provided.

Please let me know if you have any questions.

[ghost]

All CLA requirements met.

[tylerbrawl]

After some investigation, it seems like the tests are failing because the LLVM libc++ test cases have a hardcoded exception to ensure that the feature test macro __cpp_lib_ranges_zip is undefined. I can't seem to find a good way to get around this issue in the test runs I do locally. (Modifying the generate_feature_test_macro_components.py file isn't much of an option here, either.)

I'm going to remove the added definition of __cpp_lib_ranges_zip for now. This seems to be consistent with the current practice of not defining feature test macros until all aspects of the original paper (P2321R2 in this case) are implemented.

That works for now, but if the remainder of the paper is implemented and we should define the feature test macro, then what should we do? (I apologize if this is a common-sense question; this is my first contribution to this project.)

[sam20908]

After some investigation, it seems like the tests are failing because the LLVM libc++ test cases have a hardcoded exception to ensure that the feature test macro __cpp_lib_ranges_zip is undefined

You can skip the necessary libc++ tests for now, with a comment explaining why the tests are skipped (see tests/libcxx/skipped_tests.txt and tests/libcxx/expected_results.txt).

Speaking of tests, it's probably a good idea to add a test for this implementation to make sure it conforms to the Standard. See tests/std/tests for various examples.

[AlexGuteniev]

In addition to defining a feature macro, should also update VSO_0157762_feature_test_macros test and the place in yvals_core.h that says // _HAS_CXX23 directly controls:

[tylerbrawl]

Thank you both for the very helpful advice! I just made the suggested changes to expected_results.txt (the relevant entries already existed in skipped_tests.txt) and reinstated the __cpp_lib_ranges_zip feature test macro. I also added the relevant checks to the VSO_0157762_feature_test_macros and updated yvals_core.h to describe the implementation of std::views::zip.

I also agree that test suites should be added for this implementation. I'll begin work on implementing some.

[StephanTLavavej]

Ok, I think we're good to go now! I've pushed another series of fine-grained commits. FYI @strega-nil-ms as I pushed these changes after you approved.

(Halfway through making the noexcept changes, I wondered if we should simply abandon the attempt to strengthen, but I ended up completing it since you had already done tons of work to get it right.)

[StephanTLavavej]

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

[StephanTLavavej]

I've pushed 3 commits to work around compiler assertions in the internal "checked" build, which I've duly reported. Internal links:

• VSO-1655299 "C1XX Assertion failed: HasAnyErrors() || HasPendingAmpErrors(), with C++23 zip_view"
  • Workaround: Extract the CanCompare computation to a helper function. I've also simplified it with a logical OR instead of a conditional expression (unrelated to the compiler bug).
• VSO-1655455 "C1XX Assertion failed: FATAL_UNREACHABLE in PackExpander.cpp, with C++23 zip_view"
  • Perma-workaround by changing decltype((ranges)) to RangeTypes&, which it always is, and which other parts of the test were already saying.
• VSO-1655459 'C1XX Assertion failed: !"If lookup found RDSymbol before it should find something this time, too" in ParseTree.cpp, with C++23 zip_view'
  • Work around by avoiding some of the size() testing.
  • We also need to silence C4100 "unreferenced formal parameter", both internally and on GitHub, as the compiler complains that ranges is unreferenced. This is probably a compiler bug too, but at this point I was tired enough and just silenced it without reporting a bug or marking it as TRANSITION.

Congratulations on finding so many ways to improve the compiler! 😹

[StephanTLavavej]

Thanks for implementing this new view, and congratulations on your first microsoft/STL pull request (and wow, what a massive effort this was)! 😻 🎉 🚀

This will ship in VS 2022 17.5 Preview 2.