unique_ptr, pimpl/forward declaration and complete definition

Question

I already checked out the questions here and here, but still cannot figure out what is wrong.

This is the calling code:

#include "lib.h"

using namespace lib;

int
main(const int argc, const char *argv[]) 
{
    return 0;
}

This is the lib code:

#ifndef lib_h
#define lib_h

#include <string>
#include <vector>
#include <memory>

namespace lib
{

class Foo_impl;

class Foo
{
    public:
        Foo();
        ~Foo();

    private:
        Foo(const Foo&);
        Foo& operator=(const Foo&);

        std::unique_ptr<Foo_impl> m_impl = nullptr;

        friend class Foo_impl;
};

} // namespace

#endif

clang++ gives me this error:

invalid application of 'sizeof' to an incomplete type 'lib::Foo_impl'
note: in instantiation of member function 'std::default_delete::operator()' requested

You can see I already specifically declared Foo destructor. What else am I missing here?

What happens if you change std::unique_ptr m_impl = nullptr; to std::unique_ptr m_impl? I think it should work? — cageman, Sep 15 '14 at 14:46
I think a similar problem occurs here: http://coliru.stacked-crooked.com/a/984df6900bd1ba8b The Standard seems to be vague about whether or not an NSDMI has to be valid even if it is ignored. — dyp, Sep 15 '14 at 15:04
If no one comes up with a decent answer soon, I'll send a mail to std-discussion. The compiler writers seem to agree on the behaviour, but I cannot find where or if this is specified. — dyp, Sep 15 '14 at 15:24
@dyp Sounds reasonable. I also found the standard to be awfully vague in this regard. I could not find a good reference whether the compiler is actually right to reject the Op's example. — ComicSansMS, Sep 15 '14 at 15:31
@dyp I would argue that [your example](http://coliru.stacked-crooked.com/a/984df6900bd1ba8b) is a separate issue. This question is not about accessibility or existence of an appropriate constructor, but about the point of instantiation of such a constructor given its existence. (Obviously `std::unique_ptr` has an appropriately declared non-`explicit` constructor that takes `std::nullptr_t`.) — Casey, Sep 15 '14 at 16:54
I think this is a duplicate of my question http://stackoverflow.com/questions/8595471/does-the-gotw-101-solution-actually-solve-anything but I'm not going to cast the first vote (I wield Mjolnir on these tags, my vote will be the last) — Ben Voigt, Sep 15 '14 at 20:41
@BenVoigt As you might have seen from my comments, I think there's a subtlety here that involves the NSDMIs. I don't quite see where this problem appears in your question. — dyp, Sep 15 '14 at 21:18
@dyp: Well I don't see the data member as being much different from any other use of the `std::unique_ptr`. Of note, constructors also need access to member destructors, for stack unwinding in case of exception. And... you just commented that on Howard's answer. — Ben Voigt, Sep 15 '14 at 21:25

score 12 · Accepted Answer · edited May 23 '17 at 12:34

12

The implementation of Foo_impl must be complete prior to the instantiation required in std::unique_ptr<Foo_impl> m_impl = nullptr.

Leaving the type declared (but not initialised) will fix the error (std::unique_ptr<Foo_impl> m_impl;), you would then need to initialise it later on in the code.

The error you are seeing is from the implementation of a technique used to test for this; the incomplete type. Basically, sizeof will result in an error with types that are only forward declared (i.e. lack definition when used at that point in the code/compilation).

A possible fix here would look like;

class Foo_impl;

class Foo
{
  // redacted
  public:
    Foo();
    ~Foo();

  private:
    Foo(const Foo&);
    Foo& operator=(const Foo&);

    std::unique_ptr<Foo_impl> m_impl;// = nullptr;
};

class Foo_impl {
  // ...
};

Foo::Foo() : m_impl(nullptr)
{
}

Why is the complete type required?

The instantiation via = nullptr uses copy initialisation and requires the constructor and destructor to be declared (for unique_ptr<Foo_impl>). The destructor requires the deleter function of the unique_ptr which, by default, calls delete on the pointer to Foo_impl hence it requires the destructor of Foo_impl, and the destructor of Foo_impl is not declared in the incomplete type (the compiler doesn't know what it looks like). See Howard's answer on this as well.

Key here is that calling delete on an incomplete type results in undefined behaviour (§ 5.3.5/5) and hence is explicitly checked for in the implementation of unique_ptr.

Another alternative for this situation may be to use direct initialisation as follows;

std::unique_ptr<Foo_impl> m_impl { nullptr };

There seems to be some debate on the non-static data member initialiser (NSDMI) and whether this is a context that requires the member definition to exist, at least for clang (and possibly gcc), this seems to be such a context.

edited May 23 '17 at 12:34

Community

1
1

answered Sep 15 '14 at 14:42

Niall

30,036
10
99
142

[unique.ptr]/5 "The template parameter `T` of `unique_ptr` may be an incomplete type." – dyp Sep 15 '14 at 14:43
2

This is a correct answer. The type must be known at the point of _instantiation_ of the unique_ptr (or more precisely, at the point where the deleter is invoked, but since we are missing an explicit destructor, that's the same in this case), but not at the time of declaration. By doing the `= nullptr` in the header, the point of declaration and instantiation both move to a place where the enclosed type is not known. – ComicSansMS Sep 15 '14 at 14:51
@ComicSansMS, True, I'll update the answer to be more clear on this point. – Niall Sep 15 '14 at 14:54
@ComicSansMS A complete type as the template argument is only required when instantiating one of (definitions of) the member functions, not the class itself. – dyp Sep 15 '14 at 14:54
@ComicSansMS The enclosing class `Foo` does have an externally-defined ctor. – dyp Sep 15 '14 at 14:55
@dyp Yes, my initial wording was wrong. Sorry for that. I edited the comment to clarify. – ComicSansMS Sep 15 '14 at 14:55
Isn't the explicit destructor missing in your answer? – leemes Sep 15 '14 at 14:57
@leemes, redacted to focus on the `=nullptr`, but I'll add it back to avoid confusion. – Niall Sep 15 '14 at 14:59
@Niall I guessed so. Wanted to give you a hint about the possible reason for the (unexplained) downvote. – leemes Sep 15 '14 at 15:00
@leemes, Thanks. I think the downvote was that the very first wording was terse. Admittedly too terse. – Niall Sep 15 '14 at 15:02
I'm still missing an explanation in this answer: Why does `= nullptr` require the type to be complete? Intuitively it only changes the implementation of the constructor (i.e. syntactic sugar for your fix)... – leemes Sep 15 '14 at 15:02
1

@leemes, the `=nullptr` requires the constructor and destructor. The destructor requires the cleanup function (which by default requires the destructor of `Foo_impl` which is not present in the incomplete type (the compiler doesn't know what it looks like). – Niall Sep 15 '14 at 15:04
@Niall Yeah but why does `= nullptr` require the constructor and destructor (I guess you mean of Foo?) in the first place? Everything else is clear and already answered. I mean, intuitively it should only be relevant inside of the definition of Foo's constructor, which is not here. The constructor is the only piece of code which should care about that, or am I missing something? I always thought of the initializations in the header of syntactic sugar for the initialization in the initializer list of the constructor. – leemes Sep 15 '14 at 15:09
@leemes, I've updated the post, basically it results in undefined behaviour. The destructor for the `Foo_impl` (the argument for `unique_ptr`). – Niall Sep 15 '14 at 15:10
@Niall Let's not look at the fact that it is a `unique_ptr`. If I add a `Bar bar = 0` in Foo's header, why does it try to call Bar's destructor **in the header** instead of in the **implementation** of Foo's constructor? – leemes Sep 15 '14 at 15:11
@leemes, if `Bar` is not a pointer or reference, then the complete type will be needed for more than the destructor, it'll be needed for the size of `Bar` as well. One key aspect is the the destructor the the `unique_ptr` eventually requires a `delete` and the `delete` on an incomplete type is undefined behaviour, so the implementation of the library prohibits its use. – Niall Sep 15 '14 at 15:17
We confuse things here. In my example, `Bar` replaces `unique_ptr`, of which we talk about when the **destructor** is called. In case of `unique_ptr` it requires its template parameter to be complete. I wanted to emphasize that my question is about why the destructor is being called (or is required to be callable) outside of Foo's destructor. My understanding is that this is the only place where it is relevant, but putting `= nullptr` there seems to require the destructor of `unique_ptr` to be callable (and this in turn requires `Foo_impl` to be complete). – leemes Sep 15 '14 at 15:20
@leemes The destructor of data members typically is required for a stack unwinding if during the construction of the class (here: `Foo`) an exception is thrown. – dyp Sep 15 '14 at 15:28
1

@Niall I do not see where the Standard says that `= nullptr` immediately does or require something. In [class.base.init]/8, NSDMIs are linked to initialization. But the wording IMHO suggests that this initialization happens *inside* the constructor. Which is defined outside the class definition, supposedly after `Foo_impl` has been completed. – dyp Sep 15 '14 at 15:29
1

To emphasize what I am not understanding, it's the very first sentence of your explanation: *"The instantiation via `= nullptr` requires the constructor and destructor to be declared (of the `unique_ptr` here)."* => Why? I mean, why not only in the definition of the constructor and destructor of `Foo`? – leemes Sep 15 '14 at 15:30
@leemes, Correct. The `=nullptr` instantiates the destructor of `unique_ptr`, this triggers a chain of instantiations that eventually lead to `delete impl` where `impl` is a pointer to `Foo_impl`, at which point the library has a compile time check to test for the complete type. – Niall Sep 15 '14 at 15:30
@dyp. I'm not sure here. I wonder if there is possibly some lack of clarity here in the standard. Certainly in the implementation of at least clang, we see it does require the complete type here. – Niall Sep 15 '14 at 15:32
2

@Niall We're talking at cross purposes. I'm specifically asking what's the difference between `... impl = nullptr` in header and `impl(nullptr)` in the constructor definition? Why does it "move" the requirement into the header? Intuitively, nobody except the constructor should care about `...=...` in the header. I thought it was syntactic sugar for `...(...)` in the ctor init list. But apparently it does more than that. – leemes Sep 15 '14 at 15:33
What's interesting is the following. I wrote a template `class Bar` which requires `T` to be complete in its destructor (similar to `unique_ptr`. Then I tried to confirm the situation but failed. See the following code snippets (ignore the linker errors): [1](http://ideone.com/zUTUhq), [2](http://ideone.com/vmGU8t), [3](http://ideone.com/bZljRI), [4](http://ideone.com/vyXyNG). They differ in the line with the member variable. 1 and 2 use my template class, 3 and 4 `unique_ptr`. 1 and 3 add `= {}`. Only 3 complains about the incomplete type. – leemes Sep 15 '14 at 15:46
1

@leemes The difference seems to be the triviality of `Bar`'s default constructor. [Adding a non-trivial constructor to the definition of `Bar` in your example 1 generates the error](http://ideone.com/kH6Ooq). – Casey Sep 15 '14 at 15:58
Comments are not for extended discussion; this conversation has been [moved to chat](http://chat.stackoverflow.com/rooms/61290/discussion-on-answer-by-niall-unique-ptr-pimpl-forward-declaration-and-complete). – Taryn Sep 15 '14 at 16:07

score 9 · Answer 2 · edited May 23 '17 at 11:53

9

The statement:

std::unique_ptr<Foo_impl> m_impl = nullptr;

invokes copy-initialization. This has the same semantics as:

std::unique_ptr<Foo_impl> m_impl = std::unique_ptr<Foo_impl>(nullptr);

I.e. it constructs a temporary prvalue. This temporary prvalue must be destructed. And that destructor needs to see the complete type of Foo_impl. Even if the prvalue and move construction is elided, the compiler must behave "as if".

You can instead use direct-initialization, and the unique_ptr destructor will no longer be required at this point:

std::unique_ptr<Foo_impl> m_impl{nullptr};

Update

Casey points out that gcc-4.9 currently instantiates ~unique_ptr() even for the direct-initialization form. However in my tests clang does not. I do not know what other compilers may do. I believe that clang is conforming in this regard, at least with the most recent core defect reports factored in.

edited May 23 '17 at 11:53

Community

1
1

answered Sep 15 '14 at 17:56

Howard Hinnant

206,506
52
449
577

The destructor *should* not be required, but at least [g++ instantiates it anyway](http://coliru.stacked-crooked.com/a/f21c6510e6b2eb83). – Casey Sep 15 '14 at 18:14
@Casey: Are you speaking about the second (direct-initialization) form? Ah, followed your link and I see that you are. Thanks. – Howard Hinnant Sep 15 '14 at 18:24
Nice. Finally a deep explanation. So direct-initialization is (or should be) the equivalent of "the old way" (i.e. member initializer list)? Why is it that the C++11 feature to be able to initialize members in the class definition directly is then commonly advertised with code snippets using copy-initialization? (This is off-topic, I'm just wondering...) – leemes Sep 15 '14 at 20:27
@leemes: My best guess is that we (the community) are still learning how to correctly use the feature. – Howard Hinnant Sep 15 '14 at 20:59
2

Isn't the dtor also required if the construction of `Foo` exits via an exception? (destruction of constructed sub-objects during stack unwinding) – dyp Sep 15 '14 at 21:20

score 4 · Answer 3 · edited Sep 15 '14 at 14:55

4

Replace

std::unique_ptr<Foo_impl> m_impl = nullptr;

with

std::unique_ptr<Foo_impl> m_impl;

to fix the error.

edited Sep 15 '14 at 14:55

David G

94,763
41
167
253

answered Sep 15 '14 at 14:47

Jarod42

203,559
14
181
302

Ah i should have posted my idea as an answer, but i was unable to verify :) This should be the solution! – cageman Sep 15 '14 at 14:47
@5gon12eder [Which version of gcc](http://coliru.stacked-crooked.com/a/65089239b4cb51bd)? – dyp Sep 15 '14 at 14:49
@dyp g++ (GCC) 4.9.0 20140604 (prerelease) – 5gon12eder Sep 15 '14 at 14:50
Yes I was referring to the fix. Didn't realize it was ambiguous. – 5gon12eder Sep 15 '14 at 14:52
@5gon12eder Ah, ok. I wonder what's the problem with the NSDMI. The idea of externalizing the special member functions (like the default ctor) is to define them after `Foo_impl` has been completed. – dyp Sep 15 '14 at 14:53
6

I'm missing an explanation in this answer: Why does `= nullptr` require the type to be complete? +1 if you add it. – leemes Sep 15 '14 at 14:58
@leemes: It is outside of my competence :-( – Jarod42 Sep 15 '14 at 17:08
@Jarod42 I first thought it is very simple and I guessed you surely know it, but then it turned out to be very tricky (see comments in other answer)... So I give you the +1 anyway :) – leemes Sep 15 '14 at 17:17

Casey · Answer 4 · 2014-09-15T17:45:47.013

2

N3936 [temp.inst]/2 states:

Unless a member of a class template or a member template has been explicitly instantiated or explicitly specialized, the specialization of the member is implicitly instantiated when the specialization is referenced in a context that requires the member definition to exist; in particular, the initialization (and any associated side-effects) of a static data member does not occur unless the static data member is itself used in a way that requires the definition of the static data member to exist.

So this question really comes down to whether or not a declaration with a non-static data member initializer (NSDMI) constitutes "a context that requires the member definition to exist" with respect to the destructor of that member's type. While it's clear that the declarations of the type's constructor are immediately required to determine if the NSDMI is of an appropriate type to initialize the member, I would say the definitions of the constructor/destructor are required only by the constructor/destructor of the enclosing type and that the implementations are non-conforming.

That said, there several issues with the semantics of NSDMI that are currently being reviewed by the core language group:

so it's not suprising that there is confusion here.

edited Sep 15 '14 at 17:45

answered Sep 15 '14 at 16:32

Casey

41,449
7
95
125

It's not even clear to me whether or not an appropriate declaration of a ctor is required. That's what my templated example was intended to show: An error currently occurs even if the NSDMI is not used, and it's not obvious to me if it should be an error. – dyp Sep 15 '14 at 17:10
@dyp A NSDMI is still an initializer, despite being a very special case. All the requirements of 8.5 therefore still apply and must be diagnosed appropriately, whether or not the NSDMI is actually ODR-used. – Casey Sep 15 '14 at 17:25
The semantics of NSDMI are described (only) as far I can see in [class.base.init]/8, which then refers to 8.5. But this seems to take part *inside* the constructor, i.e. NSDMIs being syntactic sugar for (== equivalent to) mem-initializers. – dyp Sep 15 '14 at 17:27
@dyp 8.5/1 says "The process of initialization described in the remainder of 8.5 applies also to initializations specified by other syntactic contexts, such as the initialization of function parameters with argument expressions (5.2.2) or the initialization of return values (6.6.3)." so it implicitly applies. Excluding NSDMI from 8.5 would require an explicit exception. – Casey Sep 15 '14 at 17:35
I'm not trying to say that NSDMIs do not follow the rules of 8.5, I'm trying to say that they do so according to [class.base.init]/8. Obviously the initialization is performed when the ctor is called; the question now is, when are the checks performed? When do the rules of 8.5 apply? Directly to point of definition, or inside the ctor? – dyp Sep 15 '14 at 17:38
@dyp There is not a question of when and where, the standard does not care about when and where, it only describes the syntactic and semantic form of conforming programs. A program that contains the declaration `int i = "bob";` is ill-formed and requires a diagnostic whether it's the declaration of a global, a local, or a non-static data member. – Casey Sep 15 '14 at 17:53

unique_ptr, pimpl/forward declaration and complete definition

4 Answers4

Linked

Related