On 12 May 2008, at 19:26, Peter Dimov wrote:

Can you please file a Trac ticket for this?

Ticket #1913 Thanks, Kevin Martin

Kevin Martin wrote:

On 16 May 2008, at 16:49, Noah Roberts wrote:

...

You can do a lot more than that! Consider the case of a handle of some sort that you need to return to a service when you are done. Case in point, in order to get rid of an sqlite database connection pointer you call db_close(), NOT delete.

No big deal, just use db_close as your deleter!

Is encapsulating the sqlite pointer in an object and having the destructor do that not a better alternative?

Why would it be? What's gained?

Hi! Kevin Martin schrieb:

A layer of abstraction between the library calls and your user code.

Right. A layer of abstraction. That's just what I have: my abstraction does not expose any database header. Everything is hidden in a pimpl. But how shall I implement the pimpl? It needs to "own" a database connection. I could write a destructor that closes the connection. But I don't bother. I try to avoid writing any destructor. I stick to existant RAII classes. So I use a shared_ptr with a custom deleter that closes the connection. Feels safe. That's what I gain. :) Regards, Frank

Sohail Somani wrote:

I was recently torn between doing this or thinking whether I'm doing it wrong. I'm still not sure whether using shared pointers everywhere with null deleters for stack allocated objects is The Right Thing. It kind of defeats the purpose of shared_ptr which is to ensure that the lifetime of the pointer is guaranteed for you. With null deleters, maintenance programmers may do the wrong thing (stuff something in an event queue deep down, for example.)

If anyone has any opinion (for or against) please shout!

I certainly would not use them everywhere. That's just a waste as was expressed by another. The only time I think one would be justified in using a shared_ptr with a null deleter for a stack object is when passing one into a function that uses it. However, even this is dangerous since the fact that this function is using a shared_ptr indicates that it could make a copy of it! So you better be pretty darn sure of the lifetime of that shared_ptr so that it doesn't outlive your stack and get called somewhere down the line...pointing at stack area memory! You could return one to some internal data but this also seems like highly questionable design to me. All copies of that shared_ptr that you just handed out, pointing to your insides, better not outlive you...and you have 0 control over that. Putting an object into a shared_ptr is pretty much saying, "I don't care when this object is deleted but it better stick around long enough to be used by everyone that wants to do so." Stack objects don't fit this sentence at all.

Kevin Martin wrote:

...

I was recently torn between doing this or thinking whether I'm doing it wrong. I'm still not sure whether using shared pointers everywhere with null deleters for stack allocated objects is The Right Thing.

I think it depends how much you are relying on the smart pointer.

If you are certain about the scope of the pointer - i.e. you know where the delete is going to happen and you are using the smart_ptr purely to guarantee delete is called properly then using a null deleter and stack based objects is fine. However, if you are using the smart_ptr to let objects run loose and clear up after themselves, then this absolutely shouldn't be used.

My problem is module interfaces. I have a module (set of classes) that solve a problem, and they require data. They don't care whether that data is on the heap, the stack, or the moon. They just want a pointer to it. In this case it is arguable that the pointers passed shouldn't be smart pointers at all, just normal pointers. That is fine, but has two drawbacks:

a) You have to call get() for all shared_ptr<>s everywhere instead of just passing the pointer b) You can't do this:

void func1(int *x); boost::shared_ptr<int> func2();

func1(func2());

And to make matters worse, people might be tempted to do this!

func1(func2().get());

By having func1 take a boost::shared_ptr<int> as argument, we can do this, but we can no longer pass stack objects (unless we can put a stack object in a shared_ptr)

As was mentioned by another, func1 seems to be poorly designed in that it is taking the wrong type of argument. 'x' should most certainly be a reference, not a pointer. This does not guarantee that nobody will do something as silly as taking the address of that object and keeping a pointer around, but it does advertise that it definitely should not be. If it does you can execute the developer that did it. Having fixed that, your call looks like this: func1(*func2()); Or the more sure: boost::shared_ptr<int> p = func2(); func1(*p); Now you're not assuming that p is not a temporary!

2008/5/16 Kevin Martin :

func1() may be a member function of an object which needs to keep the argument somewhere, that is why I prefer to pass a pointer.

If it is keeping the argument, then it cares about ownership/lifetime, so one should pass a smart pointer to it.

Passing a reference gives the implication (at least to me) that the object is not going to keep any link with the argument after the call returns. Passing a pointer makes me think - hey, what is this object doing with this thing?

It doesn't convey that information to other readers of your code. All the language rules really give you is that passing by reference means that the variable must exist, while passing by pointer means that it could be NULL. -- Nevin ":-)" Liber mailto:nevin@eviloverlord.com (847) 691-1404

Hi! Noah Roberts schrieb:

Actually, I think you are correct on that. A temporary will only survive into the function call if you are accepting a const& afaik. If you are accepting a const reference then whatever is passed in will be bound to that object and is guaranteed to survive. Otherwise no.

All of my knowledge tells me: you are *not* in trouble here. But at least 10 years ago it was unclear, as this article from comp.std.c++ shows: http://groups.google.com/group/comp.std.c++/msg/3fbd47d85ab1e47d Regards, Frank

2008/5/16 Kevin Martin :

My problem is module interfaces. I have a module (set of classes) that solve a problem, and they require data. They don't care whether that data is on the heap, the stack, or the moon. They just want a pointer to it. In this case it is arguable that the pointers passed shouldn't be smart pointers at all, just normal pointers.

My general rule of thumb is that if mucking with ownership or lifetime, pass a smart pointer. If not, pass a raw pointer.

That is fine, but has two drawbacks:

a) You have to call get() for all shared_ptr<>s everywhere instead of just passing the pointer b) You can't do this:

void func1(int *x); boost::shared_ptr<int> func2();

func1(func2());

One could always add the following overload: inline void func1(shared_ptr<int> const& x) { func1(x.get()); }

And to make matters worse, people might be tempted to do this!

func1(func2().get());

What is wrong with that?

Just because the shared_ptr guarantees the lifetime of the pointer, doesn't mean you can forget about who has a copy of the pointer and why.

They are separate issues. There are times I need to know who has references to my object, other times it is sufficient to know how many outstanding references there are, and still other times where all I need to know is whether or not there are any outstanding references to my object.

If you are passing any kind of pointer/reference anywhere, then the callee should have a strict, well documented description of what it plans to do with that pointer, otherwise anyone may be modifying your object without you realizing.

Modifying your object is an orthogonal issue, better handled with judicious use of const.

I think using smart pointer without knowing both when and why it is being copied is inviting disaster, therefore I don't see the problem, but maybe someone can show me an example where this information can't/ shouldn't be known.

If you get a job at my company, I can show you a couple of examples. :-) Take your func2. It has no idea who will call it, so it can't possibly know why (or even if) its return value is being copied. Yet that is normal, not disasterous for things like factory functions. Another example: suppose I have a pool of expensive to construct cookies. My factory function returns them to some caller, and when the reference count goes to 0, I put the cookie back into the free pool. I don't know or care how many times the caller copies it; I just need to know when there are no outstanding references to it so that I can reuse it. Regards, -- Nevin ":-)" Liber mailto:nevin@eviloverlord.com (847) 691-1404

Marshall Clow wrote:

At 2:37 PM -0500 5/16/08, Nevin \":-]\" Liber wrote:

...

...

My problem is module interfaces. I have a module (set of classes) that solve a problem, and they require data. They don't care whether that data is on the heap, the stack, or the moon. They just want a pointer to it. In this case it is arguable that the pointers passed shouldn't be smart pointers at all, just normal pointers. My general rule of thumb is that if mucking with ownership or

2008/5/16 Kevin Martin : lifetime, pass a smart pointer. If not, pass a raw pointer.

A brief addenda (from my rules of thumb) for non-ownership cases: * If the parameter is optional, pass a raw pointer, and the callee should expect a NULL pointer. * If the parameter is not optional, pass a reference.

I don't like this idea. You are creating a dependency on the fact that the called function will NOT keep a copy - or you are insisting that the object in question implement the shared_from_this model. If you later decide that it would be prudent for the function or object in question to create or pass a kept copy of this object then you'll have to change the signature of the function and then each and every call to it (to remove get() calls). So while I can see some possible enforcement advantages to this in the rare case, I certainly wouldn't use it as a general rule of thumb. I can't count the number of times someone has assumed that nobody would need a copy of something and then requirements changing dictated that someone did.

2008/5/16 Noah Roberts :

I don't like this idea. You are creating a dependency on the fact that the called function will NOT keep a copy - or you are insisting that the object in question implement the shared_from_this model. If you later decide that it would be prudent for the function or object in question to create or pass a kept copy of this object then you'll have to change the signature of the function and then each and every call to it (to remove get() calls).

If this were Java, I might agree. Given your assumption, I don't see how you can pass pointers to stack variables, pointers to member variables, pointers to objects in STL containers, etc., to any function, because it is not possible for those functions to know, let alone change, the lifetime of those objects. Do you really allocate every single variable in your program on the heap and store it in a shared_ptr, just in case? Regards, -- Nevin ":-)" Liber mailto:nevin@eviloverlord.com (847) 691-1404

2008/5/16 Noah Roberts :

Nevin ":-]" Liber wrote:

...

2008/5/16 Noah Roberts mailto:roberts.noah@gmail.com>:

I don't like this idea. You are creating a dependency on the fact that the called function will NOT keep a copy

Suppose I had the function: void foo(int const* p) { if (p) std::cout << *p << std::endl; } It is perfectly legal to call it as: void bar() { int i(2); foo(&i); } int main() { bar(); bar(); } Now, if we let foo squirrel away a copy of p, as in: void foo(int const* p) { static int const* pp; if (p && pp) std::cout << *pp << ' ' << *p << std::endl; pp = p; } The program is now broken, since it is illegal to dereference pp during the second call to foo(), yet foo has no way of knowing that. What should the interface to foo() be such that it doesn't break whether or not foo() keeps a copy of p? -- Nevin ":-)" Liber mailto:nevin@eviloverlord.com (847) 691-1404

2008/5/16 Noah Roberts :

Nevin ":-]" Liber wrote:

...

void foo(int const* p) { static int const* pp; if (p && pp) std::cout << *pp << ' ' << *p << std::endl; pp = p; }

What should the interface to foo() be such that it doesn't break whether or not foo() keeps a copy of p?

You might consider a shared_ptr. Seems like the perfect place for one.

Let's recap: NL:If it is keeping the argument, then it cares about ownership/lifetime, so one should pass a smart pointer to it. MC:A brief addenda (from my rules of thumb) for non-ownership cases: * If the parameter is optional, pass a raw pointer, and the callee should expect a NULL pointer. * If the parameter is not optional, pass a reference NR:You are creating a dependency on the fact that the called function will NOT keep a copy NL:Do you really allocate every single variable in your program on the heap and store it in a shared_ptr, just in case? NR:I can't think of what your reasoning is that leads you to this conclusion from my statements. Care to explain? NL:What should the interface to foo() be such that it doesn't break whether or not foo() keeps a copy of p? NR:You might consider a shared_ptr. Seems like the perfect place for one. Please go back to the top and reread my rule of thumb (with Marshall's addendum, which I agree with and use). There is no safe, general way to have a function or class that takes a raw pointer or reference such that it can keep a copy of that pointer or reference and use it later. Someone outside of the function or class has to make external guarantees about its lifetime. If you have a function that doesn't keep a copy of a pointer or reference and then change its implementation so that it does, you are going to have to revisit every use of that function or class. In other words, code written like that is fragile and should be used sparingly. One of the few places I would use code like that is in the constructor of a smart pointer, where there are very well documented rules on how to call it. Almost everywhere else where lifetime/ownership matters past this one function call, I use a smart pointer instead of a raw one. I prefer not to have fragile code, where the caller has to "be careful" (which is merely a euphemism for "be perfect", as any mistake creates a subtle bug). If you a way of passing a raw pointer or reference where it is always safe for the called function to copy and use that pointer or reference later without any external guarantees about the lifetime of the object pointed/referred to, I'd certainly be interested in hearing about it. Regards, -- Nevin ":-)" Liber mailto:nevin@eviloverlord.com (847) 691-1404

Nevin ":-]" Liber wrote:

2008/5/16 Noah Roberts :

...

Nevin ":-]" Liber wrote:

...

2008/5/16 Noah Roberts mailto:roberts.noah@gmail.com>:

I don't like this idea. You are creating a dependency on the fact that the called function will NOT keep a copy

Suppose I had the function:

More to the point.

void foo(int const* p) { if (p) std::cout << *p << std::endl; }

I don't think foo in this case stands as an exemplary of good design choice. It should look more like so: void foo() { } void foo(int i) { std::cout << i << std::endl; } Overrides like this are one of the great features of C++. You might argue that foo looks more like: void foo(int * p = 0) // I see this a lot. { // ...bunch of stuff... *p = result of calculations // ... some cleanup maybe } Again, split it into two: int bunch_of_stuff() { ... } void cleanup() { ... } void foo() { bunch_of_stuff(); cleanup(); } void foo(int & p) { p = bunch_of_stuff(); cleanup(); } You'll notice that any time you could have the =0 foo you can use these two without any difference in calling semantics. In both you either call with an argument or don't. Only one case, and one you should never, ever do, is if you're passing around null pointers without caring. The point is that you've got a function that behaves differently based on a state variable being passed in. That may be a necessary evil but does not make for a good argument about best practice. I've happily kept my use of raw pointers to a bare minimum for quite a while now. Everyone on my team is more productive because of it. The difference here between using a pointer and splitting into two functions that use shared code but behave differently is really much more than just the difference between accepting pointer and reference as far as maintaining goes, but there's a lot there so I'm going to pass on that. What is important per this discussion is that since you are binding to the idea that you will not copy this data and hold it, make that as obvious as possible. Pointers are notoriously vague in their meaning and one always asks, "Is this function going to copy and hold this pointer?" Bind to as little as possible. Make clear any ambiguity. You've come up with a plan that works for you, but then somebody like me might come along and screw it all up. Who's fault is it, the one that didn't intuitively grasp the meaning of accepting raw pointers or the one that didn't clarify as much as possible by not doing so to begin with? If you really do need to do the whole if(p) thing, which is really smelly, then you might consider using a reference wrapper that is capable of being null. Make intentions clear, "this will not be copied," and leave the possibility of having an = 0 default. Pointers are necessary in some cases--you're not going to implement a vector without them--but really, in my opinion, should be avoided like the plague. You might be smart enough to use them without screwing up, I have to protect myself from myself. Now, again, there are cases when best practices go out the window in favor of other, more important criteria. If performance is really, really an issue then wrapping up every pointer you see is not exactly going to help you meet your criteria. So you sacrifice clarity for performance sometimes. But when you need to do this you know it...you've tested with a profiler and made your algorithm as efficient as possible and know that this shared_ptr is really, really causing you problems.

David Walthall wrote:

Overall, it seems like using a shared_ptr for stack objects is misleading, and as a result, may eventually lead to difficult to trace bugs. Unless allocating and deallocating the object is a bottleneck in your code, the potential costs outweigh the benefits.

Yes, this was my general conclusion (i.e., I didn't actually do it!) For freshly written code, I could not think of a case where I would suggest a null deleter but that doesn't mean that there isn't one. I think before adding a null deleter to Boost, it might be useful to think about the cases in which it is necessary and whether they occur enough to encourage its use. After all, if it is in Boost, chances are that it is a proper way to do things. -- Sohail Somani http://uint32t.blogspot.com