5

I was reviewing some older code of mine and I saw the code using pointers to implement a tree of Variant objects. It is a tree because each Variant can contain an unordered_map of Variant*.

I looked at the code and wondered why isn't it just using values, a std::vector<Variant>, and std::unordered_map<std::string, Variant>, instead of Variant*.

So I went ahead and changed it. It seemed okay except one thing, I got errors:

/usr/local/include/c++/6.1.0/bits/stl_pair.h:153:11: error: 'std::pair<_T1, _T2>::second' has incomplete type
       _T2 second;                /// @c second is a copy of the second object
           ^~~~~~ main.cpp:11:8: note: forward declaration of 'struct Variant'
 struct Variant
        ^~~~~~~

So I figured I could trick the compiler into delaying the need to know that type, which didn't work either.

Working Not Working! (MCVE)

I thought this worked earlier but it actually doesn't, I forgot ::type on the using HideMap...

#include <vector>
#include <unordered_map>
#include <iostream>

template<typename K, typename V>
struct HideMap
{
    using type = std::unordered_map<K, V>;
};

struct Variant
{
    using array_container = std::vector<Variant>;

    // Does not work either
    using object_container = typename HideMap<std::string, Variant>::type;

    // Fails
    //using object_container = std::unordered_map<std::string, Variant>;

private:
    union Union
    {
        std::int64_t vint;
        array_container varr;
        object_container vobj;

        // These are required when there are union
        // members that need construct/destruct
        Union() {}
        ~Union() {}
    };

    Union data;
    bool weak;
};

int main()
{
    Variant v;
    std::cout << "Works" << std::endl;
}

So, my question is, why does it work okay for vector and not unordered_map?

If the problem is the inability to use incomplete types, is there a way to delay the instantiation of the unordered_map? I really don't want every object property to be a separate new allocation.

Community
  • 1
  • 1
doug65536
  • 6,562
  • 3
  • 43
  • 53
  • `HideMap` doesn't do what you seem to think it does. – T.C. May 14 '16 at 06:13
  • @T.C. How do you know what I think it does? It causes two-phase lookup right? – doug65536 May 14 '16 at 06:48
  • @T.C are you referring to HideMap not actually being in itself an unordered map? – perencia May 14 '16 at 07:00
  • Oh! I forgot ::type! – doug65536 May 14 '16 at 07:00
  • @perencia Thanks, I changed the question. – doug65536 May 14 '16 at 07:04
  • Std collections require complete types. Vector implementations often permit more latitude than is mandated because it follows from the details of the implementation. – Richard Hodges May 14 '16 at 07:12
  • I tried to use partial specialization after `Variant`, to attempt to make it instantiate the map after the `Variant` type is complete, but that did not work either. – doug65536 May 14 '16 at 07:26
  • Probably duplicate of [c++ - How to have an unordered_map where the value type is the class it's in? - Stack Overflow](https://stackoverflow.com/questions/13089388/how-to-have-an-unordered-map-where-the-value-type-is-the-class-its-in) . – user202729 Mar 06 '21 at 16:52

1 Answers1

1

This uses placement new to defer the initialization of the Union to the constructor where Variant is a complete type. You need to reinterpret_cast everywhere you need to use the Union. I made an effort to not have any strict-alignment violations.

#include <algorithm>
#include <iostream>
#include <unordered_map>
#include <vector>

struct Variant {
    Variant();
    ~Variant();

    private:
    std::aligned_union<0, std::vector<Variant>,
                         std::unordered_map<std::string, void *>,
                         std::int64_t>::type data;
};

namespace Variant_detail {
    using array_container = std::vector<Variant>;
    using object_container = std::unordered_map<std::string, Variant>;

    union Union {
        std::int64_t vint;
        array_container varr;
        object_container vobj;

        // These are required when there are union
        // members that need construct/destruct
        Union() {}
        ~Union() {}
    };
}

Variant::Variant() {
    //make sure that std::unordered_map<std::string, Variant> is not too large
    static_assert(sizeof(std::unordered_map<std::string, Variant>) <=
                      sizeof data, "Variant map too big");
    static_assert(alignof(std::unordered_map<std::string, Variant>) <=
                      alignof(decltype(data)), "Variant map has too high alignment");
    auto &my_union = *new (&data) Variant_detail::Union;
    my_union.vint = 42;
}

Variant::~Variant() {
    reinterpret_cast<Variant_detail::Union &>(data).~Union();
}

int main() {
    Variant v;
    std::cout << "Works" << std::endl;
}
nwp
  • 9,623
  • 5
  • 38
  • 68
  • C++14 is okay, I should have tagged it c++14, I'll update tags. – doug65536 May 14 '16 at 08:52
  • @nwp Thanks! Is it even possible for the `static_assert` to fail now? cc @T.C. – doug65536 May 16 '16 at 00:02
  • What about that `void*` though? Where does that memory come from? `new` defeats the purpose. Should I change the implementation so the items in the map refer to some Variant objects held in a vector, and the map just indexes it? – doug65536 May 16 '16 at 00:11
  • @doug65536 In theory `std::unordered_map` could specialize on `void *` or on `sizeof(T)` of the actual type and be bigger/smaller or require different alignments. In practice I would not expect that to ever happen, but the `static_assert`s don't cost much, so I would leave them in. The `void *` is just some type so `unordered_map` compiles, there is no memory behind it. – nwp May 16 '16 at 00:12
  • I'm beginning to think that the vector should probably be a deque, so iterators won't invalidate, and an "object" should be an `unordered_map , reference_wrapper>` pointing into the deque – doug65536 May 16 '16 at 00:16
  • 1
    I feel that this answer is full of undefined behavior. – user202729 Mar 06 '21 at 16:12