KEMBAR78
Add missing `size_type` cast to `basic_string` by frederick-vs-ja · Pull Request #5562 · microsoft/STL · GitHub
Skip to content

Conversation

frederick-vs-ja
Copy link
Contributor

Towards #5546.

There're some currently problematic cases:

  • where size_type is narrower than int and hence usual arithmetic conversion changes the type to int, and
  • where size_type is 64-bit and size_t is 32-bit (on a 32-bit platform).

Explicit cast will resolve warnings on narrowing and errors with type mismatch in std::max/std::min in these cases.

@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner May 31, 2025 17:51
@github-project-automation github-project-automation bot moved this to Initial Review in STL Code Reviews May 31, 2025
@frederick-vs-ja frederick-vs-ja force-pushed the string-size-cast branch 3 times, most recently from 827aa18 to 1e2a465 Compare June 1, 2025 02:27
@StephanTLavavej StephanTLavavej added the bug Something isn't working label Jun 1, 2025
@StephanTLavavej StephanTLavavej self-assigned this Jun 1, 2025
_STD _Seek_wrapped(_First, _STD remove(_STD _Get_unwrapped(_First), _STD _Get_unwrapped(_Last), _Val));
_Cont.erase(_First, _Last);
return _Old_size - _Cont.size();
return static_cast<typename _Container::size_type>(_Old_size - _Cont.size());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that _Erase_remove(_if) can be C++20 (and later) -only now, which allows us to drop typename, as we've removed std::experimental::erase(_if).

@StephanTLavavej StephanTLavavej removed their assignment Oct 9, 2025
@StephanTLavavej StephanTLavavej moved this from Initial Review to Ready To Merge in STL Code Reviews Oct 9, 2025
@StephanTLavavej
Copy link
Member

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

@StephanTLavavej StephanTLavavej moved this from Ready To Merge to Merging in STL Code Reviews Oct 9, 2025
@StephanTLavavej StephanTLavavej merged commit 665118f into microsoft:main Oct 10, 2025
39 checks passed
@github-project-automation github-project-automation bot moved this from Merging to Done in STL Code Reviews Oct 10, 2025
@StephanTLavavej
Copy link
Member

Thanks for fixing so many occurrences of warnings! 🛠️ 💯 🐱

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants