[boost] Possible regression in Boost.Test

Subject: [boost] Possible regression in Boost.Test
From: Ian Emmons (iemmons_at_[hidden])
Date: 2009-11-22 21:42:11

A previously reported issue in Boost.Test, which the bug tracking system indicates as fixed, is still present in the 1.41.0 release.  The issue I am speaking of is entitled "Boost:Test pointer error with g++ 64-bit on Mac OS X 10.6 Snow Leopard", discussed here:

   https://svn.boost.org/trac/boost/ticket/3432

It looks to me like one of two things happened here.  Either the bug was fixed on a branch but the branch was never merged into the release, or a merge conflict overwrote the change before it was ever released.

In 1.38.0 and 1.39.0 the method in question looked like this:

void            clear()
{
   while( !m_test_units.empty() ) {
      test_unit_store::value_type const& tu = *m_test_units.begin();

       // the delete will erase this element from map
       if( test_id_2_unit_type( tu.second->p_id ) == tut_suite )
           delete  (test_suite const*)tu.second;
       else
           delete  (test_case const*)tu.second;
   }
}

The ticket claims to have fixed the bug like so:

void            clear()
{
   while( !m_test_units.empty() ) {
      test_unit_store::value_type const& tu = *m_test_units.begin();

       // the delete will erase this element from map
       if( test_id_2_unit_type( tu.second->p_id ) == tut_suite ) {
           test_suite const* p = (test_suite const*)tu.second;
           delete p;
       } else {
           test_case const* p = (test_case const*)tu.second;
           delete p;
       }
   }
}

But in 1.40.0 and 1.41.0 the method looks like this:

void            clear()
{
   while( !m_test_units.empty() ) {
       test_unit_store::value_type const& tu = *m_test_units.begin();

       // the delete will erase this element from map
       if( ut_detail::test_id_2_unit_type( tu.second->p_id ) == tut_suite )
           delete  static_cast<test_suite const*>(tu.second);
       else
           delete  static_cast<test_case const*>(tu.second);
   }
}

Perhaps whoever switched the C-style cast to static_cast removed the local pointer variable and reintroduced the bug?  In any case, the bug needs to be re-fixed, in the next release, and I would suggest adding a comment so that a future maintainer doesn't eliminate the apparently pointless local pointer variable, like so:

void            clear()
{
   while( !m_test_units.empty() ) {
       test_unit_store::value_type const& tu = *m_test_units.begin();

       // the delete will erase this element from map
       // Important: The pointer MUST be copied to local
       // storage in order to avoid the issue described in
       // https://svn.boost.org/trac/boost/ticket/3432
       if( ut_detail::test_id_2_unit_type( tu.second->p_id ) == tut_suite ) {
           test_suite const* p = static_cast<test_suite const*>(tu.second);
           delete p;
       } else {
           test_case const* p = static_cast<test_case const*>(tu.second);
           delete p;
       }
   }
}