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

Subject: Re: [boost] [Review:Contract] Some questions
From: Lorenzo Caminiti (lorcaminiti_at_[hidden])
Date: 2012-08-27 16:05:51

On Mon, Aug 27, 2012 at 10:10 AM, Vicente J. Botet Escriba
<vicente.botet_at_[hidden]> wrote:
> 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.

Will do.

>>> 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.

This might take me a couple of days because I'm busy with something
else right now... stay tuned.

>>>>> = 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.

I'm not sure about the warnings... but I'll just support no parameter
names, it shouldn't be hard, it'll allow to avoid extra parenthesis
around types when no name is used, and it's needed for concept
definitions (if I'll ever get around implementing those). I entered a
ticket for it: https://sourceforge.net/apps/trac/contractpp/ticket/63

>>>>> * 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.

I'm not sure if I can catch this error but if I can, I will. I've
added a ticket for it:
https://sourceforge.net/apps/trac/contractpp/ticket/64

>>>>> 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)

Take a look at this example:
http://contractpp.svn.sourceforge.net/viewvc/contractpp/releases/contractpp_0_4_1/example/contracts/default_subcontracting_base.cpp

This can be done (note no CONTRACT_CLASS/FUNCTION macros so usual C++
declaration syntax):

// base declared with contracts...

class deriv : public base
{
    public: virtual void CONTRACT_MEMBER_BODY(f) ( void )
    {
        std::clog << "deriv::f::body" << std::endl;
    }
};

But this cannot be done:

class deriv : public base
{
    public: virtual void CONTRACT_MEMBER_BODY(deriv, f) ( void )
    {
        std::clog << "deriv::f::body" << std::endl;
    }
};

Because at some point, BODY(deriv, f) has to expand to
deriv::contractXbodyXf (or just deriv::f is member function contracts
are disabled when no pre, no post, and no class inv) and you cannot
prefix a member function with the class name within the class
declaration itself (plus it'd be redundant to repeat deriv in the BODY
macro within deriv's declaration).

>> 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.

Again, if reviewers ask, I'll support. Note that the fact that a major
compiler like MSVC messes this up will inevitably make the ( ) syntax
useless for a large number of users :( even if I endure the effort and
point to support it and fully test it.

>>>>> = _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.

OK, I understand. My answer is "I'm not sure...": In pre/post/inv you
can write assertions and I wanted to support static assertions so if
some of these correctness conditions can be checked at compile-time,
they will. I think if contracts were added to C++1x, we'd expect to
naturally be able to use static_assert in the pre/post/inv. Older rev
of N1962 supported static assertions and N1962 does not because
static_assert was added to C++11:
http://www.open-std.org/JTC1/SC22/WG21/docs/papers/2005/n1866.html#static-assertions

However, I don't have a use case that shows static assertions are
critical for contract programming (only a few examples in the Example
section, search for static_assert). On the contrary, most of contract
assertions you want/need to write can only be checked at run-time.

>>>>> = 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? ;-)

Yes ;)

Thanks.
--Lorenzo