Code duplication for functions that take shared_ptr and unique_ptr

Question

Problem:

Let's assume I have an algorithm that takes a unique_ptr to some type:

void FancyAlgo(unique_ptr<SomeType>& ptr);

Now I have shared_ptr sPtr to SomeType, and I need to apply the same algorithm on sPtr. Does this mean I have to duplicate the algorithm just for the shared_ptr?

void FancyAlgo(shared_ptr<SomeType>& sPtr);

I know smart pointers come with ownership of the underlying managed object on the heap. Here in my FancyAlgo, ownership is usually not an issue. I thought about stripping off the smart pointer layer and do something like:

void FancyAlgo(SomeType& value);

and when I need to call it with unique_ptr:

FancyAlgo(*ptr);

likewise for shared_ptr.

1, Is this an acceptable style in PRODUCTION code?(I saw somewhere that in a context of smart pointers, you should NOT manipulate raw pointers in a similar way. It has the danger of introducing mysterious bugs.)

2, Can you suggest any better way (without code duplication) if 1 is not a good idea.

Thanks.

Smart pointers should almost never be passed by reference, unless you want to talk about the smart pointer itself, not the pointee. Use .get() or *. — Kerrek SB
Why not? In the old C++03 it was perfectly fine to pass pointers around. Now it's just become pointers with ownership. I don't see why it has an impact on your decision. — h9uest
Because usually the pointer isn't the object of interest, but the pointee is. — Kerrek SB
If you are simply mutating the object passed in, you don't need to bother with smart-pointers - simply work with a reference. If the argument could potentially be empty or invalid, consider using something like boost::optional to hold this information, as opposed the old practice of using pointer nullness as the indicator. — Pradhan
Yeah. Passing in the pointee solves the problem. However, in the sole context of the function(assume a different developer implements this function), there's nothing at all to hint the implementation developer that he should not manage the pointee with another smart pointer(e.g. shared_ptr<SomeType> another_ptr (&value);). If he does this then we'll have double deletion problem later. If the function signature has a smart pointer, then it's a clear sign that he should not try to manage the object with another smart pointer. — h9uest

Yakk - Adam Nevraumont Yakk - Adam Nevraumont · Accepted Answer · 2015-03-23T19:05:36

Smart pointers are about ownership. Asking for a smart pointer is asking for ownership information or control.

Asking for a non-const lvalue reference to a smart pointer is asking for permission to change the ownership status of that value.

Asking for a const lvalue reference to a smart pointer is asking for permission to query the ownership status of that value.

Asking for an rvalue reference to a smart pointer is being a "sink", and promising to take that ownership away from the caller.

Asking for a const rvalue reference is a bad idea.

If you are accessing the pointed to value, and you want it to be non-nullable, a reference to the underlying type is good.

If you want it to be nullable, a boost::optional<T&> or a T* are acceptable, as is the std::experimental "world's dumbest smart pointer" (or an equivalent hand-written one). All of these are non-owning nullable references to some variable.

In an interface, don't ask for things you don't need and won't need in the future. That makes reasoning about what the function does harder, and leads to problems like you have in the OP. A function that reseats a reference is a very different function from one that reads a value.

Now, a more interesting question based off yours is one where you want the function to reseat the smart pointer, but you want to be able to do it to both shared and unique pointer inputs. This is sort of a strange case, but I could imagine writing a type-erase-down-to-emplace type (a emplace_sink<T>).

template<class T>
using later_ctor = std::function<T*(void*)>;

template<class T, class...Args>
later_ctor<T> delayed_emplace(Args&&...args) {
  // relies on C++1z lambda reference reference binding, write manually
  // if that doesn't get in, or don't want to rely on it:
  return [&](void* ptr)->T* {
    return new T(ptr)(std::forward<Args>(args));
  };
}

namespace details {
  template<class T>
  struct emplace_target {
    virtual ~emplace_target() {}
    virtual T* emplace( later_ctor<T> ctor ) = 0;
  };
}
template<class T>
struct emplacer {
  std::unique_ptr<emplace_target<T>> pImpl;
  template<class...Args>
  T* emplace( Args&&... args ) {
    return pImpl->emplace( delayed_emplace<T>(std::forward<Args>(args)...) );
  }
  template<class D>
  emplacer( std::shared_ptr<T, D>& target ):
    pImpl( new details::emplace_shared_ptr<T,D>(&target) ) // TODO
  {}
  template<class D>
  emplacer( std::unique_ptr<T, D>& target ):
    pImpl( new details::emplace_unique_ptr<T,D>(&target) ) // TODO
  {}
};

etc. Lots of polish needed. The idea is to type-erase construction of an object T into an arbitrary context. We might need to special case shared_ptr so we can call make_shared, as a void*->T* delayed ctor is not good enough to pull that off (not fundamentally, but because of lack of API hooks).

Aha! I can do a make shared shared ptr without special casing it much.

We allocate a block of memory (char[sizeof(T)]) with a destructor that converts the buffer to T then calls delete, in-place construct in that buffer (getting the T*), then convert to a shared_ptr<T> via the shared_ptr<T>( shared_ptr<char[sizeof(T)]>, T* ) constructor. With careful exception catching this should be safe, and we can emplace using our emplacement function into a make_shared combined buffer.

Code duplication for functions that take shared_ptr and unique_ptr

1 Answers