Skip to content

use _Make_unsigned_like_t instead of make_unsigned_t for _Max_possible_v #3356

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
Merged
merged 10 commits into from
Jan 28, 2023
Merged

use _Make_unsigned_like_t instead of make_unsigned_t for _Max_possible_v #3356

merged 10 commits into from
Jan 28, 2023

Conversation

fsb4000
Copy link
Contributor

@fsb4000 fsb4000 commented Jan 21, 2023

Fixes #3025

Yes, I saw @frederick-vs-ja 's comment and we can change every use _Make_unsigned_like_t to make_unsigned_t when the C++ standard will be changed.

@fsb4000 fsb4000 requested a review from a team as a code owner January 21, 2023 14:35
frederick-vs-ja
frederick-vs-ja reviewed Jan 21, 2023
View reviewed changes
@StephanTLavavej StephanTLavavej added bug Something isn't working ranges C++20/23 ranges labels Jan 22, 2023
StephanTLavavej
StephanTLavavej reviewed Jan 23, 2023
View reviewed changes
@StephanTLavavej
Copy link
Member

Thanks! I pushed a conflict-free merge with main, followed by a rename to avoid shadowing std::prev, moving the test to the views::iota test, and testing it both at run-time and compile-time.

strega-nil-ms
strega-nil-ms approved these changes Jan 23, 2023
View reviewed changes
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.

Thanks for this!

CaseyCarter
CaseyCarter reviewed Jan 23, 2023
View reviewed changes
@StephanTLavavej
Copy link
Member

@CaseyCarter @strega-nil-ms I pushed your suggested change. I refrained from adding a _To_unsigned_like() fallback function, but we'll notice if we ever need it in the future and it can easily be added then.

@StephanTLavavej StephanTLavavej self-assigned this Jan 27, 2023
@StephanTLavavej
Copy link
Member

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

@StephanTLavavej
Work around VSO-1666180 "/clr C++20 x64 System.AccessViolationExcep…
f686035 
…tion with `views::iota` and `views::transform`".
@StephanTLavavej
Copy link
Member

I had to push an additional commit to work around the previously reported VSO-1666180 "/clr C++20 x64 System.AccessViolationException with views::iota and views::transform".

@StephanTLavavej StephanTLavavej merged commit 1a28409 into microsoft:main Jan 28, 2023
@StephanTLavavej
Copy link
Member

Thanks for fixing this bug and improving ranges usability! 🐞 🛠️ 😸

@fsb4000 fsb4000 deleted the fix3025 branch January 28, 2023 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

+3 more reviewers

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

@CaseyCarter CaseyCarter CaseyCarter approved these changes

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

Reviewers whose approvals may not affect merge requirements
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.

<iterator>: ranges::prev maybe ill-formed in debug mode
5 participants
@fsb4000 @StephanTLavavej @CaseyCarter @frederick-vs-ja @strega-nil-ms