boostorg / spirit

Boost.org spirit module
http://boost.org/libs/spirit
392 stars 161 forks source link

X3: string_view support #487

Open cmazakas opened 5 years ago

cmazakas commented 5 years ago

std:: and boost::string_view are popular types. First-class native support for them in X3 would be quite nice.

Are there plans to someday support string_view ? boost::iterator_range is showing its age somewhat.

Kojoley commented 5 years ago

It would be nice if we can stop using boost::iterator_range or at least replace it with something else (because including it includes most of the Boost.Range library), but it cannot be string_view as it limited only to character type pointers as iterators.

But the actual problem is in x3::traits::move_to that only supports types that can be constructed with pair of iterators and a strange decision to not have such constructor in string_view. The other minor problem that it is treated as a container.

The good news for you that you do not need to wait until Spirit supports every span/view/range type out of the box (there are bunch of them), because you can easily make you own raw directive that will handle the type you are interested in. https://wandbox.org/permlink/YQwzge0rssBUPfyp

cmazakas commented 5 years ago

This code's amazing!

I had done some digging into X3's internals and I had an idea that it'd be possible but I never would've come up with your implementation on my own :P

Thank you! I'm going to study this and then likely copy-paste it into my codebase. Feel free to close, if you'd like. I'm a contented user.

djowel commented 5 years ago

OK, so where are we now on this? Are we expecting a PR or something?

cmazakas commented 5 years ago

Personally, I've been using @Kojoley's solution.

But they bring up a good point about x3::traits::move_to. We could augment it to have an overload for view-like types such as span and basic_string_view in addition to the current range-based overloads.

djowel commented 5 years ago

I like string_view, let's support it.

cmazakas commented 5 years ago

Coolio! I'll take a look this weekend!

mrnerdhair commented 4 years ago

Just want to add my +1 for this feature; I've been trying for two days now to hack in the support without the compiler smashing me in the face with a frying pan, but overloading function templates reliably is hard. (Did you know that __gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> > isn't convertible to a const char*? I didn't. I'm not even sure it's true, but that's what my terminal is telling me right now.)

cmazakas commented 4 years ago

Did the above Wandbox link not work for you? Hmm, I can try taking a look myself. I've been too tired after work during the week to do any real coding outside of work so far.

mrnerdhair commented 4 years ago

I did finally figure that out today; I had to add a &* to the iterator (making the construction of the std::string_view into attr = { &*saved, typename Attribute::size_type(first - saved) };). Otherwise that __gnu_cxx iterator garbage doesn't convert to a const char* and the compiler can't find a matching constructor for std::string_view -- and it hides the error by saying it's looking for a constructor that takes (braced-init-list) and helpfully not mentioning that it's just that particular list it's having trouble with.

The main problem here, I think, is the lack of a partial specialization in raw.hpp for parse() where Attribute is std::string_view. (Or std::basic_string_view<T>, for generality.) Because you can't partially-specialize a class member template, the custom raw-directive is needed, but that duplicates boilerplate (my::raw_gen and my::raw are equivalent to the standard versions, just in a different namespace) and requires a new namespace -- which you have to name.

I do think it's reasonable to support std::string_view via the raw operator, since unlike std::string it can only repesent a contiguous range, but I'd much prefer not having to hack it in myself.

In case it helps, here's the set of helpers I've developed that let me specify X3 grammars with minimal verbosity. (Luckily I don't need recursive expressions yet; I haven't got an acceptably-readable solution for those yet.)

Kojoley commented 4 years ago

I did finally figure that out today; I had to add a & to the iterator (making the construction of the std::string_view into attr = { &saved, typename Attribute::size_type(first - saved) };).

Do not do &*it, it is generally not supported. You should not be using std::string_view with std::string::(const_)iterator, ~either~ call parse with raw character pointers (str.data(), str.data() + str.size()) ~or use std::basic_string_view<std::string::(const_)iterator>~.