
Hi- Shouldn't the constructor in intrusive_ptr be explicit? explicit intrusive_ptr(T * p, bool add_ref = true): p_(p) { if(p_ != 0 && add_ref) intrusive_ptr_add_ref(p_); } I don't have cvs access. -- Colin

Colin Rafferty wrote:
Hi-
Shouldn't the constructor in intrusive_ptr be explicit?
explicit intrusive_ptr(T * p, bool add_ref = true): p_(p) { if(p_ != 0 && add_ref) intrusive_ptr_add_ref(p_); }
No, the constructor is implicit by design. Do you have a case where the implicit constructor causes problems?

Peter Dimov wrote:
Colin Rafferty wrote:
Shouldn't the constructor in intrusive_ptr be explicit?
No, the constructor is implicit by design. Do you have a case where the implicit constructor causes problems?
Sure, this is the same problem as for shared_ptr<>. extern void foo(const intrusive_ptr<Bar>&); void baz() { Bar* bar = new Bar; foo(bar); // oops! delete bar; } I thought I was passing a raw pointer into foo(), but I'm actually creating a temporary intrusive_ptr<Bar>, passing that in, and then destroying the temporary and the pointer. -- Colin

colin rafferty wrote:
Peter Dimov wrote:
Colin Rafferty wrote:
Shouldn't the constructor in intrusive_ptr be explicit?
No, the constructor is implicit by design. Do you have a case where the implicit constructor causes problems?
Sure, this is the same problem as for shared_ptr<>.
extern void foo(const intrusive_ptr<Bar>&);
void baz() { Bar* bar = new Bar; foo(bar); // oops! delete bar; }
I thought I was passing a raw pointer into foo(), but I'm actually creating a temporary intrusive_ptr<Bar>, passing that in, and then destroying the temporary and the pointer.
More generally, the problem is that intrusive_ptr<> destroys the object it is passed (on destruction), and having a conversion constructor makes it very easy for a programmer to make a mistake (like above). Can someone please fix this to make the conversion constructor explicit? Thanks. -- Colin

Hi Colin colin.rafferty@morganstanley.com wrote:
No, the constructor is implicit by design. Do you have a case where the implicit constructor causes problems?
Sure, this is the same problem as for shared_ptr<>.
extern void foo(const intrusive_ptr<Bar>&);
void baz() { Bar* bar = new Bar; foo(bar); // oops! delete bar; }
For this to work, intrusive_ptr_add_ref and intrusive_ptr_release functions accepting a Bar * (or a base class pointer) must exist. That is, someone already decided that *all* heap-allocated Bar objects will be intrusively reference-counted. Creating a heap-allocated Bar object without immediately passing the pointer to the intrusive_ptr constructor is dangerous. Is there a reason why you do that? I can't speak for Peter but I believe he made intrusive_ptr constructor non-explicit because there is no room for abuse as long as you follow the best practices he describes. The same isn't true for heap-allocated objects pointed to by a shared_ptr: extern void foo( const shared_ptr< Bar > & ); void baz() { shared_ptr< Bar > pBar = new Bar(); foo( pBar.get() ); // compiler error here because of explicit ctor } With a non-explicit ctor, the last line would compile just fine but eventually lead to a double-delete. With intrusive_ptr this isn't a problem at all, hence the non-explicit constructor there. Regards, -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.

"Andreas Huber" <ahd6974-spamgroupstrap@yahoo.com> wrote:
colin.rafferty@morganstanley.com wrote:
No, the constructor is implicit by design. Do you have a case where the implicit constructor causes problems?
Sure, this is the same problem as for shared_ptr<>.
[elided example double-delete]
For this to work, intrusive_ptr_add_ref and intrusive_ptr_release functions accepting a Bar * (or a base class pointer) must exist. That is, someone already decided that *all* heap-allocated Bar objects will be intrusively reference-counted. Creating a heap-allocated Bar object without immediately passing the pointer to the intrusive_ptr constructor is dangerous. Is there a reason why you do that?
I agree. However, sometimes I don't have control over my base classes (or at least, they can surprise me). Imagine the case where a 3rd party library doesn't have a very good interface: class Base; extern void foo(Base*); I create a derived class and use it cleanly. class Derived : public Base { /* ... */ }; void baz() { scoped_ptr<Derived> dptr(new Derived); foo(dptr.get()); } In the next version of the library, someone has talked to the developers, and they've made it safer: extern void intrusive_ptr_add_ref(Base* p); extern void intrusive_ptr_release(Base* p); extern void foo(const intrusive_ptr<Base>&); My existing code in `baz' still compiles, but now I have the double-delete problem.
I can't speak for Peter but I believe he made intrusive_ptr constructor non-explicit because there is no room for abuse as long as you follow the best practices he describes. The same isn't true for heap-allocated objects pointed to by a shared_ptr.
I agree that an intrusive_ptr is less likely to get bitten by the implicit constructor than a shared_ptr. However, I can see three reasons why it should still be explicit: 1. It is still potentially dangerous. 2. It is consistant with every other pointer class in boost and std. 3. There is no time when someone would actually want the implicit conversion, so there's no disadvantage. This hasn't bitten me (yet), but I imagine that's only because intrusive_ptr is such a new addition. -- Colin

colin.rafferty@morganstanley.com wrote:
"Andreas Huber" <ahd6974-spamgroupstrap@yahoo.com> wrote:
colin.rafferty@morganstanley.com wrote: For this to work, intrusive_ptr_add_ref and intrusive_ptr_release functions accepting a Bar * (or a base class pointer) must exist. That is, someone already decided that *all* heap-allocated Bar objects will be intrusively reference-counted. Creating a heap-allocated Bar object without immediately passing the pointer to the intrusive_ptr constructor is dangerous. Is there a reason why you do that?
I agree. However, sometimes I don't have control over my base classes (or at least, they can surprise me). Imagine the case where a 3rd party library doesn't have a very good interface:
class Base; extern void foo(Base*);
I create a derived class and use it cleanly.
class Derived : public Base { /* ... */ };
void baz() { scoped_ptr<Derived> dptr(new Derived); foo(dptr.get()); }
In the next version of the library, someone has talked to the developers, and they've made it safer:
extern void intrusive_ptr_add_ref(Base* p); extern void intrusive_ptr_release(Base* p);
extern void foo(const intrusive_ptr<Base>&);
My existing code in `baz' still compiles, but now I have the double-delete problem.
This is a good example why any form of automatic cleanup (in this case reference counting) should be introduced right at the beginning. Doing it afterwards breaks a few very crucial assumptions. In this particular example it is just wrong to write scoped_ptr< Dervied > dptr( new Derived ). scoped_ptr doesn't know about the RCness of Derived objects and will inevitably do the wrong thing. You're just lucky if it works anyway. For example, imagine what happens if the Base member function foo() calls another function bar() to which it passes an intrusive_ptr pointing to the Base object (i.e. the Base object passes a pointer to itself). Calling foo() on a Base object that is not initially owned by an intrusive_ptr will likely lead to a disaster.
I can't speak for Peter but I believe he made intrusive_ptr constructor non-explicit because there is no room for abuse as long as you follow the best practices he describes. The same isn't true for heap-allocated objects pointed to by a shared_ptr.
I agree that an intrusive_ptr is less likely to get bitten by the implicit constructor than a shared_ptr. However, I can see three reasons why it should still be explicit:
1. It is still potentially dangerous.
It isn't as long as you follow the best practices described in the smart_ptr docs. The example above shows that even if the ctor was explicit you'd need to follow these guidelines anyway. So there's no point in making it explicit.
2. It is consistant with every other pointer class in boost and std.
Why should it be consistent? The reason why the other pointers have an explicit ctor does not apply to intrusive_ptr.
3. There is no time when someone would actually want the implicit conversion
Yes there is, I'm using intrusive_ptr and some of my code relies on the implicit conversion (this is useful when you need to pass an intrusive_ptr pointing to this to a function). Regards, -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.

"Andreas Huber" <ahd6974-spamgroupstrap@yahoo.com> wrote:
colin.rafferty@morganstanley.com wrote:
However, I can see three reasons why it should still be explicit:
3. There is no time when someone would actually want the implicit conversion
Yes there is, I'm using intrusive_ptr and some of my code relies on the implicit conversion (this is useful when you need to pass an intrusive_ptr pointing to this to a function).
Okay. I had not thought of that use. Given this, and the fact that it is hard to accidentally get in trouble, I unrequest the change. -- Colin
participants (4)
-
Andreas Huber
-
Colin Rafferty
-
colin.rafferty@morganstanley.com
-
Peter Dimov