Can a compiler place the implementation of an implicitly declared virtual destructor in a single separate translation unit?
Asked Answered
S

1

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.

Spermatozoid answered 22/10, 2019 at 14:36 Comment(4)
What happens if you build the code with Visual Studio with the /permissive- switch?Banks
Same result. I'll put that in the question.Spermatozoid
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.Goldin
Add this to Foo: struct BarDeleter { void operator()(Bar*) const noexcept; }; and change the unique_ptr to std::unique_ptr<Bar, BarDeleter> bar_;. Then in the implementation translation unit, add in void Foo::BarDeleter::operator()(Foo::Bar* p) const noexcept { try { delete p; } catch(...) {/*discard*/}}Copyboy
A
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.

Aerometer answered 22/10, 2019 at 15:22 Comment(8)
This is my reasoning as well, so far.Spermatozoid
@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...Goldin
@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.Spermatozoid
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...Goldin
@DanielLangr Always only seeing empty code boxes on wandbox, now again...Goldin
@Goldin Sorry about it, I don't know what is wrong with Wandbox. Anyway, this answer explains where the problem is.Aerometer
@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<int> data; };Goldin
@Goldin I think I finally understand. The problem is not with the default constructor of std::unique_ptr<Bar>. 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).Aerometer

© 2022 - 2024 — McMap. All rights reserved.