Re: Problems with alternative in X3

On 01/04/2016 1:02 PM, Lee Clagett wrote:
> On Sat, 26 Mar 2016 08:26:06 +0800
> Joel de Guzman <[email protected]> wrote:
>> On 24/03/2016 11:19 PM, Lee Clagett wrote:
>>> On Wed, 23 Mar 2016 15:05:52 +0000
>>> Mikael Asplund <[email protected]> wrote:
>>>> Hi!
>>>>
>>>> I have an issue with alternatives that I don't understand why it's
>>>> happening. I've simplified it even down to just x3::attr():s and I
>>>> still get the problem.
>>>>
>>>> Example here (code also supplied in attachment):
>>>>
>>>> http://coliru.stacked-crooked.com/a/bdf4718cdeb2db4d
>>>>
>>>> Why does the commented out version on line 57 work, and the active
>>>> one on line 58-59 not work? It's an alternative of two exactly
>>>> equal sequences, shouldn't that result in them joining to the
>>>> attribute type as the single sequence?
>>>>
>>>> Regards,
>>>>      Mikael
>>>>
>>>
>>> SpiritX3 does not collapse when alternative branches are identical.
>>> Instead, X3 requires the attribute after the expect parser to be a
>>> variant type [0]; only `variant<Foo>` is permitted as an attribute.
>>> SpiritV2 would remove the variant type from the expected type if it
>>> held only a single type element, allowing `Foo`, `optional<Foo>`,
>>> and `variant<Foo>` in this situation. Joel, which is the preferred
>>> behavior? I can work on a patch + tests if the SpiritV2 approach
>>> should be taken.
>>
>> Is this related to this ticket:
>>
>>     https://svn.boost.org/trac/boost/ticket/12094
>>
>
> That ticket is closely related. I agree with your statement in the
> ticket that the `parse_into_container_impl` specialization for the
> `alternative` parser was probably a mistake. I found the commit [0] that
> added this specialization, and found that it was added for cases
> similar to this:
>
>      std::string s;
>      BOOST_TEST( (test_attr("ab",
>          char_ >> char_ >> ((char_ % ',') | eps), s )) );
>
> I think the problem was actually the difference in exposed attributes
> between x3 and Qi:
>
>      a: A, b: A --> (a | b): A          // Qi
>      a: A, b: A --> (a | b): variant<A> // x3
>
> So the exposed attribute above would be:
>
>      sequence<char, char, container<char>>          // Qi
>      sequence<char, char, variant<container<char>>> // x3
>
> I tested a change to `attribute_of<alternative<...>>` to match the
> behavior of Qi, and removed the `parse_into_impl` specializations for
> `expect` and `alternative`. This forces the sequence parser to pass the
> container to the alternative parser when both alternations collapse to
> the same container type. All of the x3 tests pass, the code in the bug
> ticket passes, but the code in the OP does not. The `static_assert`
> still fails in detail/sequence.hpp because `sequence_size` counts the
> size of the sequence in the alternative to be 1:
>
>      a: A, b: B, c: C, d: B, e: C
>          -> a >> ((b >> c) | (d >> e)) :
>              sequence<A, B, C> but sequence_size<...>::value returns 2
>
> I _think_ this case would work too if the static_assert were updated.
> Also, it does not appear the alternative parser in x3 will consume
> `optional` with a single variant type.
>
> Lee
>
> [0]https://github.com/boostorg/spirit/commit/f16822815a08bfeb70cf75c393d05343b7a7c9f5

I think we should revert to the behavior of Qi, if possible. Can you make
a PR on your changes to the revert_to_qi_behavior branch I created just
now? Please add some relevant test cases too, if you can. That way,
we can work on this issue and hopefully merge back to develop when we
have a reasonable solution.

Thanks in advance!

Regards,
-- 
Joel de Guzman
http://www.ciere.com
http://boost-spirit.com
http://www.cycfi.com/


------------------------------------------------------------------------------