On Sun, Sep 2, 2012 at 4:15 PM, Steven Watanabe <watanabesj@gmail.com> wrote:
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.
This is possible however N1962, Eiffel, D, A++, etc all automatically disable class invariant checking for non-public member function calls.
First of all, the fact that a member is not public doesn't necessarily imply that it doesn't require the invariants to hold.
From http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n1962.html#class-inv...
... The class invariant has the following properties: ... 3. It is called implicitly from public ....
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.
I agree that code written using this library should to be understandable without having to read the documentation carefully but /by programmers that understand Contract Programming/. That only public member functions check class invariants is not something special about the library. It's a Contract Programming requirement so users should be familiar with it a-priori (or by reading the Contract Programming Overview section). It's like knowing that preconditions are executed before body execution, or that old-values are copied before body execution, or that subcontracting checks precondition in logic-or, etc. All of this semantic should be known to the users, they can learn it from Contract Programming Overview section, but it's really all about Contract Programming and there's nothing special about the lib here (the syntax instead for example is special about the lib).
- 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.
For default parameters the = is replaced by ", default" and "," plays a key role. I'd have to replace: auto old_size = CONTRACT_OLDOF size() // (1) with something like: auto old_size, as size() // (2) I personally prefer (1) because more readable even you are correct that technically (2) (even better without the "as") would be more consistent with the rest of the 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.
OK.
- "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.
OK.
- "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.
Yeah, sorry... I don't know what I was thinking of... I'll fix the int const. https://sourceforge.net/apps/trac/contractpp/ticket/75
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.
My bad for not investigating the root cause of the errors I was getting. Then I'll fix this so users that don't like CONTRACT_OLDOF can just do #define oldof CONTRACT_OLDOF ans use oldof within the postcondition code :) https://sourceforge.net/apps/trac/contractpp/ticket/76 Thanks a lot! --Lorenzo