[boost] [ptr_container] ptr_map insert with non-const vs. const reference

Subject: [boost] [ptr_container] ptr_map insert with non-const vs. const reference
From: Martin B. (0xCDCDCDCD_at_[hidden])
Date: 2009-12-14 05:14:01

Hi there,

After thinking a while about the issue with the two insert functions of
ptr_map and why the one with the raw pointer takes a non-const reference
  ...
See:
http://www.boost.org/doc/libs/1_41_0/libs/ptr_container/doc/faq.html#why-does-ptr-map-t-insert-replace-take-two-arguments-the-key-and-the-pointer-instead-of-one-std-pair-and-why-is-the-key-passed-by-non-const-reference
or http://lists.boost.org/boost-users/2007/09/31014.php

I would like to add the following thoughts:
ptr_map provides two functions
(a) std::pair<iterator,bool> insert( key_type& k, value_type x );
(b) template< class U > std::pair<iterator,bool> insert( const key_type&
k, std::auto_ptr<U> x );

Now, I understand the rationale behind having (a) take a non-const
reference since this means less potential ctor calls means less
potential for a leak. I think the rationale makes sense but it's still
easily possible for users to shoot themselves in the foot. Maybe the
documentation of these two functions should be expanded upon as I've
just spent about 1 1/2 hours to make sense of it all :-)

Consider these examples:

using namespace std;
using namespace
typedef boost::ptr_map<string, Foo> foomap_t;

foomap_t the_foo_map;

// 1 (a) - correct usage
string key("key-string"); // need key object to take reference
Foo* p = new Foo();
the_foo_map.insert(key, p); // compiles OK

// 2 (a) - incorrect usage
Foo* p = new Foo();
string key("key-string"); // need key object to take reference - leaks p
if throws
the_foo_map.insert(key, p); // compiles OK

// 3 (b) - correct usage
std::auto_ptr<Foo> p(new Foo());
the_foo_map.insert("key-string", p);

// 4 (b) - incorrect usage
Foo* p = new Foo();
the_foo_map.insert("key-string", std::auto_ptr<Foo>(p)); // compiles OK,
but may leak if the implicit Key constructor throws (which only happens
for string for bad_alloc, but this is just an example)

So my point is, that the non-const reference insert() fails to protect
the user from shooting himself in the foot, but it succeeds at
highlighting the problem, so the problem should be expanded upon in the
docs, shouldn't it?
If we have a raw-pointer then no matter of fiddling on ptr_maps part can
make it completely exception safe, the user code has to provide that.
The raw pointer should already be wrapped in a smart-pointer.
So (a) is unsuited for the case where the raw-pointer exists prior to
the key object. There is only one case where having the (a) version
around makes sense and that is when we have another smart-ptr type:
// 5 (a) correct usage
string key("key-string");
the_foo_map.insert(key, p.Detach());
// makes a lot more sense than
std::auto_ptr<Foo> ap(p.Detach());
the_foo_map.insert("key-string", ap);

cheers,
Martin