Re: Problems with alternative in X3

On Wed, 6 Apr 2016 00:11:59 +0800
Joel de Guzman <[email protected]> wrote:

> 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:  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
>  [...]  
> >
> > 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  
>  [...]  
> >              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,

The X3 alternative parser now reports the expected attribute like Qi in
this branch. I have another open pull request which shows that
`boost::optional` works as expected in X3 without further code changes.
So I think X3 behavior now matches the documented behavior of Qi. The
code from the OP still does not compile, and I am unsure whether it
should be "fixed" since this would be a new technique for flattening
consumed fusion attributes.

Consider this trivial example which is a twist on the code from the OP:

    int_ >> (
        (':' >> int_ >> ':' >> int_) | 
        (';' >> int_ >> ';' >> int_)
    ) >> '-' >> int_

The expected attribute is `tuple<int, tuple<int, int>, int>`. Should
`tuple<int, int, int, int>` also work? This flattened sequence is not
documented to work in Qi or X3. Qi compiles but generates incorrect
code, and X3 hits a helpful static_assert.

So perhaps this is worth a new thread - should the sequence operator
allow for the flattening of nested sequences? It _should_ be possible
to do this generically (i.e. no specializations for alternative), and
could be helpful in certain situations. Although this would mean that
any of the following consumed attributes would work:

    tuple<int, tuple<int int>, int, tuple<int, int>> // Expected
    tuple<int, tuple<int int>, int, int, int>
    tuple<int, int int, int, tuple<int, int>>
    tuple<int, int int, int, int, int>

Is that confusing?

Lee

------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z