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

Subject: [boost] [Review.Contract] Vicente's review
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2012-09-02 17:39:59

Hi,

here is my short review.

My vote is YES subject to these condition.

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

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

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

* The contract broken handlers are noexcept, so that adding contracts 
don't change the original prototype.

=== STANDARD REVIEW QUESTIONS ===

* What is your evaluation of the design?

While Lorenzo has show that using the preprocessor we can define a EDSL that is close to the native syntax (besides the parentheses stuff), the resulting interface is *closed* and interacts badly with c++ features not taken in account by the EDSL. I'm guessing that the user can not extend this EDSL without modifying the library. This is also the case of other libraries that make use of the pre-procesor to define a EDSL as Boost.Parameters or Toward Boost.Generics.

It will be great if the user can extend the EDSL, but I suppose this could be very hard, if not impossible.

I understand that Lorenzo is trying to reduce the preprocessor time using whatever is needed, but making the interface more complex would not help to improve the usability. I'm not sure the _TPL macros are really needed and whether the resulting code could perform better or not.

* What is your evaluation of the implementation?

I've not look inside.

* What is your evaluation of the documentation?
Very good.

I would appreciate if the documentation includes a sections showing what is behind the scenes, what is generated by these macros.

It would be great if you add a section/table with all the limitations, as e.g. these ones
       
     * Function and array types cannot be directly used as function
     parameter types within the contract macros ...

     
     * Each function parameter must always specify both its type and its
     name (parameter names can instead by omitted in usual C++
     declarations).

     * All tokens must be specified in the fixed order listed in theGrammar  section (it is not possible to specify
     postconditions before preconditions,|volatile|  before|const|
     for member functions, etc).

     * Unfortunately, this library does not allow to specify contracts for
     unions.

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

* What is your evaluation of the potential usefulness of the library?

I don't think the library could be used in a industrial context.

The domain of this preprocessor EDSL is too large (C++ declarations + contracts + named parameters + concepts) and we will need to expand it yet to be useful (c++11, compiler specifics, ...). While the syntax is quite close to C++ syntax the library intends to emulate, the compiler errors will be quite unreadables.

* Did you try to use the library? With what compiler? Did you have any problems?

No.
  
* How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I have not do a in-depth study. I has been concentrated inspecting the usability of the library and trying to identify the limitations it could introduce.

* Are you knowledgeable about the problem domain?

Yes, from a theoretical point of view. I have never used contract programming in real world projects.

Best,
Vicente