
* What is your evaluation of the design? * What is your evaluation of the implementation? It's an extremely interesting library, and I really liked many of the design decisions made. I tried converting a simple Spirit grammar into an xpressive regex. I'll admit that this isn't a good idea, at least in my case, but it was an entertaining test of the library. The library seemed to do very well at most things I threw at it. I did run into one or two bugs, but Eric has already fixed them. MSVC7.1 frequently generated internal compiler errors, but the regex I was creating is hopefully significantly more complex than a regex should be. (I didn't profile it, but it looked as though it would be extremely slow as a regex.) One thing that I'd like to see improved is the ability to organize complex regexes. I think one example that I hope is easy to implement would be an ability to name a set of characters. There are a number of literals (_d, _w, etc.) that already do this. I would have found it useful to be able to name my own character sets in static regexes. * What is your evaluation of the documentation? Overall, it was extremely helpful. I did have a few minor complaints that I'll detail below: The syntax cheat sheet should have a string literal added to it. I'd like to see it be complete, and this is the only thing I could find that wasn't there. In addition, I'd like to be able to define character sets of my own that could be used in multiple places in my static regex. I'd like the documentation to better describe the _w identifier. I couldn't find the definition of word character. (I can imagine only one definition making sense, but that's not the same as having a definition.) On the page grammars_and_nested_matches.html, there is a discussion of how the library nicely handles all sorts different lifetime scenarios, but it does not discuss the lifetime of the regex refered to by a regex object that is refered to only through a by_ref clause in another regex. Does by_ref copy the object and manage the lifetime or not? One big advantage of static xpressive regexes is the potential of making a complex regex relatively readable. I'd like to see some discussion of this in the docs. (Something along the lines of http://www.boost.org/libs/spirit/doc/style_guide.html) The documentation of ~as_xpr( character_constant ) should be included. This is a useful construct. * What is your evaluation of the potential usefulness of the library? This will be an extremely useful library. I can see three reasons why this would be valuable to a user: It gives users another set of options to look at when trying to improve the performance of a regex. It makes it possible to create some fairly complex regexes that would be impossible or unreadable with the current boost regex library. The ability to refer to subexpressions in a static regex allows you to make a regex significantly easier to read and understand. Static regexes may be easier to read and understand than traditional string based regex languages, but I don't have enough experience to be confident of that statement. * Did you try to use the library? With what compiler? Did you have any problems? I played with it a bit. (See above.) I used MSVC7.1 and had slight troubles. (I also broke my personal record for long error messages. Even Spirit has never generated a 10MB error message for me before!) * How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I read the documentation reasonably carefully and spent an afternoon writing some code with it. I did not test the code carefully, but I did get it to compile. * Are you knowledgeable about the problem domain? I've used boost::regex a fair amount. I've also used Spirit occasionally. * Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion. Yes.

Thanks for the feedback. Comments inline ... Fred wrote:
(I didn't profile it, but it looked as though it would be extremely slow as a regex.)
If you ever do profile it, please post the results. I've dome some very limitted tests which show xpressive to be faster than Spirit, but I'd be very interested in real-world data.
One thing that I'd like to see improved is the ability to organize complex regexes. I think one example that I hope is easy to implement would be an ability to name a set of characters. There are a number of literals (_d, _w, etc.) that already do this. I would have found it useful to be able to name my own character sets in static regexes.
Good idea. Shouldn't be too hard to implement.
The syntax cheat sheet should have a string literal added to it. I'd like to see it be complete, and this is the only thing I could find that wasn't there.
Noted.
I'd like the documentation to better describe the _w identifier.
You're right. _w == (alpha | digit | '_'). I'll add it to the docs.
On the page grammars_and_nested_matches.html, there is a discussion of how the library nicely handles all sorts different lifetime scenarios, but it does not discuss the lifetime of the regex refered to by a regex object that is refered to only through a “by_ref” clause in another regex. Does by_ref copy the object and manage the lifetime or not?
regex uses pimpl. by_ref() increments the impl's ref-count, so you never get dangling references. I'll make the docs more clear on this point.
One big advantage of static xpressive regexes is the potential of making a complex regex relatively readable. I'd like to see some discussion of this in the docs. (Something along the lines of http://www.boost.org/libs/spirit/doc/style_guide.html)
That would be useful.
The documentation of ~as_xpr( character_constant ) should be included. This is a useful construct.
Yup, an oversight. <snip>
(I also broke my personal record for long error messages. Even Spirit has never generated a 10MB error message for me before!)
I consider unreasonably long error messages to be bugs in xpressive. Anybody encountering such an error should send me a bug report, and I'll see about adding a static assertion to give a better error message.
* Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion. Yes.
Thanks for taking the time. -- Eric Niebler Boost Consulting www.boost-consulting.com
participants (2)
-
Eric Niebler
-
Fred