BCB5, the pimpl idiom bug and the new Unit Test Framework

From: Fernando Cacciola (fcacciola_at_[hidden])
Date: 2001-12-20 12:37:37

Hi!

Incidentally, I was set today to the task of writing a new unit test for
some of my code.
I figured that it was a perfect opportunity to give the latest Unit Test
Framework a try.

I created a BCB5 project so as to build up unit_test.dll (BTW, it really
needs a jamfile!)

As soon as I tried to compile it, I stumbled across the BCB bug regarding
the pimpl idiom:

   < Borland C++ 5.5.1 parses the destructor of the scoped_prt<> even though
it is not parsing the
      destructor of the outer class. Fortunately, THANKS to the
checked_delete<>, it yields an error instead
      of silently fail to delete the implementation>

I already reported in this list that the buggy behavior is related to the
'destructor cleanup' option (being ON) which enables automatic objects to be
destroyed in the occurrence of exceptions.

Because I really want to use the test library seriously, so I can't really
turn off 'destructor clenaup', I searched with Google on
"borland.public.cppbuilder.language" to see if there is practical
workaround.

On a thread, I found a response from Nicola Musatti stating that the Alan
Griffiths's "grin_ptr" does work with BCB, and can be used as a replacement
for 'boost::scoped_ptr' (scoped_ptr was explicitly mentioned)

So I turned to Alan's web site and read about his grin_ptr<>.

As I understood it, Alan's grin_ptr<> is intended to solve the same
'deleting incomplete type' problem that boost solves using checked_delete<>;
but, because of the peculiar indirection it uses, it does work with BCB!
(actually, half-works, as I'll explain)

The trick behind grin_ptr<> is to use a callback to actually "delete ptr" in
the grin_ptr<> destructor.
This callback is initialized when the grin_ptr<> is constructed, which
happens to be inside the outer class constructor.

Consider this:

struct A
{
  // A() omitted.
  ~A() ;
  friend class B ;
  boost::scoped_ptr<B> pimpl ;
} a ;

On a good compiler, since ~A() isn't parsed, nor is ~scoped_ptr<> so there
shouldn't be any problem with
the fact that B is incomplete.
BCB, however, when 'destructor cleanup' is turned on, DOES parses
~scoped_ptr<B> (even though it doesn't need it), but then the
checked_delete<> complains about B being an incomplete type.
The result is that the code above simply doesn't compile with Borland.

If I explicitly add the DECLARATION of the default constructor inside A
(which has nothing to do with destroying pimpl):

struct A
{
  A();
  ~A() ;
  friend class B ;
  boost::scoped_ptr<B> pimpl ;
} a ;

it still doesn't compile.

I changed scoped_ptr to use a callback do_delete(), initialized upon
construction, exactly in the same way that grin_ptr<> does.
With this change, the pimpl idiom WITH the constructor declaration (with
defined elsewhere) does compile with BCB.

With the new scoped_ptr<> I was able to compile entirely the unit test
framework!

So...

Should we change scoped_ptr<> (and scoped_array_ptr<>) as I outlined?
Would be any problems in doing so?
In that case, should we ask Alan express permission? Is it enough to add a
credit line?
Or perhaps, should we ask Alan to submit his grin_ptr<> and add it to
smart_ptr.hpp?

Well,
here's the updated scoped_ptr implementation:

template<typename T> class scoped_ptr : noncopyable {

  T* ptr;
  typedef void (*do_delete_func) ( T* p ) ;
  do_delete_func do_delete ;
  static void do_delete_impl( T* p ) { checked_delete(p) ; }

 public:
  typedef T element_type;

  explicit scoped_ptr( T* p=0 ) : ptr(p) , do_delete(do_delete_impl) {}  //
never throws
  ~scoped_ptr()                 { do_delete(ptr); }
  void reset( T* p=0 )          { if ( ptr != p ) { do_delete(ptr); ptr =
p; } }
  T& operator*() const          { return *ptr; }  // never throws
  T* operator->() const         { return ptr; }  // never throws
  T* get() const                { return ptr; }  // never throws
#ifdef BOOST_SMART_PTR_CONVERSION
  // get() is safer! Define BOOST_SMART_PTR_CONVERSION at your own risk!
  operator T*() const           { return ptr; }  // never throws
#endif
  };  // scoped_ptr

For details about grin_ptr<> visit:

http://www.octopull.demon.co.uk/arglib/TheGrin.html

One last note: grin_ptr<> actually adds also a 'deep' copy semantic in which
contained objects are 'duplicated' or cloned when grin_ptr<> is copied.

Fernando Cacciola
Sierra s.r.l.
fcacciola_at_[hidden]
www.gosierra.com