On Sun, Aug 26, 2012 at 10:04 AM, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr> wrote:
Le 26/08/12 11:16, Vicente J. Botet Escriba a écrit : Hi,
First of all, thanks Lorenzo for your work on pre-processor programming. With this library, you have showed to me how far the pre-processing can go in terms of expressiveness.
I have some questions before doing a complete the review:
Hi again,
more comments and questions follows
= member initializers =
Why the following limitation? "Unfortunately, when member initializers are specified, the constructor body must be defined together with its declaration and contract."
See note 34: http://contractpp.svn.sourceforge.net/viewvc/contractpp/releases/contractpp_...
= C++03 limitations =
It would be great if you add a section/table with all the limitations, as e.g. these ones
Will do.
* Function and array types cannot be directly used as function parameter types within the contract macros ... * Each function parameter must always specify both its type and its name (parameter names can instead by omitted in usual C++ declarations). * All tokens must be specified in the fixed order listed in the Grammar section (it is not possible to specify postconditions before preconditions, |volatile| before |const| for member functions, etc). * Unfortunately, this library does not allow to specify contracts for unions. * Unfortunately, when member initializers are specified, the constructor body must be defined together with its declaration and contract
and use some kind of as Warning or Important when documenting them.
= use of deferred =
IMO deferred is related to the time dimension, while I think you are using it on the space dimension. "The function body definition can be separated" is OK, but deferred :(
I tough deferred is standard terminology in this context (I even double-check it somewhere a couple of years back)... maybe I'm wrong... I'm happy to use whatever the standard terminology is for a function that is defined in a place different from which it is first declared.
= _BODY macros =
Why do you need to make a difference between CONTRACT_FREE_BODYand CONTRACT_MEMBER_BODY while there is a single CONTRACT_FUNCTION?
See note 32: http://contractpp.svn.sourceforge.net/viewvc/contractpp/releases/contractpp_...
What if the user uses CONTRACT_FREE_BODYwhen it should use CONTRACT_MEMBER_BODY? It will not have the associated class invariants.
No, but when you partially disable contracts you might get compiler-errors like "function ...XbodyXpush_back is not defined...".
The user will see this error very late when she realize that the function is breaking a class invariant (by other means). I think it will be better if the syntax forces an error in this case. What about
template< typename T, T Default > bool CONTRACT_MEMBER_BODY((natural<T, Default>),equal) ( natural const& right ) const { return not less(*this, right) && not greater(*this, right); }
This will make more homogeneous |CONTRACT_CONSTRUCTOR_BODY|, |CONTRACT_DESTRUCTOR_BODY|, and |CONTRACT_MEMBER_BODY.| What do you think?
It used to be MEMBER_BODY(func_name), then it was MEMBER_BODY(class_type, func_name), and now it's back to MEMBER_BODY(func_name) ;) See note 47: http://contractpp.svn.sourceforge.net/viewvc/contractpp/releases/contractpp_...
= ( void ) || empty macro parameters = | For projects that don't care about portability, or just use compilers that supports the | empty macro parameters it will be great if they can use () instead of ( void ) to represent no parameters. Could you provide this?
On a decent pp (g++), ( ) should work instead of ( void ). The lib implementation should supports both (even if this complicates the implementation and I didn't fully test () ) -- I'll state this in the docs.
= _TPL macros and performance =
Could you give some figures of the gain of the change of introducing the _TPL macros?
No, I don't have the numbers :( Now it'll be too much work to implement the without _TPL case to compare... However, the without _TPL implementation created an extra 4 template functions for each user defined functions. In the _TPL case the extra 4 functions are template functions only when the user function is also a template.
= Static assertions =
What is the interest of disabling static assertion when |CONTRACT_CONFIG_NO_PRECONDITIONS|, |CONTRACT_CONFIG_NO_POSTCONDITIONS| are defined?
Not sure... I wondered this myself. I guess such static assertions for pre/post/inv are a new feature so it's just my best guess. Allowing to disable a precondition static assertion when you disable precondition made some sense to me -- otherwise, what will be the difference between a static assertion in preconditions, postconditions, or class invariants? or even within the body itself? However, I thought about this and I wan't 100% sure either way... I'm happy to discuss this requirement more.
= return =
^" *Rationale.* The result type needs to be copyable in order for the function itself to return a value so this library can always evaluate return value declarations."
This is only true if rvalue references are not supported. Otherwise, for MovableOnly types the variable used to store the return value could be a rvalue reference and move semantics should be applied. Even if you don't support C++11, the user can use Boost.Move.
I think so. I don't mention rvalues at all (emulated nor C++11) in the docs... I can add a few notes about rvalues when supporting C++11. Thanks, --Lorenzo