R.smart

R.smart: Smart pointers

R.20: Use unique_ptr or shared_ptr to represent ownership

Reason

They can prevent resource leaks.

Example

Consider:

void f()
{
    X* p1 { new X };              // bad, p1 will leak
    auto p2 = make_unique<X>();   // good, unique ownership
    auto p3 = make_shared<X>();   // good, shared ownership
}

This will leak the object used to initialize p1 (only).

Enforcement

• (Simple) Warn if the return value of new is assigned to a raw pointer.
• (Simple) Warn if the result of a function returning a raw owning pointer is assigned to a raw pointer.

R.21: Prefer unique_ptr over shared_ptr unless you need to share ownership

Reason

A unique_ptr is conceptually simpler and more predictable (you know when destruction happens) and faster (you don't implicitly maintain a use count).

Example, bad

This needlessly adds and maintains a reference count.

void f()
{
    shared_ptr<Base> base = make_shared<Derived>();
    // use base locally, without copying it -- refcount never exceeds 1
} // destroy base

Example

This is more efficient:

void f()
{
    unique_ptr<Base> base = make_unique<Derived>();
    // use base locally
} // destroy base

Enforcement

(Simple) Warn if a function uses a Shared_pointer with an object allocated within the function, but never returns the Shared_pointer or passes it to a function requiring a Shared_pointer&. Suggest using unique_ptr instead.

R.22: Use make_shared() to make shared_ptrs

Reason

make_shared gives a more concise statement of the construction. It also gives an opportunity to eliminate a separate allocation for the reference counts, by placing the shared_ptr's use counts next to its object.

Example

Consider:

shared_ptr<X> p1 { new X{2} }; // bad
auto p = make_shared<X>(2);    // good

The make_shared() version mentions X only once, so it is usually shorter (as well as faster) than the version with the explicit new.

Enforcement

(Simple) Warn if a shared_ptr is constructed from the result of new rather than make_shared.

R.23: Use make_unique() to make unique_ptrs

Reason

make_unique gives a more concise statement of the construction. It also ensures exception safety in complex expressions.

Example

unique_ptr<Foo> p {new Foo{7}};    // OK: but repetitive

auto q = make_unique<Foo>(7);      // Better: no repetition of Foo

Enforcement

(Simple) Warn if a unique_ptr is constructed from the result of new rather than make_unique.

R.24: Use std::weak_ptr to break cycles of shared_ptrs

Reason

shared_ptr's rely on use counting and the use count for a cyclic structure never goes to zero, so we need a mechanism to be able to destroy a cyclic structure.

Example

#include <memory>

class bar;

class foo {
public:
  explicit foo(const std::shared_ptr<bar>& forward_reference)
    : forward_reference_(forward_reference)
  { }
private:
  std::shared_ptr<bar> forward_reference_;
};

class bar {
public:
  explicit bar(const std::weak_ptr<foo>& back_reference)
    : back_reference_(back_reference)
  { }
  void do_something()
  {
    if (auto shared_back_reference = back_reference_.lock()) {
      // Use *shared_back_reference
    }
  }
private:
  std::weak_ptr<foo> back_reference_;
};

Note

??? (HS: A lot of people say "to break cycles", while I think "temporary shared ownership" is more to the point.) ???(BS: breaking cycles is what you must do; temporarily sharing ownership is how you do it. You could "temporarily share ownership" simply by using another shared_ptr.)

Enforcement

??? probably impossible. If we could statically detect cycles, we wouldn't need weak_ptr

R.30: Take smart pointers as parameters only to explicitly express lifetime semantics

See F.7.

R.31: If you have non-std smart pointers, follow the basic pattern from std

Reason

The rules in the following section also work for other kinds of third-party and custom smart pointers and are very useful for diagnosing common smart pointer errors that cause performance and correctness problems. You want the rules to work on all the smart pointers you use.

Any type (including primary template or specialization) that overloads unary * and -> is considered a smart pointer:

• If it is copyable, it is recognized as a reference-counted shared_ptr.
• If it is not copyable, it is recognized as a unique unique_ptr.

Example, bad

// use Boost's intrusive_ptr
#include <boost/intrusive_ptr.hpp>
void f(boost::intrusive_ptr<widget> p)  // error under rule 'sharedptrparam'
{
    p->foo();
}

// use Microsoft's CComPtr
#include <atlbase.h>
void f(CComPtr<widget> p)               // error under rule 'sharedptrparam'
{
    p->foo();
}

Both cases are an error under the sharedptrparam guideline: p is a Shared_pointer, but nothing about its sharedness is used here and passing it by value is a silent pessimization; these functions should accept a smart pointer only if they need to participate in the widget's lifetime management. Otherwise they should accept a widget*, if it can be nullptr. Otherwise, and ideally, the function should accept a widget&. These smart pointers match the Shared_pointer concept, so these guideline enforcement rules work on them out of the box and expose this common pessimization.

R.32: Take a unique_ptr<widget> parameter to express that a function assumes ownership of a widget

Reason

Using unique_ptr in this way both documents and enforces the function call's ownership transfer.

Example

void sink(unique_ptr<widget>); // takes ownership of the widget

void uses(widget*);            // just uses the widget

Example, bad

void thinko(const unique_ptr<widget>&); // usually not what you want

Enforcement

• (Simple) Warn if a function takes a Unique_pointer<T> parameter by lvalue reference and does not either assign to it or call reset() on it on at least one code path. Suggest taking a T* or T& instead.
• (Simple) ((Foundation)) Warn if a function takes a Unique_pointer<T> parameter by reference to const. Suggest taking a const T* or const T& instead.

R.33: Take a unique_ptr<widget>& parameter to express that a function reseats the widget

Reason

Using unique_ptr in this way both documents and enforces the function call's reseating semantics.

Note

"reseat" means "making a pointer or a smart pointer refer to a different object."

Example

void reseat(unique_ptr<widget>&); // "will" or "might" reseat pointer

Example, bad

void thinko(const unique_ptr<widget>&); // usually not what you want

Enforcement

• (Simple) Warn if a function takes a Unique_pointer<T> parameter by lvalue reference and does not either assign to it or call reset() on it on at least one code path. Suggest taking a T* or T& instead.
• (Simple) ((Foundation)) Warn if a function takes a Unique_pointer<T> parameter by reference to const. Suggest taking a const T* or const T& instead.

R.34: Take a shared_ptr<widget> parameter to express shared ownership

Reason

This makes the function's ownership sharing explicit.

Example, good

class WidgetUser
{
public:
    // WidgetUser will share ownership of the widget
    explicit WidgetUser(std::shared_ptr<widget> w) noexcept:
        m_widget{std::move(w)} {}
    // ...
private:
    std::shared_ptr<widget> m_widget;
};

Enforcement

• (Simple) Warn if a function takes a Shared_pointer<T> parameter by lvalue reference and does not either assign to it or call reset() on it on at least one code path. Suggest taking a T* or T& instead.
• (Simple) ((Foundation)) Warn if a function takes a Shared_pointer<T> by value or by reference to const and does not copy or move it to another Shared_pointer on at least one code path. Suggest taking a T* or T& instead.
• (Simple) ((Foundation)) Warn if a function takes a Shared_pointer<T> by rvalue reference. Suggesting taking it by value instead.

R.35: Take a shared_ptr<widget>& parameter to express that a function might reseat the shared pointer

Reason

This makes the function's reseating explicit.

Note

"reseat" means "making a reference or a smart pointer refer to a different object."

Example, good

void ChangeWidget(std::shared_ptr<widget>& w)
{
    // This will change the callers widget
    w = std::make_shared<widget>(widget{});
}

Enforcement

• (Simple) Warn if a function takes a Shared_pointer<T> parameter by lvalue reference and does not either assign to it or call reset() on it on at least one code path. Suggest taking a T* or T& instead.
• (Simple) ((Foundation)) Warn if a function takes a Shared_pointer<T> by value or by reference to const and does not copy or move it to another Shared_pointer on at least one code path. Suggest taking a T* or T& instead.
• (Simple) ((Foundation)) Warn if a function takes a Shared_pointer<T> by rvalue reference. Suggesting taking it by value instead.

R.36: Take a const shared_ptr<widget>& parameter to express that it might retain a reference count to the object ???

Reason

This makes the function's ??? explicit.

Example, good

void share(shared_ptr<widget>);            // share -- "will" retain refcount

void reseat(shared_ptr<widget>&);          // "might" reseat ptr

void may_share(const shared_ptr<widget>&); // "might" retain refcount

Enforcement

• (Simple) Warn if a function takes a Shared_pointer<T> parameter by lvalue reference and does not either assign to it or call reset() on it on at least one code path. Suggest taking a T* or T& instead.
• (Simple) ((Foundation)) Warn if a function takes a Shared_pointer<T> by value or by reference to const and does not copy or move it to another Shared_pointer on at least one code path. Suggest taking a T* or T& instead.
• (Simple) ((Foundation)) Warn if a function takes a Shared_pointer<T> by rvalue reference. Suggesting taking it by value instead.

R.37: Do not pass a pointer or reference obtained from an aliased smart pointer

Reason

Violating this rule is the number one cause of losing reference counts and finding yourself with a dangling pointer. Functions should prefer to pass raw pointers and references down call chains. At the top of the call tree where you obtain the raw pointer or reference from a smart pointer that keeps the object alive. You need to be sure that the smart pointer cannot inadvertently be reset or reassigned from within the call tree below.

Note

To do this, sometimes you need to take a local copy of a smart pointer, which firmly keeps the object alive for the duration of the function and the call tree.

Example

Consider this code:

// global (static or heap), or aliased local ...
shared_ptr<widget> g_p = ...;

void f(widget& w)
{
    g();
    use(w);  // A
}

void g()
{
    g_p = ...; // oops, if this was the last shared_ptr to that widget, destroys the widget
}

The following should not pass code review:

void my_code()
{
    // BAD: passing pointer or reference obtained from a non-local smart pointer
    //      that could be inadvertently reset somewhere inside f or its callees
    f(*g_p);

    // BAD: same reason, just passing it as a "this" pointer
    g_p->func();
}

The fix is simple -- take a local copy of the pointer to "keep a ref count" for your call tree:

void my_code()
{
    // cheap: 1 increment covers this entire function and all the call trees below us
    auto pin = g_p;

    // GOOD: passing pointer or reference obtained from a local unaliased smart pointer
    f(*pin);

    // GOOD: same reason
    pin->func();
}

Enforcement

• (Simple) Warn if a pointer or reference obtained from a smart pointer variable (Unique_pointer or Shared_pointer) that is non-local, or that is local but potentially aliased, is used in a function call. If the smart pointer is a Shared_pointer then suggest taking a local copy of the smart pointer and obtain a pointer or reference from that instead.