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

Implement LWG-3717 common_view::end should improve random_access_range case #3266

Merged
merged 3 commits into from
Dec 6, 2022
Merged

Implement LWG-3717 common_view::end should improve random_access_range case #3266

merged 3 commits into from
Dec 6, 2022

Conversation

frederick-vs-ja
Copy link
Contributor

Fixes #3221.

Perhaps "random access iterators that only work with its difference type" is useful for conformance test and can be used elsewhere....

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner December 5, 2022 17:04
@strega-nil-ms
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@strega-nil-ms
Copy link
Contributor

That was a weird error - looks like the boost-math submodule wasn't checked out.

@StephanTLavavej StephanTLavavej added the LWG Library Working Group issue label Dec 5, 2022
@StephanTLavavej
Copy link
Member

Looking at the logs for the initial failed test run, I see that "Checkout boost-math source" was displayed as successful, but actually logged:

error: No such remote 'boostorg'
fatal: protocol error: bad pack header
fatal: ambiguous argument 'FETCH_HEAD': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
Finishing: Checkout boost-math source

This also affected the LLVM and google/benchmark submodule checkouts, but the microsoft/STL repo checkout succeeded. I'm not sure what could have caused this, especially with how each of the 8 VMs experienced the same thing.

StephanTLavavej
StephanTLavavej approved these changes Dec 5, 2022
View reviewed changes
tests/std/tests/P0896R4_views_common/test.cpp
Comment on lines +394 to +395
friend constexpr bool operator==(difference_type_only_iterator, difference_type_only_iterator) = default;
friend constexpr auto operator<=>(difference_type_only_iterator, difference_type_only_iterator) = default;
Copy link
Member

Choose a reason for hiding this comment

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

No change requested: This is correct, but defaulting just the spaceship would be sufficient. This is WG21-N4917 [class.compare.default]/4.

tests/std/tests/P0896R4_views_common/test.cpp
friend constexpr auto operator<=>(difference_type_only_iterator, difference_type_only_iterator) = default;

using iterator_concept = contiguous_iterator_tag;
using value_type = remove_cvref_t<T>;
Copy link
Member

Choose a reason for hiding this comment

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

No change requested: After static_assert(is_object_v<T>) which forbids references, it would be sufficient to say remove_cv_t<T>.

@StephanTLavavej StephanTLavavej self-assigned this Dec 5, 2022
@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 4483e87 into microsoft:main Dec 6, 2022
@StephanTLavavej
Copy link
Member

Thanks for these endless conformance improvements! ♾️ 😻 🎉

@frederick-vs-ja frederick-vs-ja deleted the lwg-3717 branch December 7, 2022 02:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@strega-nil-ms strega-nil-ms strega-nil-ms approved these changes

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

Assignees

@StephanTLavavej StephanTLavavej

Labels
LWG Library Working Group issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

LWG-3717 common_view::end should improve random_access_range case
3 participants
@frederick-vs-ja @strega-nil-ms @StephanTLavavej