-
Notifications
You must be signed in to change notification settings - Fork 159
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
ForwardIterator -> ReadableIteratorConcept & ForwardTraversalConcept #320
Conversation
The concept of ForwardIterator is flawed because it mixed 2 sets of concepts (value access and traversal) into 1 package. http://www.boost.org/doc/libs/1_65_1/libs/iterator/doc/new-iter-concepts.html It requires value_type (const)& as return type when dereference is applied, which is not mandatory in spirit parsing. A return type which is convertible to value_type is good enough. ReadableIteratorConcept and ForwardTraversalConcept should be what we need for the iterator check. For example, the iterator of the range returned by boost::adaptors::transform(std::string, func) is normally not a ForwardIterator. But it fulfills ReadableIteratorConcept and ForwardTraversalConcept and should be able to be parsed by spirit.
a4a30c4
to
766cc4c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems perfectly logical to me.
@djowel I'm going to merge this in a day or two if you have nothing against.
Hmmm, seems my previous comment did not get through? Or is this a different PR? |
Who knows. |
OK... anyway, what I wrote was: I like the new iterator traits, but it is not really "new". It was written in 2003 and so far, I don't see it assimilated by the standards (C++11 and onwards). Correct me if I am wrong. So, unless there's real activity towards that, this falls into the category of "don't fix it if it ain't broken". |
The PR is not about simply switching to "new" iterator concepts, but to make spirit less paranoid and allow the things like |
I see that now. OK, we need this alright. It fixes something that's broken. I'll merge this one. |
And thanks @wanghan02 and @Kojoley |
The concept of
ForwardIterator
is flawed because it mixed 2 sets of concepts (value access and traversal) into 1 package.http://www.boost.org/doc/libs/1_65_1/libs/iterator/doc/new-iter-concepts.html
It requires
value_type (const)&
as return type when dereference is applied, which is not mandatory in spirit parsing. A return type which is convertible to value_type is good enough.ReadableIteratorConcept
andForwardTraversalConcept
should be what we need for the iterator check.For example, the iterator of the range returned by
boost::adaptors::transform(std::string, func)
is normally not aForwardIterator
. But it fulfillsReadableIteratorConcept
andForwardTraversalConcept
and should be able to be parsed by spirit.Test cases are added for both qi and x3.
There is also a ticket for this but the solution suggested is not correct.
https://svn.boost.org/trac10/ticket/12817