Skip to content

Conversation

StephanTLavavej
Copy link
Member

Fixes #4241. Test coverage will be provided by the upcoming libcxx update.

When _First == _Last, the iterator will compare equal to default_sentinel_t and the callsite _Measure_string_prefix() will do the right thing, we just need to avoid calling _Decode_utf() for an empty range. This will leave _Next as null, which is fine - we only need it when we increment the iterator.

_Measure_string_prefix_iterator_legacy already appears to handle empty ranges correctly.

… internal check `_First < _Last`

When `_First == _Last`, the iterator will compare equal to `default_sentinel_t` and the callsite `_Measure_string_prefix()` will do the right thing, we just need to avoid calling `_Decode_utf()` for an empty range.

This will leave `_Next` as null, which is fine - we only need it when we increment the iterator.

`_Measure_string_prefix_iterator_legacy` already appears to handle empty ranges correctly.
@StephanTLavavej StephanTLavavej added bug Something isn't working format C++20/23 format labels Dec 6, 2023
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner December 6, 2023 04:13
_Next = _Decode_utf(_First, _Last, _Val)._Next_ptr;
if (_First != _Last) {
_Next = _Decode_utf(_First, _Last, _Val)._Next_ptr;
}
}

constexpr _Unicode_codepoint_iterator() = default;
Copy link
Contributor

@CaseyCarter CaseyCarter Dec 6, 2023

Choose a reason for hiding this comment

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

Drive-by WTF: Why does line 364 check _Last == _Other._Last when line 363 say it's a precondition? Should we fix that in another PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch. I assume I wrote the INTERNAL_CHECK on a different side of a coffee break than the return statement :D

@CaseyCarter CaseyCarter removed their assignment Dec 6, 2023
@StephanTLavavej
Copy link
Member Author

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

@StephanTLavavej StephanTLavavej merged commit ec7f7ba into microsoft:main Dec 7, 2023
@StephanTLavavej StephanTLavavej deleted the format-empty-string branch December 7, 2023 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working format C++20/23 format
Projects
None yet
Development

Successfully merging this pull request may close these issues.

<format>: format("{:a<10}", "") fails STL internal check _First < _Last
3 participants