Re: [boost] [Review.Contract] Vicente's review

Subject: Re: [boost] [Review.Contract] Vicente's review
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2012-09-03 02:14:45

Le 03/09/12 00:14, Lorenzo Caminiti a écrit :
> On Sun, Sep 2, 2012 at 2:39 PM, Vicente J. Botet Escriba
> <vicente.botet_at_[hidden]> wrote:
>> Hi,
>>
>> here is my short review.
>>
>> My vote is YES subject to these condition.
> Thanks a lot for the review.
>
>> * Removal of the Concepts part.
>> The use of Boost.ConceptCheck to emulate C++ concepts proposal is in my
>> opinion erroneous. Boost.Concept.Check is used to assert the actual
>> parameter satisfy statically the concept conditions, while if I'm not wrong
>> the C++ Concepts proposal use the requires clause to state the conditions of
>> a possible instantiations. Using the same grammar with different semantics
>> is confusing. In a last resort, if this feature is preserved, the requires
>> keyword must be changed by another one.
> If that were to be the final decision, I'll remove the interface to
> Boost.ConceptCheck or I can use any (alphanumeric) mane rather than
> requires (e.g., where).
>
> However, I can use the requires clausule to do multiple things:
> 1. Check concepts defined using Boost.ConceptCheck (current, I'd leave
> it as is).
> 2. Check static_assert (under consideration, to be moved here from
> within pre/post/inv).
> 3. Check Boost.Generic-like concepts implemented using C++11
> expression SFINAE (for future... this will be *a lot* of work...).
> I should be able to support of all those because I can distinguish 2
> using the pp (starts with static_assert)
I will really prefer the static_assert goes outside requires, e.g. in a 
new *check* clause. This clause could also contain the ConceptCheck 
part. In this way it is clear that the requires part will disable the 
instantiation if not satisfied, and the check part will report a 
compile-time error if not satisfied.
> and I can distinguish between
> 1 and 2 using the compiler for example using a (hidden) tagging base
> class to all concepts defined for 3.
I'm not sure this distinction is possible. For function templates, the 
requires part associated to ConceptCheck should be included in the 
function definition, while the one associated to enable_if/SFINAE c++ 
proposal concepts emulation should be part of the C++ declaration. For 
template classes, ConceptCheck goes to the members part, while Concepts 
goes to the template parameter part.

As you can see the semantics are different, so a specific keyword would 
help to identify the difference.
>
> IMO, I'd stick with requires, leave 1 as is, maybe add 2 for now, and
> consider 3 for future.
>
>> * static assertions are removed from the pre/post conditions. They can be
>> moved to a specific "static assertions" section at the same level than the
>> requires clause. This section could also include the concept check included
>> now in the requires clause.
> OK (but I'd personally bundle all into requires).
>
>> * The library extends the EDSL to use directly C++11 features, in particular
>> move semantics.
>> Now that since the C++11 standard is released, more and more people are
>> using C++11 specific features. I, in particular, will not use Contract if I
>> can not make use of C++11 features provided by the compiler.
> I'd prefer to add C++11 in future releases because the lib is usable
> as is for C++03 and adding the C++11 features will take time but it
> won't break the interface. Of course, I'll comply with whatever the
> final decision is.
>
>> * The contract broken handlers are noexcept, so that adding contracts don't
>> change the original prototype.
> They will be when C++11 support is added (again, I'd prefer later but
> I'm OK otherwise even if that will delay a possible release).
If you are allowed to release without C++11 support, it will be great if 
the documentation states clearly that the contract broken handlers can 
not throw. Otherwise, when moving to C++11, the C++03 usage will be 
broken. Even if you don't take care of C++11 features for the functions 
declaration, you could request that the contract broken handlers are 
noexcept using the Boost macro BOOST_NOEXCEPT, which is empty in C++03.

Best,
Vicente