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

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

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-doe...
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
participants (2)
-
Martin B.
-
Thorsten Ottosen