AMDG On 09/02/2012 10:16 AM, Lorenzo Caminiti wrote:
On Sat, Sep 1, 2012 at 2:59 PM, Steven Watanabe <watanabesj@gmail.com> wrote:
index.html:
- I don't understand why the library needs to deal with access control. Why can't the user just use public: private: protected: as usual?
The user can use the access specifiers outside the macros:
CONTRACT_CLASS( class (x) ) { CONTRACT_CLASS_INVARIANT( void )
protected: typedef int size_type; // access specifier outside macros
CONTRACT_FUNCTION( public void (f) ( void ) // access specifier in macros ) {} };
However, the access specifiers also needs to be in the macros because private and protected functions do not check class invariants. Therefore, the macros need to know if the function being contracted is public of not so to expand the invariant checking code or not. See note [26]: http://contractpp.svn.sourceforge.net/viewvc/contractpp/releases/contractpp_...
I think it would be better to make this explicit like noinvariant instead. First of all, the fact that a member is not public doesn't necessarily imply that it doesn't require the invariants to hold. Second, I don't like the idea of making the semantics depend on access control. It goes against the way access control works in the core language. I think that code written using this library needs to be understandable without having to read the documentation carefully and features that change the behavior in non-obvious ways militate against this.
- CONTRACT_OLDOF kind of sticks out at me. I don't really like the way it looks having it a macro when everything else is keywords that are parsed by the top level macros.
I know but there's no way to parse it as a keyword because it's nested within the assignment statement. It being a macro is part of the mechanism to handle the `=` symbol:
[auto|fundamental_type|(type)] variable_name = CONTRACT_OLDOF expression
The pp parses this into 3 traits [auto|fundamental_type|(type)], variable_name =, and expression. The type is either auto, known (fundamental type like int, long double, etc), or wrapped within parenthesis so it can be parsed and stripped away from the front. Then the CONTRACT_OLDOF macro essentially expands to )( so it allows me to separate variable_name = from expression (using pp sequences) that otherwise I couldn't separated because they are unknown tokens plus they contain non-alphanumeric symbols (so I can't use concatenation to handle them).
Okay, so you can't parse the = with the preprocessor. I think you should really consider getting rid of the = entirely then. You've already replaced '=' for default parameters with a keyword. Something along the same lines would be more consistent with the rest of your grammar.
- "The implementation of this library uses..., templates with partial specializations and function pointers (similarly to Boost.Function), ..." I don't understand the connection between partial specialization and function pointers. Why are they grouped together?
No particular connection, I was just trying to say that my lib implementation uses both... among other things that need to be supported by the compiler.
In that case, I would separate them with a ',' instead of "and," so they're separate entries in the outer list.
- "1. The extra processing required to check the assertions. 2. The extra processing required by the additional function calls (additional functions are invoked to check preconditions, postconditions, class invariants, etc)." How is (2) different from (1)?
Because there is a cost in the assertion instruction itself plus the cost of calling the function that checks pre/post/inv:
void f_pre ( ... ) { // cost of calling f_pre (2) assert(...); // cost of executing the assertions (1) assert(...); ... }
I din't necessarily had to put all pre/post/inv in separate functions (even if that's really the only sensible way to implement this especially when subcontracting comes into play because then you need to access the base pre/post/inv separately from the base function body).
You can just fold the function call overhead into the cost of checking the assertions, unless you want to be really pedantic. Even then, the compiler can eliminate it by inlining.
- "int const" IIRC, you can't have a const rvalue of a built-in type.
I need to review the lib and the docs for C++11 features... rvalue included.
rvalues aren't a C++11 feature.
What do you think of this bit of PP magic:
#define TEST_EXPAND_EXPAND_OLDOF(x) CONTRACT_OLDOF #define TEST_EXPAND_NOTHING(x) oldof #define TEST_EXPAND_I(x) TEST_EXPAND_ ## x #define TEST_EXPAND(x) TEST_EXPAND_I(x) #define oldof TEST_EXPAND(EXPAND_OLDOF(NOTHING(z))) #define EXPAND_OLDOF(expr) expr
oldof -> expands to itself EXPAND_OLDOF(random text oldof more random text) -> expands to random text CONTRACT_OLDOF more random text
Hum, tricky... this works on MSVC but not on GCC :(
Unfortunately, GCC is correct and there's no way to make it work.
Also with the current implementation of the lib #define oldof CONTRACT_OLDOF won't work...
The only reason is doesn't work is because the name oldof is used by the library. If I use #define myoldof CONTRACT_OLDOF it works fine. In Christ, Steven Watanabe