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.

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

Fix VS2017 string_view support #658

Closed
wants to merge 1 commit into from

Conversation

alabuzhev
Copy link
Contributor

I've tried to address #657 here by encapsulating the iterator type.

I'm not sure if it's the best possible approach, but at least it works.

Note: cmake scripts really should be updated to add /std:c++latest compiler option to enable the "real" VS17 string_view - right now this path is not tested, which is presumably why this issue was not identified earlier.

@alabuzhev alabuzhev force-pushed the vs17_string_view_iterator branch from 4bc2922 to 4f14318 Compare February 18, 2018 11:33

// Advances the begin iterator to ``it``.
FMT_CONSTEXPR void advance_to(iterator it) {
format_str_.remove_prefix(it - begin());
Copy link
Contributor Author

@alabuzhev alabuzhev Feb 18, 2018

Choose a reason for hiding this comment

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

VS17 asserts that iterators must point to the same string_views:

constexpr difference_type operator-(const _String_view_iterator& _Right) const _NOEXCEPT
{	// return difference of iterators
#if _ITERATOR_DEBUG_LEVEL >= 1
	_IDL_VERIFY(_Mydata == _Right._Mydata && _Mysize == _Right._Mysize,
		"cannot subtract incompatible string_view iterators");

After remove_prefix both format_str_'s data and size will be different and this check won't pass.
I've replaced format_str_ with a pair of string_view iterators to resolve this.

Note: it still might fire somewhere since end_ will not be updated and will point to a 'different' string_view after advance_to. I was not able to reproduce it, but it probably should be checked more thoroughly.

@vitaut
Copy link
Contributor

vitaut commented Feb 24, 2018

Fixed by using fallback string_view (#664), but thanks anyway.

@vitaut vitaut closed this Feb 24, 2018
@alabuzhev alabuzhev deleted the vs17_string_view_iterator branch February 24, 2018 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants