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

Subject: Re: [boost] [Review:Contract] Some questions
From: Lorenzo Caminiti (lorcaminiti_at_[hidden])
Date: 2012-08-27 11:17:37

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

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

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

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

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

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

>>> * Unfortunately, when member initializers are specified, the constructor
>>> body must be defined together with its declaration and contract

Maybe using C++11 delegating constructors? Currently, marked for future work:
http://sourceforge.net/apps/trac/contractpp/ticket/51

But I'll send another email to discuss ideas and I could try to remove
this limitation if reviewers indicated it's important.

>>> 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.
>
> Maybe a native English could replay to this point.I'm not really sure my
> interpretation is correct.

I'll double check the C++ standard anyway.

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

>>> 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 :(

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

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

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

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

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

Thanks a lot!
--Lorenzo