37

For one of my projects, what I really wanted to do was this (simplifying it to the bare minimum);

struct Move
{
    int src;
    int dst;
};

struct MoveTree
{
    Move move;
    std::vector<MoveTree> variation;
};

I must admit that I assumed that it wouldn't be possible to do this directly, I thought a vector of MoveTree s within a MoveTree would be verboten. But I tried it anyway, and it works beautifully. I am using Microsoft Visual Studio 2010 Express.

Is this portable ? Is it good practise ? Do I have anything to worry about ?

Edit: I've asked a second question hoping to find a good way of doing this.

Community
  • 1
  • 1
Bill Forster
  • 6,137
  • 3
  • 27
  • 27
  • std::vector& variation; MoveTree(): variation(*new std::vector){}; ~MoveTree(){delete &variation;} seems to be portable – user396672 Jun 29 '11 at 08:33
  • @Scheff You do realize that this question was created and answered in 2011, right? – SteamyThePunk May 11 '19 at 03:59
  • 1
    @Scheff Maybe I read more malice than you intended. Following up later with factual updates to the state of the art after the fact is one thing, but ridiculing posts about their lack of future knowledge seems less honorable. – SteamyThePunk May 13 '19 at 00:42
  • 1
    @SteamyThePunk "would be verboten" looked to me like a wordplay (with an accidentally mixed in German word). I was not aware that "verboten" is a common English word. Just found out the opposite, as Oxford dict. lists it (with (Surprise.) denoting German origin). Just having learnt something else... (Please forget about it.) – Scheff's Cat May 13 '19 at 05:38

6 Answers6

32

The C++ Standard (2003) clearly says that instantiating a standard container with an incomplete type invokes undefined-behavior.

The spec says in §17.4.3.6/2,

In particular, the effects are undefined in the following cases:

__ [..]
— if an incomplete type (3.9) is used as a template argument when instantiating a template component.
__ [..]

Things have changed with the C++17 standard, which explicitely allows this types of recursion for std::list, std::vector and std::forward_list. For reference, see http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4510.html and this answer: How can I declare a member vector of the same class?

Kai Petzke
  • 2,150
  • 21
  • 29
Nawaz
  • 353,942
  • 115
  • 666
  • 851
  • 4
    +1 one for actually giving a correct answer, citing the standard in doing so. – James Kanze Jun 29 '11 at 07:56
  • Another reason for this is because the size of incomplete types is not known at compile time therefore the compiler doesn't know how much memory it needs to allocate for each instantiation of that structure. – Jesus Ramos Jun 29 '11 at 07:58
  • 3
    I am trying to find the exact quote that explains *where* is the vector instantiated in the above code. Any hints? – David Rodríguez - dribeas Jun 29 '11 at 08:00
  • @David: The line `std::vector variation` instantiating the vector, isn't it? Or are you talking about point of instantiation of the vector? :-/ – Nawaz Jun 29 '11 at 08:13
  • 2
    After going back and forth, I am quite convinced that the declaration that you quote is actually the point of instantiation of the template. Had it been in a member function (including the constructor, whether user defined or implicitly generated) then the code would have been correct, since a class is considered complete inside the definition of the methods (i.e. if the point of instantiation where `MoveTree::MoveTree() : variation() {}` then the code would be correct (§14.7.1 explains the rules for implicit instantiaitons for those curious enough to read on) – David Rodríguez - dribeas Jun 29 '11 at 08:33
  • @David: Are you saying using the initialization-list makes the code correct? Are you not implying `initialization` is same as `instantiation`? As far as I think, instantiation of a template is done at compile-time, and (dynamic) initialization is done at runtime. That means, no matter whether you use initialization-list or not, the behavior of the code is undefined as long as you've `std::vector variation` as member data. – Nawaz Jun 29 '11 at 08:43
  • 1
    @Nawaz: No, I was unsure of where the point of instantiation is, and I just added a comment stating why it matters: if the template was considered instantiated (which it is not, the reference I provided §14.7.1 says otherwise) then the code would be correct, which it is not, I insist. That is: `struct Test { std::vector* p; Test() : p ( new std::vector() ) {} ~Test() { delete p; } };`, as an example is valid because the instantiation (in this *different* case) is not performed in the class definition, but in the constructor. – David Rodríguez - dribeas Jun 29 '11 at 14:29
  • @David: My question is: is that *instantiation* Or *initialization*? – Nawaz Jun 29 '11 at 14:52
  • @Nawaz: I find quite confusing to maintain a conversation with you, I thought it should be clear from the last two comments. The comment that contains the first reference to §14.7.1, so again: I wondered at first where instantiation occurs which *could* be either where the member attribute is declared, or at initialization, as that would have an effect, then after looking at the standard I found that the instantiation happens when the member is declared in the original question's code. – David Rodríguez - dribeas Jun 29 '11 at 15:34
  • Then you asked (and I found confusing) whether *using the initialization list makes it correct* and then went into some strange digression on instantiation of templates happening at compile time (yes, always, but the point of instantiation matters). Trying to make the difference clear (and failing to do so) I tried to provide an example where a) the template is instantiated in the initialization list b) the code is well defined, which is the `Test` type in the previous comment, where I *insisted* that the original code and that example are different. – David Rodríguez - dribeas Jun 29 '11 at 15:37
  • In that `Test` example, *instantiation* of the vector template with the argument `Test` is performed as part of the *initialization* of the member `p`, both of them occur in the same initializer `p( new std::vector() )`. Again, different from having the member attribute as in the originating example. – David Rodríguez - dribeas Jun 29 '11 at 15:38
  • 13
    I think this needs to be updated . Please see [this](http://stackoverflow.com/q/41913578/1870232) – P0W Jan 28 '17 at 19:16
5

MoveTree is an incomplete type inside its definition. The standard does not guarantee instantiation of STL templates with incomplete types.

Mehrdad Afshari
  • 414,610
  • 91
  • 852
  • 789
  • 3
    I think this needs to be updated . Please see [this](http://stackoverflow.com/q/41913578/1870232) – P0W Jan 28 '17 at 19:17
5

Use a pointer to the type in the Vector, this will be portable.

struct Move
    {
        int src;
        int dst;
    };

struct MoveTree;

struct MoveTree
    {
        Move move;
        std::vector<MoveTree*> variation;
    };
David Allan Finch
  • 1,414
  • 8
  • 20
  • But pointers are bugprone; and part of the whole point of using STL containers is to *avoid* manual pointer-jiggling. – Eamon Nerbonne Jun 29 '11 at 08:41
  • 4
    If the only tool you have is a hammer then everything is a nail. – David Allan Finch Jun 29 '11 at 08:53
  • 2
    Alternatively, you can use `shared_ptr> variation` and only dynamically allocate the vector (which will contain the objects by value). This has the negative effect of changing the value semantics of the object, so you will need to provide copy constructor / assignment operator. – David Rodríguez - dribeas Jun 29 '11 at 15:40
  • @David Rodriguez: But doesn't Nawaz's answer say this is undefined behavior? Regardless of whether the restriction makes sense, it still seems against the standard. – Merlyn Morgan-Graham Jul 01 '11 at 04:56
  • 1
    @David Allen Finch: "if the only tool..." I understand the idiom, but not what you are applying it to. The STL, the `std::vector`, or `std:vector` to non-pointers? – Merlyn Morgan-Graham Jul 01 '11 at 05:00
  • 1
    @Merlyn Morgan-Graham: No. When a standard container is *instantiated* the contained type must be *complete*. Now, if the `vector` is dynamically allocated, the point of instantiation moves from the declaration of the member (which in this case it is a `shared_ptr`) to a later time, when the `vector` is actually allocated. That allocation will happen during construction, and a type is always considered *complete* while processing the member functions. That is, in `MoveTree::MoveTree() : variation( new std::vector() ) {}`, `MoveTree` is complete. Read the comments in @Nawaz's answer. – David Rodríguez - dribeas Jul 01 '11 at 07:27
  • @David Rodriguez: I got a similar response from a different answer to the linked question (from Cat Plus Plus). The important part I missed is that it is the *standard containers* that have this restriction, not templates in general. Edit: And it appears I mis-read your answer *again*. He suggested `vector >`. Yours is yet a different beast. – Merlyn Morgan-Graham Jul 01 '11 at 07:33
  • @Merlyn Morgan-Graham: They do differ, but when the problem to solve is *completeness* of the type used as a vector argument, both solve it in different ways: `vector>` solves it by using a complete type `shared_ptr` in the instantiation of the vector even when `X` is incomplete, `shared_ptr>` solves it by delaying instantiation of the `vector` until `X` is complete. The main difference is how the memory is laid out and the implications that it has, with my approach the original memory layout is maintained (all `MoveTree` are contiguous). – David Rodríguez - dribeas Jul 01 '11 at 07:43
  • 2
    Note that there will be roughly the same number of dynamically allocated objects (i.e. `shared_ptr`), as each `MoveTree` either is held by a `smart_ptr` or holds a `smart_ptr` to a vector, I preferred the approach of holding the vector as it allows you to publish a *reference* to it, and user code does not need to be modified (can walk through a `vector`, does not need to perform extra dereferences). – David Rodríguez - dribeas Jul 01 '11 at 07:46
  • Using a smart ptr (whatever its specific version) is a ptr and solves the problem. – David Allan Finch Jul 01 '11 at 10:59
  • 1
    @David, I've just come back to see if there are any new insights and I think your approach is the closest yet to a real answer to my problem. Thanks. Maybe you could write it up as an answer to my follow up question (link now appears in this question). – Bill Forster Jul 02 '11 at 04:06
  • @Bill Forster: In the other question there is a suggestion by @Johannes that is better than this, since the `recursive_wrapper` will take care of providing the copy constructor and assignment operator for you. – David Rodríguez - dribeas Jul 02 '11 at 13:05
  • @David Allan Finch, I am terribly sorry my comment was actually directed at David Rodríguez - dribeas because I like the way his refinement avoids the need to have a vector of pointers. @David Rodríguez - dribeas - see comments above (my previous addressing was ambiguous - too many Davids) – Bill Forster Jul 05 '11 at 22:45
2

The MoveTree elements in std::vector are in an allocated (as in new []) array. Only the control information (the pointer to the array, the size, etc) are stored within the std::vector within MoveTree.

Ben Jackson
  • 90,079
  • 9
  • 98
  • 150
  • 3
    Maybe I'm wrong but stl implementation may choose to allocate the array anywhere, for example, may allocate short arrays in memory reserved inside vector object itself – user396672 Jun 29 '11 at 08:26
2

No, it's not portable. codepad.org does not compile it.

t.cpp:14:   instantiated from here
Line 215: error: '__gnu_cxx::_SGIAssignableConcept<_Tp>::__a' has incomplete type
compilation terminated due to -Wfatal-errors.
Tugrul Ates
  • 9,451
  • 2
  • 33
  • 59
  • 2
    @Eamon 4.2.2 doesn't, at least not if you use the options for strict verification. (I think it's `-D_GLIBCXX_CONCEPT_CHECKS` which controls this.) It's undefined behavior. (It was intended that C++11 require a diagnostic, but that was part of the concepts package, which I don't think made it into the final version.) – James Kanze Jun 29 '11 at 07:55
  • Ok, if I turn that check on - it fails. That's still not a huge portability stumbling block. – Eamon Nerbonne Jun 29 '11 at 08:07
0

You should define copy constructors and assignment operators for both Move and MoveTree when using vector, otherwise it will use the compiler generated ones, which may cause problems.

Blazes
  • 4,721
  • 2
  • 22
  • 29
  • I can't see how the decision of whether or not to supply a user defined copy constructor and assignment operator should in any way be influenced by whether or not the objects are used in a vector. And as it is, I can't see why either of these classes would need either of those things. – Benjamin Lindley Jun 29 '11 at 07:59
  • @Benjamin, thanks for taking the time to comment. I accept the point about it having nothing to do with his question about incomplete the type. vector needs the copy constructor; in his case, the default one. he asked if there was anything to look out for, I thought it was worth pointing out. – Blazes Jun 29 '11 at 09:10
  • The code as it stands (i.e. a type that contains a vector of it's own type) is incorrect, regardless of whether you provide constructors, assignment operators or none of them.. – David Rodríguez - dribeas Jun 29 '11 at 15:41