shared_ptr destructor, copy and incomplete type

Question

I have a header file foo.h like this (unrelated stuff omitted):

#pragma once
#include <memory>

class Bar;


struct Foo
{
  std::shared_ptr<Bar> getBar();

  std::shared_ptr<const Bar> getBar() const
  {
    return const_cast<Foo*>(this)->getBar();
  }
};

The non-const overload of getBar() is implemented in a .cpp file, which also sees the full definition of Bar.

When foo.h is included from another file (which does not see the definition of Bar), VS 2010 is giving me a warning like this:

warning C4150: deletion of pointer to incomplete type 'Bar'; no destructor called

on the const overload of getBar() (or actually on something deep in the standard library instantiated from that overload).

My question is whether that warning can safely be ignored.

The way I look at it, there are two member functions of std::shared_ptr<Bar> being called in getBar() const: the converting constructor and the destructor.

// converting constructor
template <class Y>
std::shared_ptr<const Bar>::shared_ptr(std::shared_ptr<Y> &&r)

This is used to initialise the return value of getBar() const from the return value of getBar(). This does not list any prerequisites (C++11 27.2.2.1 §20-22) which would require Y (Bar in my case) to be complete.

// destructor
std::shared_ptr<const Bar>::~shared_ptr()

27.2.2.2 §1 states that when the shared pointer being destroyed is empty, there are no side effects.

I understand why I'm getting the warning - the destructor code also has to care for the situation when delete has to be called on the stored pointer, and this code would indeed delete an incomplete type. But the way I see it, it can never be reached in my situation, so getBar() const is safe.

Am I correct, or have I overlooked a call or something which could make getBar() const actually delete an incomplete type?

Answer 1

I can find no rationale for the warning. Nor can I replicate the warning with clang/libc++.

In general, given a shared_ptr<Bar>, without seeing the construction of shared_ptr<Bar> that takes a Bar*, and optionally a deleter, there is no way to know for sure if ~Bar() is ever called. There is no way to know what deleter was stored in the shared_ptr<Bar>, and given some unknown deleter d that is stored in the shared_ptr<Bar>, along side of the Bar* (say p), there is no requirement that d(p) call ~Bar().

For example, your Bar may have no accessible destructor:

class Bar
{
    ~Bar();
};

And your Foo::getBar() could be implemented like this:

std::shared_ptr<Bar>
Foo::getBar()
{
    // purposefully leak the Bar because you can't call ~Bar()
    return std::shared_ptr<Bar>(new Bar, [](Bar*){});
}

There is no way for the compiler to know without seeing foo.cpp.

This warning looks like a compiler bug to me, or possibly a bug in the implementation of std::shared_ptr.

Can you ignore the warning? I don't know. It looks to me like you are dealing with a bug in the implementation, and so that bug could well mean that the warning is real. But assuming a fully conforming implementation, I see no requirement for Bar to be a complete type in the code you have shown.

Answer 2

No; the warning cannot safely be ignored. Your code creates a shared_ptr object. The shared_ptr constructor is a template that creates and stores the deleter. By adding code to create a shared_ptr in your header, you wound up prematurely instantiating the template constructor.

The deleter trick used by shared_ptr allows you to declare them before defining the class, but they still need to see the complete type before you first use them. Your code has no guarantee to call Bar's destructor, ever. What's worse is that, today, it could even work, but might be a time bomb, making a very difficult to debug bug appear in your code.

The problem has nothing to do with the contents of the pointer in your code. Just the fact that you have code that creates a shared pointer, which does not see the complete definition of Bar, is enough.

The fix is easy. Just put that code in your implementation file, after the complete declaration of Bar.

Edit: The warning given by g++ fits this situation, but the code does not. For now I'm going to leave this answer up until I have time to investigate (or someone else figures out why g++ gives this warning).