[boost] Re: Boost.Assertion

From: Jesse Jones (jesjones_at_[hidden])
Date: 2001-04-17 21:23:32

>I've uploaded an updated header and a test file.

I just looked it over and I have some comments for you:

1) I think it'd be a good idea if Stack tested to see if nested 
checks work properly. For example:

     int& top()
     {
         BOOST_CHECK_INVARIANTS();
         return _stack[_top];
     }

     void push(int i)
     {
         BOOST_REQUIRE(_top < _capacity);
         BOOST_CHECK_INVARIANTS();
         BOOST_OBSERVE(unsigned, _top);

         do_push(i);

         BOOST_ENSURE(BOOST_OLD(_top) == _top - 1);
         BOOST_ENSURE(_stack[_top - 1] == i);
     }

private:
     void do_push(int i)
     {
         // break the invariant
         unsigned old_cap = _capacity;
         _capacity = 0;
        
         // call a public method: invariant should not fire
         top() = i;
         ++_top;
        
         // restore the invariant
         _capacity = old_cap;
     }

2) If you make this change you'll see that the check_invariants 
function is buggy. It should look more like this:
         public: inline void check_invariants(bool entering) const { \
             unsigned& count = _boost_invariant_count.get(); \
             if (entering) { if (++count > 1) return; } \
             if (!entering) { if (--count > 0) return; }
(I also switched the argument to a bool, but that's no big deal).

3) The check_invariants class is overly complex and does heap 
allocations which is pretty nasty. You can fix that by using that 
voodoo I mentioned earlier:
     class check_invariants {
     public:
         template <typename T>
         check_invariants(const T* object) {
             _object = object;
             _invoker = invoke<T>;
             _invoker(_object, true);
         }
         ~check_invariants() {
             _invoker(_object, false);
         }

         template <class T>
         static void invoke(const void* object, bool entering)    {
             void (T::*method)(bool) const = &T::check_invariants;
             (static_cast<const T*>(object)->*method)(entering);
         }

     private:
         void (*_invoker)(const void*, bool);
         const void* _object;
     };

4) It'd be nice if main printed a message for the expected errors. 
You might also want to wrap all of main in a try/catch block to catch 
any unexpected errors.

5) The code is setup so that the invariant is always an inline method 
in the header. I really don't like this: invariants are often too big 
to be good candidates for inlining and I don't want to touch a header 
every time I change an invariant.

One possible solution is to get rid of BOOST_BEGIN_INVARIANTS 
/BOOST_END_INVARIANTS, and add BOOST_SETUP_INVARAINT:
     #define BOOST_SETUP_INVARAINT    \
         mutable boost::invariant_count _boost_invariant_count;    \
         void check_invariants(bool entering) const { \
             unsigned& count = _boost_invariant_count.get(); \
             if (entering) { if (++count > 1) return; } \
             if (!entering) { if (--count > 0) return; } \
             this->invariant(); \
        }

In the header clients have to add " BOOST_SETUP_INVARAINT;" and 
declare the invariant method. In the cpp they define the invariant 
method as a normal method and use BOOST_INVARIANT to do the checks.

   -- Jesse