Boost C++ Libraries: Ticket #4704: Support for multicapture and balancing groups

status changed

Eric Niebler — Wed, 06 Oct 2010 18:16:23 GMT

status new → assigned

Eric Niebler — Thu, 07 Oct 2010 00:37:46 GMT

The capture conditional:

dynamic: (?P(name)stuff)

Looking at the implementation of this in parse_group, "stuff" is parsed by calling parse_sequence. Is it really the case that stuff can not have alternates? Can I not say "(?P(name)this|that)"? If so, this would be the only grouping construct that does not permit alternates. I have a bad feeling about that. Note that your implementation of that feature ends up returning early---the only one to do so---and doesn't reset the traits flags. In short, this feels like a great big ugly hack to me. Can you clean this up?

Also, would be very nice if you followed my coding convention. Eg:

   if(this)
   {
       // 4 space indent
   }

not this:

   if (this) {
     // 2 space indent
   }

Finally, all these new features need documentation.

Still reviewing your code, more comments to come I'm sure. Don't be discouraged, I still am very happy to have your contribution and have every intention of accepting it, once the kinks are worked out.

Eric Niebler — Thu, 07 Oct 2010 00:46:32 GMT

In parser_traits.hpp:

            case BOOST_XPR_CHAR_(char_type, 'P'):
                this->eat_ws_(++begin, end);
                BOOST_XPR_ENSURE_(begin != end, error_paren, "incomplete extension");
                switch(*begin)
                {
                case BOOST_XPR_CHAR_(char_type, '<'):
                    this->eat_ws_(++begin, end);
                    switch (*begin)
                    ^^^^^^^^^^^^^^^
Whoops! Doesn't check that begin != end before dereferencing begin!

Eric Niebler — Thu, 07 Oct 2010 00:56:20 GMT

There's probably a simple answer, but why did you change the handling of token_named_mark to allow the same capture name to be reused? Previously it was an error.

Eric Niebler — Thu, 07 Oct 2010 01:16:13 GMT

Oh wait! I just realized from looking at your change to mark_end_matcher that you are always populating the captures member of the sub_match struct (for non-hidden captures). This will have disastrous performance implications. There cannot potentially be an allocation every time you match a marked sub-expression. I can't accept this change.

I suggest you find a way to make this behavior opt-in.

Fri, 08 Oct 2010 00:37:50 GMT

Replying to eric_niebler:

There's probably a simple answer, but why did you change the handling of token_named_mark to allow the same capture name to be reused? Previously it was an error.

I thought it to be nice to have the same capabilities in a dynamic expression as in static one. A static expression (s1 = whatever) << (s1 = whatever) is perfectly legal but the dynamic compiler frowned upon (?P<name> whatever) (?P<name> whatever). Now they behave the same. It makes it hard to figure out the resulting group numbers though, so these constructs are really only useful when using named multi captures. You can capture on several places in the expression to the same named stack of captures. Can be useful at times.

Sun, 10 Oct 2010 15:14:24 GMT

Replying to eric_niebler:

Oh wait! I just realized from looking at your change to mark_end_matcher that you are always populating the captures member of the sub_match struct (for non-hidden captures). This will have disastrous performance implications. There cannot potentially be an allocation every time you match a marked sub-expression. I can't accept this change.

I suggest you find a way to make this behavior opt-in.

I have now uploaded an updated version of the multicapture functionality. Multicapture is now opt-in with very little impact on performance if not used.

Some documentation about multicapture and the new syntax constructs is added and I've also gone over the changes to make sure it follows the same coding standard as the rest of the code.

I also changed the mark conditional construct to accept alternates, (?P(name)this|that|andwhatnot).

The new syntax for static pop capture required some extra code in the grammar, is_pure and width_of. I would be grateful for some extra reviewing in those areas since I am uncertain that I fully grasp all the details about what's going on there.

The new code can be reached through the same URL as the old version.

matt.c — Thu, 29 Jun 2017 18:58:58 GMT

Did this ever get integrated?