Custom class for a borrowed unique_ptr<T>?

Question

In C++11, I tend to have master objects that maintain sole ownership of a collection of some children objects. Let's assume these children objects are non-copyable (for instance, boost::noncopyable).

Other code needs to access these children objects to do some temporary processing, but not keep references, or share ownership over these objects.

As such, my master objects tend to have some sort of a get_child() method.

As an example, let's say I have a Master object like this:

class Master {
private:
  std::vector<std::unique_ptr<Child>> children;
  ...
public:
  ??? get_child(int index);
};

When writing the get_child() method, I have 2 obvious options:

1. Return by pointer:

   Child* get_child(int_index) {
     return children[index].get();
   }
   

2. Return by reference:

   Child& get_child(int index) {
     return *(children[index]);
   }
   

I thought I didn't like either of these (for reasons I explain below), so I came up with the following:

borrowed_ptr<Child> get_child(int index) {
  return make_borrowed<Child>(children[index].get());
}

where borrowed_ptr is a trivial class wrapping a pointer with no smarts. I provide its implementation at the end of this question.

To me, borrowed_ptr indicates that there exists a unique_ptr someplace, and this borrowed_ptr should be used for a short time, and not be recorded for any circumstance.

I don't like returning by raw pointer (1.) because I feel that Child* doesn't imply that there exists a unique_ptr<Child> someplace. Namely, I already have a lot of raw pointers floating around (think handles received from libraries, or my code which is not using smart pointers). I feel borrowed_ptr gives a little more information over a raw pointer.

I don't like returning by reference (2.) because of an additional reason. I am worried that I will write

Child child = get_child();

and invoke a copy when I didn't intend to.

My question: Is borrowed_ptr silly? Does it raise red flags with experienced C++ developers? Is it a faux pas? Does it feel like a code smell? Do you feel I should rather do 1., 2. or something else altogether?

Here is my implementation of borrowed_ptr:

template<typename T>
class borrowed_ptr {
private:
  T* ptr;
public:
  borrowed_ptr(const unique_ptr<T>& r) {
    ptr = r.get();
  }

  inline T* operator->() {
    return ptr;
  }
  inline const T* operator->() const {
    return ptr;
  }
  inline T& operator*() {
    return *ptr;
  }
  inline const T& operator*() const {
    return *ptr;
  }
};

Answer 1

My question: Is borrowed_ptr silly? Is it a faux pas? Does it feel like a code smell?

It seems a bit overengineered to me (YMMV).

Do you feel I should rather do 1., 2. or something else altogether?

Something else. Consider this alternative:

class Master {
private:
    std::vector<std::unique_ptr<Child>> children;
    ...
public:
    // an observed ptr is not modified in any way by client code
    typedef Child const * const ObservedChildPtr;

    ObservedChildPtr get_child(int index) const;
};

This has the following advantages:

• the semantics for an observed (i.e. not modified) pointer became part of the class' API
• it is explicit and self-explanatory (like your borrowed_ptr)
• it is minimalistic (you don't define/need the extra class)
• it provides a natural expansion point, in case you actually do need the extra class (see line above) - if you need to expand it, replace Child const * const with a class definition.
• client code can be written in terms of Master::ObservedChildPtr (which you can change later without having to update client code).

Answer 2

I've implemented and used this pattern before. There are cases where shared ownership (via shared_ptr) is not correct or desirable, where the ownership policy is as you have described (one thing owns it, but may pass along temporary non-owning references).

In the past I would adhere to the convention that the appropriate smart pointers should be used when possible, and thus that any return of a bare pointer through some API was implicitly considered a scenario where the API was maintaining full control over the lifetime of the objects in question (not sharing it, not transferring it to you).

The problem with this worldview is that it often must be explained; it's not obvious if the function T * getSomething() is returning a so-called borrowed pointer, or if it just does return new T(); // good luck. I've lost track of the number of times I've had knee-jerk "you should use std::shared_ptr there" reactions from people upon seeing any use of a bare pointer whatsoever.

So I do think there is probably some value in codifying the way the pointer will be used in its type like you've implemented (or like what N3840 proposes).

But it's possible we can do better. Using a pass-through wrapper type lets us encode this extra documentation, but it doesn't help us prevent any of the errors you still have with such an ownership paradigm (namely, somebody holding on to a borrowed pointer beyond the time in which the owner has decreed it can be relied on). This results in the classic "dangling pointer" crash.

It would be slightly more useful if we could tell, either when we delete the owning pointer or when we attempt to use the borrowed pointer, that we've made a mistake (of the two, I think the former is better; the latter is not really that much different than the basic crash unless we spend a lot of memory storing information about the call site where the former happened anyway).

You can do this by using shared_ptr internally, and passing around weak_ptr objects as the "borrowed" pointers (at least in theory, I've never used them as such extensively enough to be sure there are no unusual edge cases). But that does carry the cost of the overhead of shared_ptr, which is small but non-zero and generally cannot be easily compiled away.

But you can implement an owned_ptr which functions more-or-less like unique_ptr except that it also provides a way to hand out borrowed_ptr objects and thus can keep track (in a fashion similar to how shared_ptr does) of any outstanding borrowers of the pointer, allowing you to assert and perform diagnostics if you delete the original pointer with outstanding borrowers. You can do this in such a way as to trivially allow all this extra record-keeping to be compiled away, so in the end you just have the equivalent of bare pointers again.

But is all that extra work and complexity worth it? We can't approach a system as robust as what, for example, Rust has in its borrow-checker. We can only make it slightly easier to catch a specific case of pointer-related bug slightly-earlier (and with a non-trivial amount of upfront implementation effort and ongoing maintenance of the related custom smart pointers). It's definitely arguable that spending that time rethinking the API (and specifically, the ownership semantics thereof) to avoid the need to hand out "borrowed" pointers would solve the problem better and more completely.

I'm not completely sold on it, though I definitely like the theory. I have an implementation I've been using for a while, hoping that continued experimentation and use will lead me to a more solid conclusion.

Answer 3

Personally I think that's overkill.

I would return a reference.

Child& get_child(int index) {
  return *(children[index]);
}

Look at std::vector<Child> you have exactly the same situation. People know that the vector has ownership of the objects in it but if you use operator[] you will get a reference to an owned object.

I think people already realize that when you return a reference they should not be making a copy (as that will be a complete different object). Also copying something that comes from a container of pointers is dangerious as you will more than likely end up with a sliced object.

Answer 4

Warning, This might be more of an opinion than real reasoning:

Ownership is one of the central themes of resource management in C++ and is best expressed with RAII techniques (for exception safety). As such a raw pointer is never a good idea to hold ownership:

Namely, I already have a lot of raw pointers floating around (think handles received from libraries, or my code which is not using smart pointers)

You should try to change the code you have under control to use smart pointers and to wrap the pointers you get from the libraries into RAII enabled classes ASAP.

After having established that raw pointers are never used to actually hold (or own) data, they are mere pointers (no pun intended) thus already communicating that the ownership lies with someone else.

In this case your borrowed_ptr only helps to raise awareness for those who do not know this convention.

So to summarize this: IMHO borrowed_ptr duplicates what raw pointers should be used for.

Reversed view

To cut to the bone of the question: Where would you use raw pointers when you have borrowed_ptr?

Answer 5

1. The unique_ptr inside a vector:

   You could use std::deque<Child> instead (no smart pointer needed, deque preserves pointers to objects).

2. Why making another class when you have std::reference_wrapper?

   There are even nice helpers - ref and cref instead of make_borrowed.

3. The question of returning raw pointer or reference

   Well, this will be more opinion-based: I find no problem in returning a pointer or a reference (both are good). Pointer is better to signal a distant object that will stay there, while reference may mean short-lived reference, that can get invalidated (e.g. by growing the vector). For this reason, I would rather use raw pointer (or std::reference_wrapper).

   Return unique_ptr if you want to pass ownership, pointer like long-lived iterator and reference like short-lived use now reference.