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

Subject: Re: [boost] [ptr_container] ptr_map insert with non-const vs. const reference
From: Thorsten Ottosen (thorsten.ottosen_at_[hidden])
Date: 2009-12-16 20:03:09

Hi Martin,

> 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.

Yes, it's not fool-proof.

> 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?

It's a good description, and with your permission I will include it
in the docs...

> 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);

One reason is enough, isn't it? Anyway, it's also slightly more 
convienent just to say new T.

I wish that the order was or evaluation was specified, that would have 
been the best solution IMO.

-Thorsten