-2

The following https://godbolt.org/z/5qd8WbYz9 doesn't work and after reading the duplicate I thought I would have to define an custom deleter

However, I managed to get it working https://godbolt.org/z/dzYvsdKP1 by

  • adding a default constructor implemented in BB.cpp (to avoid implicit in-class default constructor)
  • removing the {nullptr} in-class initialization of the unique_ptr

Can someone explain why the {nullptr} make's it break?

It's important to note that std::default_deleter<AA> can be instantiated even when AA is incomplete.

//BB.h
#include <memory>
struct AA;
struct BB
{
    
    // B(); <- needed
    ~BB();
    std::unique_ptr<AA> u{ nullptr }; // need to remove {nullptr} initializer
};
// BB.cpp
#include "BB.h"
struct AA {};
BB::~BB() = default;
// BB::BB() = default; <- needed
// main.cpp
#include "BB.h"
int main()
{
    BB bb{};
}
Tom Huntington
  • 2,260
  • 10
  • 20
  • The default deleter needs to see the complete definition of `AA` to be able to compile. How should it otherwise know if the destructor exists at all? – Aconcagua Jul 03 '23 at 09:23
  • 1
    First, the `default` specifier should be placed in the header IMHO. Second, In order to destroy `BB`, the member `u` shall be destroyed, and `u` needs to destroy the `AA` it points to, and in order to do so, `AA` definition shall be known. – Fareanor Jul 03 '23 at 09:26
  • 1
    @Fareanor the idea is to break the dependency of `AA` on `main.cpp` just use a custom deleter I think – Tom Huntington Jul 03 '23 at 09:28
  • 1
    unique_ptr just like shared_ptr handles underlying objects automatically. That's the idea of the smart pointer. You don't need to bother. Your unique pointer does not know anything about your BB class nor should it. Your code does not compile because e.g. you moved your AA class definition to the source file and then you marked the destructor as default. – Ruh Roh Raggy Jul 03 '23 at 09:28
  • Or give AA an abstract baseclass (interface) and use that for your member pointer, this will also set you on the way to unit testability (where the unique_ptr to the abstract baseclass is injected into BB). And is one of the ways to break dependencies as @TomHuntington mentioned. – Pepijn Kramer Jul 03 '23 at 09:30
  • 1
    @RuhRohRaggy There's a big difference between `unique_ptr` and `shared_ptr` though and you do have to bother a bit with `unique_ptr` in case you want to destroy objects through pointers to a base class. – Ted Lyngmo Jul 03 '23 at 09:31
  • Do you need to avoid circular dependency, i.e. break up `AA.h` including `BB.h` and vice versa? If so, you can provide a *custom* deleter as second template argument to u and implement that one's `operator()` within `BB.cpp`. – Aconcagua Jul 03 '23 at 09:38
  • [Custom deleter demo](https://godbolt.org/z/f9zn6hhqq)… – Aconcagua Jul 03 '23 at 10:11
  • 1
    @Aconcagua reread the question you don't need a custom deleter. Also vote to reopen pls – Tom Huntington Jul 03 '23 at 10:15
  • @Aconcagua okay there is an implicit `BB() = default;` in the class unless we provide it explicitly. What about the `{nullptr}`? Do you know why that breaks it? – Tom Huntington Jul 03 '23 at 10:31
  • @TomHuntington Ah, have been inattentive, sorry… Indeed, that's another issue. Note, though, that this change of the question broke being a duplicate and invalidated all the comments. It would have been better just to ask a new question – just keep that in mind for next time ;) – Aconcagua Jul 03 '23 at 10:41
  • 1
    @Aconcagua you can instantiate `default_delete` to an incomplete type https://godbolt.org/z/7o8Yzhh9c or else it could never work by you first comment. `default_delete` only operates on `T*` – Tom Huntington Jul 03 '23 at 10:50
  • 1
    @TomHuntington Not my day today... *Of course* you can, one is included within a (default) `std::unique_ptr` as well, you couldn't the latter either otherwise ;) – Aconcagua Jul 03 '23 at 11:07

2 Answers2

3

AA can not be an incomplete type at the time BB::~BB is defined since AA must be a complete type at the time std::default_delete::operator() is called (which will happen in BB::~BB. Otherwise, the program is ill-formed.

cppreference:

The reason for this requirement is that calling delete on an incomplete type is undefined behavior in C++ if the complete class type has a nontrivial destructor or a deallocation function, as the compiler has no way of knowing whether such functions exist and must be invoked.

The situation is the same as when you are implementing something using the pimpl idiom.

  • AA can be incomplete when defining BB.
  • AA must be complete when defining BB::~BB.
  • Similarly, leaving BB::BB implicitly defined or = defaulted triggers a static_assert by default_delete in g++ and clang++ since it would then need to define unique_ptr<AA, default_delete<AA>>s destructor too early:
    ~unique_ptr() { default_delete<AA>{}(the_pointer); } // oups, sizeof(AA) unknown
    
  • Dererring the definitions of BB member functions until AA is defined is the idiomatic solution.
Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
-1

So pimpl with assignment initialization doesn't compile for any venders anymore (although it used to on clang and msvc). But with brace initialization it's only broken on msvc (not conformant I guess)

https://godbolt.org/z/KchervEY3

#include <memory>
struct incomplete_type;
struct A
{
    A();
    ~A();
    std::unique_ptr<incomplete_type> u1; // always fine
    std::unique_ptr<incomplete_type> u2{ nullptr }; // error in msvc only
    //std::unique_ptr<incomplete_type> u3 = nullptr; // always error
};
int main()
{
    A bb{};
}

I'm still not sure exactly why assignment initialization shoudln't compile but it was discussed below that P0859r0 brought it about

https://developercommunity.visualstudio.com/t/vs2022-173-Preview-std::unique_ptr-need/10107855#T-N10306156

Tom Huntington
  • 2,260
  • 10
  • 20