Re: [boost] [Review:Contract] Some questions

Subject: Re: [boost] [Review:Contract] Some questions
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2012-08-27 13:10:15

Le 27/08/12 17:17, Lorenzo Caminiti a écrit :
> On Sun, Aug 26, 2012 at 11:33 PM, Vicente J. Botet Escriba
> <vicente.botet_at_[hidden]> wrote:
>> Le 27/08/12 03:20, Lorenzo Caminiti a écrit :
>>
>>> On Sun, Aug 26, 2012 at 10:04 AM, Vicente J. Botet Escriba
>>> <vicente.botet_at_[hidden]> 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_0_4_1/doc/html/contract__/tutorial.html#ftn.contract__.tutorial.forward_declarations_and_body_definitions.f3
>> I have read these notes. This seems to me a *hard* limitation and I would
> Why "hard"? (For example, take a look to what Boost.Parameter requires
> to deal with constructs...)
>
> You can just program the constructors with member initializers in the
> class declarations when using contracts:
>
> CONTRACT_CLASS(
>      class (x)
> ) {
>      CONTRACT_CLASS_INVARIANT( void )
>
>      CONTRACT_CONSTRUCTOR(
>          public (x) ( void )
>              initialize( a_(1), b_(2), c_(3) )
>      ) {
>          ... // body right here
>      }
>
>      private: int a_, b_, c_;
> };
>
> If the body is really just too complex for an header... then you can
> always separate it with an init function (essentially, only the
> initializer list remains in the class declaration):
>
> CONTRACT_CLASS(
>      class (x)
> ) {
>      CONTRACT_CLASS_INVARIANT( void )
>
>      CONTRACT_CONSTRUCTOR(
>          public (x) ( void )
>              initialize( a_(1), b_(2), c_(3) )
>      ) {
>          init(); // simple body
>      }
>
>      private: void init ( void ); // deferred
>
>      private: int a_, b_, c_;
> };
>
> // possibly in another file...
> void x::init ( void )
> {
>      ... // body now here (far from the declaration)
> }
You are right, hard was to strong ;-) Maybe the workaround merits to be 
include in the documentation.
>> like you explain here deeply and broadly why, maybe someone could give you
>> some ideas on alternatives.
> With C++11 delegating constructors, this limitation /could/ be removed
> -- but I'm not sure because I didn't try. I will follow up with a
> separate email thread listing some snapshot of the generated code so
> we can discuss ideas and details :)
Yes, showing the generated code will help.
>
>>>> = C++03 limitations =
>>>>
>>>> It would be great if you add a section/table with all the limitations, as
>>>> e.g. these ones
>>> Will do.
>> Maybe you could already replay by completing the list below, and for each
>> one if there is a possibility/plan to remove it.
> The list below should be complete.
>
>>>> * Function and array types cannot be directly used as function parameter
>>>> types within the contract macros ...
> Can't remove this limitation because C++ declaration syntax mixes
> types and names for these parameters :(
>
> void f ( int* (*fptr) ( int, int ), int* array[2][3] )
>
> Names are within the types... instead of <type> <name>. The pp simply
> can't go into such token expressions and extract the names unless I
> add parenthesis to the expressions:
>
> CONTRACT_FUNCTION(
>      void (f) ( (int*) ((*)fptr) ( int, int ), (int*) (array)[2][3] )
> ) ...
>
> General grammar:
>      Function parameters: (function-result-type)
> ([(*)|(&)]function-parameter-name) ( function-parameters )
>      Array parameters: (array-type) (array-parameter-name) [array-dimension]...
>
> IMO, that's a more complex syntax than using the typedef workaround.
I agree.
>
>>>> * Each function parameter must always specify both its type and its name
>>>> (parameter names can instead by omitted in usual C++ declarations).
> This can be removed (just more pp code). Furthermore, if no name is
> specified, the type can but it does not have to be wrapped by extra
> parenthesis. I must do this if I extend the lib for concept
> definitions and I could do this in any case if reviewers asked for it.
I can live with this limitation as soon as we don't get warnings when 
the parameter is not used.
>
>>>> * 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).
> Removing this limitation will complicate the pp parsing code
> significantly (because I need to handle all the combinations, probably
> also increasing preprocessing-time) without adding any functionality.
> It can be done for sure but I've no plan to do this.
I can live with this limitation.
>>>> * Unfortunately, this library does not allow to specify contracts for
>>>> unions.
> I'm not sure... I could look into this if reviewers indicated it is
> important. Also unions changed in C++11... I don't remember how...
> This is planned for future work:
> http://sourceforge.net/apps/trac/contractpp/ticket/50
I can live with this limitation.
>
>>>> = _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_0_4_1/doc/html/contract__/tutorial.html#ftn.contract__.tutorial.forward_declarations_and_body_definitions.f1
>> This let me think that for uniformity it would be an alternative to have
>> CONTACT_MEMBER and CONTRACT_FREE or maybe CONTACT_MEMBER_FUNCTION and
>> CONTRACT_FREE_FUNCTION.
> I don't thinks so but it's largely a matter of personal taste... the
> BODY macros do something different from the FUNCTION macros (i.e.g,
> BODY = definition, FUNCTION = declaration) so IMO it's OK if the names
> are not symmetric. I like that the FUNCTION macro is smart enough to
> automatically distinguish between free and member function
> declarations.

You are right, it is a matter of taste.
>
>>>> 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...".
>> This is not enough. The user would need to test her application using
>> several combinations, which is not admissible.
> Well, the user needs to use the correct macros as the docs say :)
> Ideally, I'd catch all syntax errors but I know I can't :(
I agree the library can not catch all syntax/semantic errors, but IMO 
the library can avoid this one, even if the consequences are not so 
important.
>
>>>> 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_0_4_1/doc/html/contract__/advanced_topics.html#ftn.contract__.advanced_topics.pure_virtual_functions.f0
>> I don't see how this note respond to my point. Could you clarify, please?
> This is possible:
>
> CONTRACT_MEMBER_BODY((natural<T,  Default>),equal)
>
> In fact, this was the API of this macro in 0.4.0. I changed it (back) to:
>
> natural<T,  Default>::CONTRACT_MEMBER_BODY(equal)
>
> In 0.4.1 because CONTRACT_MEMBER_BODY(equal) can be used if you want
> to derive a base class with contracts but without using the contract
> macros in programming the derived class (in this case the derived
> class has the exact same contracts as the base class). This is a nice
> feature because if I program a base class using contracts then other
> programmers can derive it and they are not forced to use contracts in
> programming the derived class. In other words, using contracts doesn't
> force neither the callers nor other programmers (namely the derived
> class programmers) to also use contracts.

Could you explain why the older form

CONTRACT_MEMBER_BODY((natural<T,  Default>),equal)

can not be used for a derived class

CONTRACT_MEMBER_BODY(DERIVED,equal)

> I think this feature is more
> important than the homogeneity between CONTRUCTOR/DESTRUCTRO/MEMBER
> BODY -- note that CONTRACTUOR/DESTRUCTOR and MEMBER/FREE have
> homogeneous interfaces, the mnemonic difference is between creation
> and functions instead than between members and free functions.
I must agree with you here.
>
>>>> = ( 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.
>> Do you plan to support it?
> Sure, I can if reviewers ask for it. IMO, ( void ) is good enough (but
> that's a matter of personal taste).
The fact that this is a matter of taste let me think that the library 
should implement (if this is not too complex) it so that the user can 
write code according to her taste.

>
>>>> = _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.
>> How many non-template functions are created for the non _TPL implementation?
> Always 4 extra functions:
>      _TPL: 4 extra functions which are templated only if the user
> function was a template.
>      non-_TPL: 4 extra functions which are always templated (even if
> the user function was not a template).
I'm not sure the fact the function are templates instead of inlined 
functions change a lot. But of course, it is up to you to provide a 
simpler interface.
> On a side note, it /might/ be possible to limit the number of extra
> functions to 2:
> http://sourceforge.net/apps/trac/contractpp/ticket/42
>
>>> 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.
>> Disabling preconditions/postconditions or invariants has a sense as these
>> are run-time checks, but static assertions as checked at compile-time.
> Yes, of course. But I was asking:
>
> CONTRACT_FUNCTION(
>      template( typename T )
>      void (f) ( (T const&) x )
>          precondition( static_assert(sizeof(T) > sizeof(int), "big") ) // (1)
>          postcondition( static_assert(2 * sizeof(T) > sizeof(long),
> "twice big") ) // (2)
> ) {
>      static_assert(sizeof(T) > sizeof(int), "big"); // (3)
>      ... // body
>      static_assert(2 * sizeof(T) > sizeof(long), "twice big"); // (4)
> }
>
> What would be the difference between (1), (2), and even (3) and (4) if
> CONTRACT_CONFIG_NO_PRECONDITION/POSTCONDITION did not disable (1) and
> (2)? (I'm not really sure about this... I'm happy to discuss.)
My concern was, what could be the interest of  introducing static 
assertions on the pre/post condition? Of course, if pre/post conditions 
are disabled, all the code included in (including static assertions) 
must be removed.
>>>> = 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.
>>>
>>>
>> As you are copying the return value, the user can not use move semantics,
>> and the library need to take care of this explicitly.
>> Do you plan to take care of move semantics before release (if accepted)?
> Sure if a reviewer ask for it ;)
Do you mean if, for example, I request it on a formal review? ;-)

Best,
Vicente