Cédric Venet wrote:
The title of my first post was misleading, so I repost it.
Thank you, I missed it the first time.
I am not sure to be qualified to make a review but I will give it a try.
Your feedback is valuable.
First, I think the library should be accepted.
Thanks.
- What is your evaluation of the design?
Seems good. I must say I didn’t read or try the transform part of proto.
- What is your evaluation of the implementation?
Didn’t read the code
- What is your evaluation of the documentation?
I think there is some (more) work to do on it. I had some probleme with users_guide\expression_construction\construction_utils.html Because it was not clear to me how to put it in pratice.
I'll probably redesign the users' guide to be more example-driven. I've gotten a lot of feedback about that.
In fact after a fast read of the doc, I mostly used the exemple as reference of how to do something.
So, I don't know exactly what can be done but some ideas may be:
*Provide more basic example *Refer to the example in the documentation to show the practical use of what is described. *make a FAQ
OK.
One difficulty I didn't had time to study is how to do polymorphic grammar or semantic checking. (perhaps I should not put this in the review). Mor clearly, I add two example:
First I would like to have a grammar which can work on any terminal but can only have compatible terminal in a one expression (for example, you can't have vector and list in one expression but you can have one expression with lists and one with vector and they would follow the same grammar). My idea was to do a basic proto grammar which just check operator use and then to use a transform to fold the type of the terminals, checking they are all compatible.
Proto grammars are context-free, so I think that forces you to do the compatibility checking as a separate step with a transform, as you suggest. It would be an interesting exercise to try to imagine what a context-sensitive Proto grammar might look like, but for now we have to make do. Since Dave also wants to do terminal-based compatibility checks, I banged together an example. The following transform checks to see if all terminals have the same type. It uses a state parameter that is callable ... you stuff terminal types into it and it keeps track of whether they're all the same or not. Pretty simple. The only tricky bit is treating the result of the _state transform as callable and invoking it. I use proto::bind for that (to be renamed proto::lazy). #include <iostream> #include <boost/assert.hpp> #include <boost/mpl/or.hpp> #include <boost/mpl/and.hpp> #include <boost/type_traits/is_same.hpp> #include <boost/type_traits/is_void.hpp> #include <boost/xpressive/proto/proto.hpp> #include <boost/xpressive/proto/transform.hpp> namespace mpl = boost::mpl; namespace proto = boost::proto; template<typename IsValid = mpl::true_, typename Value = void> struct Accumulator : IsValid { template<typename Value2> struct validate : mpl::and_< IsValid , mpl::or_< boost::is_void<Value> , boost::is_same<Value, Value2> > > {}; template<typename Sig> struct result; template<typename This, typename Value2> struct result<This(Value2)> { typedef Accumulator<validate<Value2>, Value2> type; }; template<typename Value2> typename result<void(Value2)>::type operator ()(Value2 const &) const { return typename result<void(Value2)>::type(); } }; struct SameTerminals : proto::or_< proto::when< proto::terminal<proto::_> // treat the current state as a callable // object and pass it the value of the terminal. , proto::bind<proto::_state(proto::_arg)> > , proto::otherwise< proto::fold<proto::_, proto::_state, SameTerminals> > > {}; int main() { int dummy = 0; proto::terminal<int>::type i = {0}, j = {1}, k = {2}; proto::terminal<short>::type s = {42}; BOOST_ASSERT( SameTerminals()(i+j*k, Accumulator<>(), dummy) ); BOOST_ASSERT(!SameTerminals()(i+s*k, Accumulator<>(), dummy) ); } Note that this example exposed a bug in the proto::make transform, which is already fixed. Be sure to get the latest, either from the file vault or subversion.
Second, I have a grammar where I can add terminals 'a+b' and index terminal 'a[b]'. you can have '(a+b)[c]'. some terminals may be indexable but other not. I could make a grammar for indexable terminal and another for not indexable but it would be duplicated. And if I add more properties to my terminals (for example Boolean, integer or float), the number of grammar explode. I was thinking to again use a transform to check the 'semantic' of the expression.
If I understand correctly, I think you can do this with a Proto grammar. See below. #include <vector> #include <iostream> #include <boost/type_traits/is_array.hpp> #include <boost/xpressive/proto/proto.hpp> #include <boost/xpressive/proto/transform.hpp> namespace proto = boost::proto; using proto::_; struct IndexableTerminals : proto::or_< proto::terminal<std::vector<_,_> > , proto::and_< proto::terminal<_> , proto::if_<boost::is_array<proto::_arg>() > > > {}; struct NonIndexableTerminals : proto::and_< proto::terminal<_> , proto::not_<IndexableTerminals> > {}; template<typename TerminalGrammar = NonIndexableTerminals> struct MyGrammar : proto::or_< TerminalGrammar , proto::subscript< MyGrammar<IndexableTerminals>, _ > , proto::nary_expr<_, proto::vararg<MyGrammar<TerminalGrammar> > > > {}; template<typename Expr> void assert_valid(Expr const &) { BOOST_MPL_ASSERT((proto::matches<Expr, MyGrammar<> >)); } template<typename Expr> void assert_not_valid(Expr const &) { BOOST_MPL_ASSERT_NOT((proto::matches<Expr, MyGrammar<> >)); } int main() { proto::terminal<std::vector<int> >::type vi; proto::terminal<int[4]>::type ai = {{1,2,3,4}}; proto::terminal<int>::type i = {0}; assert_valid(i + i * 1); assert_valid(vi[1]); assert_valid((vi + ai)[1]); assert_valid(i + (vi + ai)[1]); assert_not_valid(i + vi + ai); assert_not_valid((i + vi)[1]); }
Is it the way to go? I didn't see any reference to this in the documentation, but this seems a common pattern to me? The proto grammar would only check the 'parsing' and one transform would check the semantic.
A yes, I would add two note which seem relevant for VS user: * the compiler flag -Zm (for example -Zm150) to change the maximum size of the precompiled header * the pragma inline_recursion and inline_depth (default off and 8)
OK.
- What is your evaluation of the potential usefulness of the library?
Extremely useful.
- Did you try to use the library? With what compiler? Did you have any problems?
I used it with VS2005. I didn’t had any problem (at least not one which wasn’t my fault)
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I read the doc and started writing a small test. Didn’t had the time to try transform.
Thanks for your feedback. -- Eric Niebler Boost Consulting www.boost-consulting.com