Thanks for taking the time, John. Some comments inline. John Maddock wrote:
Finally, too late for even the extended deadline, here's my xpressive review:
Actually, the deadline was extended to tomorrow, so you're good.
*What is your evaluation of the design?
It's very good.
I have to admit that I find the static regexes very hard to read, but a lot of that will be a cultural problem: I'm so used to seeing normal regex syntax that I can't quite get my head around the static versions, the quantifiers as prefixes particularly throw me. However, the syntax is concise and consistent with spirit, so I can hardly complain that much. Possibly xpressive isn't the last word on this, or the best way to go, personally I prefer more verbose forms like
repeat(x)
There is a repeat function for feature-parity with {n,m} style quantifiers, but it could also be used in place of * and + if you prefer. For instance this: *x is equivalent to: repeat<0,inf>(x)
but again, that's an initial reaction which may change with more familiarity.
Quite possibly.
* What is your evaluation of the implementation?
Looks good to me, and I know from past experience that Eric knows what he's doing.
The only complaint I have is one that's common to all expression template code: it's impossible to understand by debugging, which means casual maintenance by folks other than the author would be hard, you really need to read the whole source before you can get a grip on what it's doing (OK you ought to do that anyway, but it is still hard to understand code that isn't executed in the traditional sense). As I said though this is an issue with all metaprogramming, and expression templates in particular, I guess what we really need is a compile-process debugger :-)
Indeed. I wonder if you were debugging the execution of a regex, or the compilation of one. The code that gets executed during regex matching is actually pretty straight-forward. Most of what you need to know is in xpressive/detail/matchers.hpp, which should read fairly well. The expression template code is much harder to read (and write!). I tried to factor much of it out into a reusable sub-library (xpressive/detail/proto/). It generates executable code from an expression template given a set of MPL-like transformations. If this tool got wider adoption, perhaps it would give us a common language with which to talk about this stuff. You're right though, a compile-process debugger would rock.
* What is your evaluation of the documentation?
It's excellent.
* What is your evaluation of the potential usefulness of the library?
Clearly there are lots of uses. There is also overlap with Regex and Spirit, but I don't believe that should prevent acceptance, Boost is meant to drive forward innovation after all.
* Did you try to use the library? With what compiler? Did you have any problems?
Not for as long as I would have liked :-(
I've only tried with VC7.1, the sample code compiled cleanly, but compile times were quite long for even simple static regexes, I didn't try anything complex.
* How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
A couple hours reading the docs, and playing with some of the samples.
One thing I did notice is that several of the test programs don't appear to do anything except "return 0", I'm not sure if these are deprecated, or just not finished (see test_regex_token_iterator.cpp for example).
There are a bunch of tests that simply ensure that each of the public headers compiles cleanly on its own. I should change these from "run" to "compile" in the test matrix and add some comments in the cpp files.
The test cases in the testX.cpp files may be a little short on the ground as well: Boost.Regex has about a thousand cases, PCRE a similar number, where as there are 171 here. The smaller number of cases ought to be enough, but in practice I've found you can never have too many! It's a pity that the Boost.Regex and / or PCRE cases can't be automatically harvested, but I suspect this is next to impossible for static regexes?
Not impossible, or even that hard really. It has long been my intention to write a tool that takes a perl-style regex and spits out a static xpressive style regex. (Naturally, it will be written with xpressive!) It has also been my plan to try to port as many of the Boost.Regex test cases to xpressive as possible. Of course, compiling 1000 static regexes could take a looong time, so I might need to be a bit more targeted in my approach.
* Are you knowledgeable about the problem domain?
I hope so :-)
Indeed, you are! :-)
* Do you think the library should be accepted as a Boost library?
Absolutely.
John.
Many thanks. -- Eric Niebler Boost Consulting www.boost-consulting.com