9 most common pitfalls every C++ programmer eventually falls into

C++ is an old and complex language with a lot of legacy and dark avenues where problems lurk. Aside of very obscure and arcane problems you might experience when working with C++, there’s a set of very common ones which eventually everyone will come across. So, let’s have some fun and name a few.

Most vexing parse

Most vexing parse is a declaration ambiguity in the language. What might look like a variable declaration, really is a function declaration. Here’s an example:

1
2
3
4
5
6

void numbers_is_not_what_you_might_think() {
    std::vector<int> numbers();
    for (const auto i : numbers) {
        // ...
    }
}

numbers is actually a function prototype. This won’t compile but thankfully, new compilers will warn you about the problem:

1
2
3
4
5
6
7
8
9

/Users/tomasz/vexing.cpp:4:29: warning: empty parentheses interpreted as a function declaration [-Wvexing-parse]
    std::vector<int> numbers();
                            ^~
/Users/tomasz/vexing.cpp:4:29: note: replace parentheses with an initializer to declare a variable
    std::vector<int> numbers();
                            ^~
                            {}
/Users/tomasz/vexing.cpp:5:23: error: invalid range expression of type 'std::vector<int> ()'; no viable 'begin' function available
    for (const auto i : numbers) {

This problem has been remedied with list initialisation introduced in C++11 and just as the compiler suggests, parentheses have to be replaced with {} to fix the problem and remove the ambiguity.

Why not just declare as: std::vector<int> numbers;? Well that brings me to my second point.

Expecting zero initialisation to always happen for free

Zero initialisation is not done automatically for all declarations. There’s a significant difference between the below:

1
2

int i;   // won't be zero-initialised
int j{}; // will be zero-initialised

C++’s main principle which is

you only pay for what you use

is very much applicable here.

This is not a huge problem with std::vector as it has a set of constructors making sure that it’s always instantiated with a well defined state but in case of aggregates with no constructors, there’s a difference. Consider the following:

1
2
3
4
5
6
7
8

// `Point` has no constructors
struct Point {
    int x;
    int y;
};

Point p1;   // x, y won't be zero-initialised
Point p2{}; // x, y will be zero-initialised

const shared_ptr& is not const

Before talking about shared_ptr let’s do a short pointers grammar recap.

Pointer to constant data

const int* p - p is a pointer to constant int. The pointer itself can be changed, the data it points to can’t be altered.

1
2
3
4

void foo(const int* p) {
    // *p = 123; // won't compile - an attempt to modify data through the pointer
    p = (int*)0x12345678; // the pointer itself can be modified
}

Constant pointer to data

int* const p - p is a constant pointer to int. The pointer itself can’t be changed, the data it points to can be changed without any problems.

1
2
3
4

void foo(int* const p) {
    *p = 123; // no problems at all
    // p = (int*)0x12345678; // won't compile - an attempt to modify a const pointer
}

Constant pointer to constant data

const int* const p - p is a constant pointer to constant int. Both the data and the pointer are constant. Nothing can be changed.

1
2
3
4

void foo(const int* const p) {
    // *p = 123; // won't compile
    // p = (int*)0x12345678; // won't compile
}

With the above out of the way, let’s go back to std::shared_ptr. One would expect similar semantics when using a const reference to a smart pointer i.e.:

1
2
3
4

void foo(const std::shared_ptr<int>& p) {
    *p = 123; // this is fine
    // p.swap(std::make_shared<int>(456)); // won't compile swap() is not a `const` function
}

The assignment works because operator* is actually const. Therefore operator* guarantees that shared_ptr won’t be mutated but it returns T* so, data can be altered with no limits.

As expected, the pointer can’t be re-seated as swap is not a const function.

Calling shared_from_this from constructors

On a note of shared_ptr, I’ve been bitten in the past, trying to use shared_from_this in a constructor. Consider the following:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21

class Foo : public std::enable_shared_from_this<Foo> {
public:
    Foo();

    void hello() {
        std::cout << "hello from Foo" << std::endl;
    }
};

void use_foo_ptr(std::shared_ptr<Foo> foo) {
    foo->hello();
}

Foo::Foo() {
    use_foo_ptr(shared_from_this());
}

int main() {
    auto foo = std::make_shared<Foo>();
    return 0;
}

This will crash. The reason is simple. std::enable_shared_from_this internally contains a std::weak_ptr to Foo. It cannot obtain this pointer though until the construction is finished. So, an attempt to use shared_from_this in constructor is a bit of a catch 22 situation - you want to use shared_from_this - which locks a shared_ptr from weak_ptr but that weak_ptr is only going to be set once you’re done with construction.

The usual workaround is to do two stage initialisation like so:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30

class Foo : public std::enable_shared_from_this<Foo> {
public:
    void hello() {
        std::cout << "hello from Foo" << std::endl;
    }

    static std::shared_ptr<Foo> create();

private:
    void init();
};

void use_foo_ptr(std::shared_ptr<Foo> foo) {
    foo->hello();
}

std::shared_ptr<Foo> Foo::create() {
    auto foo = std::make_shared<Foo>();
    foo->init();
    return foo;
}

void Foo::init() {
    use_foo_ptr(shared_from_this());
}

int main() {
    auto foo = Foo::create();
    return 0;
}

Using virtual functions in constructors/destructors

Constructors and destructors are truly a special functions that should always be treated with care and attention. One might be surprised that the following:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22

class Base {
public:
    Base() {
        foo();
    }

    virtual void foo() {
        std::cout << "Base::foo" << std::endl;
    }
};

class Derived : public Base {
public:
    void foo() override {
        std::cout << "Derived::foo" << std::endl;
    }
};

int main() {
    auto p = std::make_shared<Derived>();
    return 0;
}

Will actually produce:

1
2

$ ./a.out 
Base::foo

Instead of expected Derived::foo. Why does that happen?

The construction of Derived will begin once the Base::constructor is done so, it’s not possible to call anything from Derived at that stage as it’s in undefined state, therefore as a rule of thumb

What happens if we attempt to call a pure virtual function?

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20

class Base {
public:
    Base() {
        foo();
    }

    virtual void foo() = 0;
};

class Derived : public Base {
public:
    void foo() override {
        std::cout << "Derived::foo" << std::endl;
    }
};

int main() {
    auto p = std::make_shared<Derived>();
    return 0;
}

This is an undefined behaviour very well described in the C++ standard. The compiler will issue a warning (this should be a compile error, in my opinion, but unfortunately it’s not) in such instance:

1
2

warning: call to pure virtual member function 'foo' has undefined behavior; overrides of 'foo' in subclasses are not available in the constructor of 'Base' [-Wcall-to-pure-virtual-from-ctor-dtor]
        foo();

Although this is an undefined behaviour, on most platforms you can expect either a SIGABRT similar to this one:

1
2
3

$ ./a.out 
libc++abi: Pure virtual function called!
Abort trap: 6

or just a link-error (which is probably a much better outcome):

1
2
3

/usr/bin/ld: /tmp/cciBZg4z.o: in function `Base::Base()':
(.text._ZN4BaseC2Ev[_ZN4BaseC5Ev]+0x26): undefined reference to `Base::foo()'
collect2: error: ld returned 1 exit status

Confusion around universal/forwarding and rvalue references

Universal references are a completely different beast than rvalue references. Depending on the value provided, they can be easily converted to rvalue or lvalue references to implement perfect forwarding. It’s easy to mislead one for another though.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12

// x is a universal reference here
template <typename X>
void foo(X&& x) {
    // ...
}

template <typename X>
class Foo {
public:
    // x is an rvalue reference here
    void foo(X&& x);
};

Here’s some more examples:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13

// mc is an rvalue reference
MyClass&& mc = createMyClass();

// mc2 is a universal/forwarding reference
auto&& mc2 = createMyClass();

// v is an rvalue reference
template <typename T>
void f(std::vector<T>&& v);

// t is an universal/forwarding reference
template <typename T>
void f(T&& r);

Throwing exceptions from destructors

Implicitly all destructors are marked as noexcept unless there are class data members or base classes that have a noexcept(false) destructor but, in general, throwing an exception from a destructor is allowed by the language and is a valid, defined behaviour. However, it is problematic and should be avoided at all cost!

The destructors are called due to object’s lifetime end. This may happen due to an explicit deletion, scope exit, ownership release or any other reason. Scope exit itself may occur naturally or as a result of an already thrown exception.

Consequently, it’s quite likely that throwing in a destructor will result in throwing during an already happening exception handling and this leads to std::terminate. This is very well described in both throw keyword documentation and a page detailing exceptions and exception safety.

Most C++ programmers know about this gotcha and, in general, avoid using throw directly in the destructor. It’s easy to forget though about all other functions that you’re calling in the destructor that may not necessarily be noexcept(true) and may throw so, as a good measure, it’s good to wrap the destructor’s body with a try/catch block like so:

1
2
3
4
5
6
7
8
9

Foo::~Foo() {
    try {
        // ...
        // destruction code
        // ...
    } catch(...) {
        // ...
    }
}

Since the body of the destructor is formed entirely of a try/catch block, you might be tempted to just use the function-try-block but there’s another gotcha. The snippet below behaves differently than the prior one:

1
2
3
4
5
6

Foo::~Foo()
    try {
    }
    catch (...) {
        // WARNING: exception is re-thrown here
    }

First of all:

Before any catch clauses of a function-try-block on a destructor are entered, all bases and non-variant members have already been destroyed.

So, to re-phrase, all non static class data members will be destroyed prior to entering any of the catch blocks. Conversely, with try/catch in the destructor’s body, these will still be available.

Additionally (and this one is very important):

Reaching the end of a catch clause for a function-try-block on a destructor also automatically rethrows the current exception as if by throw;, but a return statement is allowed.

As suggested, you need a return to make it work as expected:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22

class SomeClass {
public:
    ~SomeClass()         
        try {
            std::cout << "~SomeClass" << std::endl;
            throw std::runtime_error("from ~SomeClass");
        } catch(const std::exception& e) {
            std::cout << "SomeClass try/catch: " << e.what() << std::endl;

            return; // VERY IMPORTANT!!!!
        } 
};

int main() {
    try {
        SomeClass s;
        throw std::runtime_error("from main");
    } catch(const std::exception& e) {
        std::cerr << "main try/catch: " << e.what() << std::endl;
    }
    return 0;
}

Throwing exceptions from deleter functions

You might be tempted to just throw an exception when overriding a delete operator or providing a custom deleter for a smart pointer but this should never be done. Custom deleter functions for e.g. unique_ptr require that the deleter is a nothrow CopyConstructible callable:

unique_ptr(pointer p, const A& d) noexcept;

(1) (requires that Deleter is nothrow-CopyConstructible)

unique_ptr(pointer p, A&& d) noexcept;

(2) (requires that Deleter is nothrow-MoveConstructible)

As for delete operator - most of these have noexcept(true) in their specification. How to handle de-allocation errors then? Consider using std::exit - which will allow destruction of thread local storage objects, global static variables and will result in execution of functions registered with std::atexit.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16

void custom_deleter(MyClass* ptr) {
    if (fatal error whilst deleting) {
        // Don't throw exceptions
        // throw std::runtime_error("unable to delete");

        // Consider just exiting the programming if the error is
        // non-recoverable to allow for basic clean-up to happen
        std::exit(1);
    }
}

void foo() {
    std::unique_ptr<MyClass, decltype(&custom_deleter)> a(nullptr, custom_deleter);
    a.reset(new MyClass);
    // ...
}

Forgetting to use virtual destructors

When using virtual inheritance it’s very important to use virtual destructors in base classes otherwise, you may have to deal with resource leaks. Consider this example:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18

class Base {
public:
    ~Base() 
    {
        std::cout << "~Base" << std::endl;
    }
};

class Derived : public Base {
public:
    ~Derived() {
        std::cout << "~Derived" << std::endl;
    }
};

int main() {
    std::unique_ptr<Base> p = std::make_unique<Derived>();
}

In the example above, the destruction of Derived is happening through a Base class pointer. Since the Base class’s destructor is not virtual, only Base::~Base will be called. Derived::~Derived won’t be executed - leading to resource leaks or clean-up problems.

To solve that problem ~Base has to be marked as virtual:

1
2
3
4
5

// ...
virtual ~Base() {
    // ...
}
// ...