First let me apologize for the short review. It won't make justice to the huge contribution Eric made to the Boost Libraries. I definitely think Proto should be accepted and included into an official Boost release as soon as practical. - What is your evaluation of the design? Beautiful. - What is your evaluation of the implementation? Lot of attention has been put into making sure that compilation time stayed with reason, not an easy task with today's compilers. Also, to the extent it didn't conflict with the compilation speed goal, Eric made substantial restructuring of the code to prevent unwanted captures due to ADL and otherwise reduce surprises for users. I haven't really worked with the version submitted for review, but one of the issue I did have with v2 was that and_ and or_ didn't allow for 1 and 0 arguments, which is useful in a variety of folding situations. I remember Eric agreeing in the end that this could be indeed useful in some cases and the problem could be compilation time. I don't know if it has been solved one way or the other: a cursory look at the new documentation doesn't show any minimum arity (one thing that the docs should do, irrespective on whether the minimum is 0,1 or 2) Another issue I remember was related to BOOST_PROTO_MAX_ARITY. It is (was?) controlling both the maximum arity of boost expressions _and_ the arity of templates in grammars. Eric promised to decouple the two uses, but in my quick look at the docs I couldn't find out whether it has happened or not. I believe this change should really go in. The last design issue I had was about whether expressions should allow to store terminals by value. This might be different with the work on integrating fusion, I don't know. In my application (fixed-point arithmetic) I had cases where my object could have been stored by value and taking the space of an int (or less) and I would have liked to be able to do so. I admit mine was probably a very special case, where the quality of the assembler code generated was of upmost importance (must rival hand-written code). I don't remember the details of the conversation with Eric on this, but I think his conclusion was that allowing the user to specify the storage policy would have made compilation in the typical case slower. Still... - What is your evaluation of the documentation? Haven't had to much time to spend on it, but it seems to have improved substantially. Once proto gets in the hand of new users we'll know whether it does a good job in introducing complex concepts which are probably new to the average C++ programmer. - What is your evaluation of the potential usefulness of the library? Huge. - Did you try to use the library? With what compiler? Did you have any problems? I did use the previous version from April to October 2007 and have been a regular harrasser of Eric. Compilers: - GCC, many versions including SVN heads - Intel C++ compiler, few 9.x.y versions Problems: a few in the beginning, promptly fixed by Eric. I remember the Intel compiler exposed a number of issues. - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? the review itself consisted of a quick glance to the new documentations. The work with the previous version of the library was 3-4 months (FTE) - Are you knowledgeable about the problem domain? I believe so.
Maurizio Vitale wrote:
First let me apologize for the short review. It won't make justice to the huge contribution Eric made to the Boost Libraries.
I definitely think Proto should be accepted and included into an official Boost release as soon as practical.
Thanks.
- What is your evaluation of the design? Beautiful.
- What is your evaluation of the implementation? ... I haven't really worked with the version submitted for review, but one of the issue I did have with v2 was that and_ and or_ didn't allow for 1 and 0 arguments, which is useful in a variety of folding situations. I remember Eric agreeing in the end that this could be indeed useful in some cases and the problem could be compilation time. I don't know if it has been solved one way or the other:
Not yet. It's on my list. I'll fix it post-review.
Another issue I remember was related to BOOST_PROTO_MAX_ARITY. It is (was?) controlling both the maximum arity of boost expressions _and_ the arity of templates in grammars. Eric promised to decouple the two uses, but in my quick look at the docs I couldn't find out whether it has happened or not. I believe this change should really go in.
Refresh my memory ... what do you mean by, "the arity of templates in grammars"? Are you referring to the number of template parameters that proto::or_ and proto::and_ accept? If so, then yes, you can control this separately with BOOST_PROTO_MAX_LOGICAL_ARITY (not documented, darn!). If you mean the max arity of expressions like terminal< MyGinormousTemplate<_,_,_,_,_,_,_,_> > then no, this cannot be controlled independently. Do you want that?
The last design issue I had was about whether expressions should allow to store terminals by value. This might be different with the work on integrating fusion, I don't know. In my application (fixed-point arithmetic) I had cases where my object could have been stored by value and taking the space of an int (or less) and I would have liked to be able to do so. I admit mine was probably a very special case, where the quality of the assembler code generated was of upmost importance (must rival hand-written code). I don't remember the details of the conversation with Eric on this, but I think his conclusion was that allowing the user to specify the storage policy would have made compilation in the typical case slower. Still...
There is a way to get what you want today, using a custom generator. If you want, I can show you how. If it doesn't meet your needs, I'm open to considering alternatives. <snip rest> Thanks for the feedback. -- Eric Niebler Boost Consulting www.boost-consulting.com
Eric Niebler <eric@boost-consulting.com> writes:
Another issue I remember was related to BOOST_PROTO_MAX_ARITY. It is (was?) controlling both the maximum arity of boost expressions _and_ the arity of templates in grammars. Eric promised to decouple the two uses, but in my quick look at the docs I couldn't find out whether it has happened or not. I believe this change should really go in.
Refresh my memory ... what do you mean by, "the arity of templates in grammars"? Are you referring to the number of template parameters that proto::or_ and proto::and_ accept? If so, then yes, you can control this separately with BOOST_PROTO_MAX_LOGICAL_ARITY (not documented, darn!). If you mean the max arity of expressions like
terminal< MyGinormousTemplate<_,_,_,_,_,_,_,_> >
then no, this cannot be controlled independently. Do you want that?
It is the latter. It bit me with two consecutive strikes: - I had no idea there was a limit, as it is not (I believe) mentioned anywhere - I didn't guess that the the controller was BOOST_PROTO_MAX_ARITY, as this is documented as controlling the maximum arity of proto expressions. I'd prefer to have a separate macro for each separate control (especially if there's a compile time gain in increasing only one). And since you have BOOST_PROTO_MAX_LOGICAL_ARITY I think it makes sense for orthogonality. What about BOOST_PROTO_MAX_TEMPLATE_ARITY. But at minimum the effects of BOOST_PROTO_MAX_ARITY should be fully documented. Seeing it mentioned might raise a red flag for people - if I remember correctly the error messages from GCC for this case weren't particularly enlightning. [I think you ruled out an explicit tests on compilation speed grounds]
The last design issue I had was about whether expressions should allow to store terminals by value. This might be different with the work on integrating fusion, I don't know. In my application (fixed-point arithmetic) I had cases where my object could have been stored by value and taking the space of an int (or less) and I would have liked to be able to do so. I admit mine was probably a very special case, where the quality of the assembler code generated was of upmost importance (must rival hand-written code). I don't remember the details of the conversation with Eric on this, but I think his conclusion was that allowing the user to specify the storage policy would have made compilation in the typical case slower. Still...
There is a way to get what you want today, using a custom generator. If you want, I can show you how. If it doesn't meet your needs, I'm open to considering alternatives.
Not now, but in the summer I plan to get back to my fixed-point library. If I'll still have problems after studying the new implementation and documentation I'll ask. Thanks, Maurizio
Maurizio Vitale wrote:
Eric Niebler <eric@boost-consulting.com> writes:
If you mean the max arity of expressions like
terminal< MyGinormousTemplate<_,_,_,_,_,_,_,_> >
then no, this cannot be controlled independently. Do you want that?
What about BOOST_PROTO_MAX_TEMPLATE_ARITY.
Sounds good. -- Eric Niebler Boost Consulting www.boost-consulting.com
participants (2)
-
Eric Niebler -
Maurizio Vitale