8

The following code compiles and links with Visual Studio (both 2017 and 2019 with /permissive-), but does not compile with either gcc or clang.

foo.h

#include <memory>

struct Base {
    virtual ~Base() = default; // (1)
};

struct Foo : public Base {
    Foo();                     // (2)
    struct Bar;
    std::unique_ptr<Bar> bar_;
};

foo.cpp

#include "foo.h"

struct Foo::Bar {};            // (3)
Foo::Foo() = default;

main.cpp

#include "foo.h"

int main() {
    auto foo = std::make_unique<Foo>();
}

My understanding is that, in main.cpp, Foo::Bar must be a complete type, because its deletion is attempted in ~Foo(), which is implicitly declared and therefore implicitly defined in every translation unit that accesses it.

However, Visual Studio does not agree, and accepts this code. Additionally, I've found that the following changes make Visual Studio reject the code:

  • Making (1) non-virtual
  • Defining (2) inline -- i.e. Foo() = default; or Foo(){};
  • Removing (3)

It looks to me as if Visual Studio does not define an implicit destructor everywhere it is used under the following conditions:

  • The implicit destructor is virtual
  • The class has a constructor that is defined in a different translation unit

Instead, it seems to only define the destructor in the translation unit that also contains the definition for the constructor in the second condition.

So now I'm wondering:

  • Is this allowed?
  • Is it specified anywhere, or at least known, that Visual Studio does this?

Update: I've filed a bug report https://developercommunity.visualstudio.com/content/problem/790224/implictly-declared-virtual-destructor-does-not-app.html. Let's see what the experts make of this.

Mark
  • 1,016
  • 6
  • 10
  • 1
    What happens if you build the code with Visual Studio with the [/permissive-](https://learn.microsoft.com/en-us/cpp/build/reference/permissive-standards-conformance?view=vs-2019) switch? – Jesper Juhl Oct 22 '19 at 14:46
  • 1
    Same result. I'll put that in the question. – Mark Oct 22 '19 at 14:49
  • 1
    Changes 2 and 3 are clear, you need a complete type when (default) deleter is called (in destructor of unique_ptr, which again happens in constructor of Foo, so when the latter is inline, type needs to be complete already in header). Change 1 surprises me, though, no explanation for. – Aconcagua Oct 22 '19 at 14:54
  • Add this to Foo: `struct BarDeleter { void operator()(Bar*) const noexcept; };` and change the unique_ptr to `std::unique_ptr bar_;`. Then in the implementation translation unit, add in `void Foo::BarDeleter::operator()(Foo::Bar* p) const noexcept { try { delete p; } catch(...) {/*discard*/}}` – Eljay Oct 22 '19 at 15:18

1 Answers1

2

I believe this is a bug in MSVC. As for the std::default_delete::operator(), the Standard says that [unique.ptr.dltr.dflt/4]:

Remarks: If T is an incomplete type, the program is ill-formed.

Since there is no "no diagnostic required" clause, a conforming C++ compiler is required to issue a diagnostic [intro.compliance/2.2]:

If a program contains a violation of any diagnosable rule or ..., a conforming implementation shall issue at least one diagnostic message.

together with [intro/compliance/1]:

The set of diagnosable rules consists of all syntactic and semantic rules in this document except for those rules containing an explicit notation that “no diagnostic is required” or which are described as resulting in “undefined behavior”.

GCC uses static_assert to diagnose the type completeness. MSVC seemingly does not perform such a check. If it silently passes a parameter of std::default_delete::operator() to delete, then, this causes undefined behavior. Which might correspond with your observation. It may work, but until it is guaranteed by the documentation (as a non-standard C++ extension), I wouldn't use it.

Daniel Langr
  • 22,196
  • 3
  • 50
  • 93
  • This is my reasoning as well, so far. – Mark Oct 22 '19 at 15:51
  • 1
    @DanielLangr **OHHHH [@$%*&+!]**, totally overlooked, it's the default *constructor* that is provided, not the *destructor*!!! Having a virtual destructor in the base class has driven me away already... *Sorry*. You are totally right then, of course. Wondering now if providing ctor instead of dtor was deliberate or by accident... – Aconcagua Oct 22 '19 at 16:36
  • 1
    @Aconcagua, it is deliberate. The issue is that msvc defines the implicit destructor in a translation unit where it isn't used, and does not define it in a translation unit where it is used. – Mark Oct 22 '19 at 16:52
  • Hm, but that explains quite a lot: As long as base destructor is virtual, derived is as well. So we need or might need a separate function we can place into the vtable. Apparently MSVC *always* creates that function then and gets along quite well using it – *always*. This explains then, too, why change (1) breaks compilation: As base is not virtual, derived is not either, so *can* be inlined and we end up in the trouble we originally expected as well... (3) is always clear, we never have a complete type then, but now (2) gets a mystery to me... – Aconcagua Oct 22 '19 at 17:06
  • @DanielLangr Always only seeing empty code boxes on wandbox, now again... – Aconcagua Oct 22 '19 at 17:39
  • @Aconcagua Sorry about it, I don't know what is wrong with Wandbox. Anyway, [this answer](https://stackoverflow.com/a/27624369/580083) explains where the problem is. – Daniel Langr Oct 22 '19 at 17:46
  • 1
    @DanielLangr Thanks for the link... But it makes sense for the default constructor as well, just consider a class with complex members: `class Demo { std::vector data; };` – Aconcagua Oct 22 '19 at 17:49
  • 1
    @Aconcagua I think I finally understand. The problem is not with the default constructor of `std::unique_ptr`. The problem is with the default constructor of `Foo`. If there is an exception, `Foo::Foo()` needs to destruct the already-constructed subobject `bar_` (rollback). – Daniel Langr Oct 22 '19 at 17:51