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

Subject: Re: [boost] [Review.Contract] Vicente's review
From: Lorenzo Caminiti (lorcaminiti_at_[hidden])
Date: 2012-09-04 11:22:50

On Tue, Sep 4, 2012 at 4:01 AM, Vicente J. Botet Escriba
<vicente.botet_at_[hidden]> wrote:
> Le 04/09/12 03:01, Lorenzo Caminiti a écrit :
>
>> On Sun, Sep 2, 2012 at 11:14 PM, Vicente J. Botet Escriba
>> <vicente.botet_at_[hidden]> wrote:
>>>
>>> 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.
>>
>> Can you provide an example of how all of this would ideally look like?
>> You can also pick a "keyword" other than requires that makes sense to
>> you and then indicate where to use requires and where to use the other
>> keyword in the example. Maybe you start from the static_assert example
>> I gave:
>>
>>      template< typename To, typename From >
>>          requires // new concepts??
>>              Convertible<From, To> // disable this declaration using
>> SFINAE??
>>          unless // static_assert and Boost.ConceptCheck??
>>              static_assert(sizeof(To) >= sizeof(From), "destination too
>> small"), // generate compiler-error??
>>              boost::Copyable<From> // generate compiler-error??
>>      To* memcopy ( To* to, From* from )
>>          precondition {
>>              to; // pointer not null
>>              from; // pointer not null
>>          }
>>      {
>>          // ...
>>      }
>>
>
> I will use *check* as keyword for the static_assert and ConceptCheck part
>
>
>     template< typename To, typename From >
>     requires // used with enable_if in the declaration part
>             is_convertible<From, To>::value
>     check // included in the function body
>
>             assert(sizeof(To) >= sizeof(From), "destination too small")
>          && boost::Copyable<From>
>
>     To* memcopy ( To* to, From* from )
>         precondition {
>             to; // pointer not null
>             from; // pointer not null
>         }
>     {
>         // ...
>     }
>
>
> You could also use the check
>
> sizeof(To) >= sizeof(From)
>
> in the requires part if you wanted to disable overload resolution when the
> condition fails as in
>
>
>     template< typename To, typename From >
>     requires // used with enable_if in the declaration part
>             is_convertible<From, To>::value
>          && sizeof(To) >= sizeof(From)
>     check // included in the function body
>             boost::Copyable<From>
>
>     To* memcopy ( To* to, From* from )
>     // ...
>
>
> Note however that the Concept Copyable from ConceptCheck can not be used in
> the requires clause (I'm looking for), it is not a bool metafunction (or a
> trait). It just compile fail when the parameter doesn't models the concept.

This makes sense to me and, as far as I can tell, it is also in line
with Andrzej suggestion. I'm happy to implement it this way.

CONTRACT_FUNCTION(
    template( typename To, typename From )
        requires( // used with enable_if in the declaration part
             is_convertible<From, To>::value,
             is_convertible<To, From>::value
        )
        check( // included in the function body
            static_assert(sizeof(To) >= sizeof(From), "destination too small"),
            boost::Copyable<From>
        )
    (To*) memcopy ( (To*) to, (From*) from )
        precondition(
            to; // pointer not null
            from; // pointer not null
        )
) {
    // ...
}

I will implement the check clausule now and the require one later and
only if I'm able to extend the lib to declare and check
Boost.Generic-like concepts.

Just to consider all alternatives, we could also do this:

CONTRACT_FUNCTION(
    template( typename To, typename From )
        if( // used with enable_if in the declaration part
             is_convertible<From, To>::value,
             is_convertible<To, From>::value
        )
        requires( // included in the function body
            static_assert(sizeof(To) >= sizeof(From), "destination too small"),
            boost::Copyable<From>
        )
    (To*) memcopy ( (To*) to, (From*) from )
        precondition(
            to; // pointer not null
            from; // pointer not null
        )
) {
    // ...
}

Given that the 1st clausule is more like enable-if and the 2nd
clausule states "hard" requirements (in that you get a "hard" compiler
errors when the requirements are not met). But I'm happy with
requires/check and if/requires the same way (and I still haven't read
N3331 which might suggest a syntax vs another...).

Thanks,
--Lorenzo