2016-06-28 38 views
2

Ich versuche, die folgende Datenstruktur zu konvertieren:Wie kopiert man eine nicht-triviale C++ Union?

template<typename ValueT, typename ChildT> 
class MyUnion 
{ 
public: 
    MyUnion() : mChild(NULL) {} 
private: 
    union { 
     ChildT* mChild; 
     ValueT* mValue; 
    }; 
}; 

ValueT kann sowohl POD (int, float, usw.) und nicht-triviale Sachen wie Vec3, std::string, die der Grund ist es zunächst als implementiert ist ein Zeiger auf dynamisch zugewiesenen Speicher. Mit C++ 11 können wir den Wert nun direkt in der Klasse speichern. Das Ergebnis ich suche, ist dies:

template<typename ValueT, typename ChildT> 
class MyUnion 
{ 
public: 
    MyUnion() : mChild(NULL) {} 
private: 
    union { 
     ChildT* mChild; 
     ValueT mValue; 
    }; 
}; 

Dies zu ändern, macht der Compiler beschweren sich, dass die Copykonstruktor fehlt, so möchte ich auch

MyUnion(const MyUnion& other); 
MyUnion& operator=(const MyUnion& other); 

und in idealer Weise die Bewegung Bauer implementieren. Zuvor hat der Compiler diese für mich implementiert. Mit POD könnte ich eine memcpy oder etwas Ähnliches machen - kann ich das selbe jetzt benutzen und erwarte das richtige Ergebnis?

+1

Nein, im Allgemeinen dürfen Sie 'memcpy' nicht verwenden. Genau das macht eine Klasse wie "std :: string" "nicht-trivial" (nicht einfach kopierbar, um genau zu sein). Sie müssen wissen, welcher der Gewerkschaftsmitglieder gerade aktiv ist, und Ihren Kopierkonstruktor entsprechend schreiben. Alternativ verwenden Sie etwas wie ['boost :: variant'] (http://www.boost.org/doc/libs/1_61_0/doc/html/variant.html) –

+0

' ValueT' muss auch kopiert und verschoben werden Konstruktor/Zuweisung/etc. Es wird dies für eine Aufgabe viel einfacher machen, denn dann können Sie sie kopieren und konstruieren, wenn 'MyUnion' ist. Sogar POD funktioniert mit initialization-copy und 'std :: move' afaik –

+0

@XerenNarcy Das Problem liegt nicht so sehr darin, * wie * ein Mitglied zu kopieren, als zu wissen, * welches * Mitglied zu kopieren ist. –

Antwort

2

Erstens, wenn mValue ein Zeiger auf dynamisch zugewiesenen Speicher war, dann war der Standard-Kopierkonstruktor für diese Klasse sehr unsicher, es sei denn, Sie waren froh, nur den Speicher zu verlieren.

Weil, welche der Kopien ist verantwortlich, das Objekt zu löschen? Sie sehen beide identisch aus, und es gibt keinen gemeinsamen Zeiger. Also nehme ich an, dass du es gerade durchgesickert hast. (Vielleicht hatten Sie eine "Manager" -Klasse? Aber dann würden Sie nicht fragen, wie Sie es nach Wert in der Union jetzt speichern, würden Sie. Also, tsk für Lecks: p)

In den meisten Fällen, was Sie Wollen Sie ein zusätzliches Flag speichern, das Ihnen sagt, welches Mitglied gerade initialisiert wird. Dann wird es eine "diskriminierte Vereinigung" genannt, da es greifbare Informationen gibt, die Sie verwenden können, um zu unterscheiden, welcher der beiden Zustände es ist.

Ich gebe eine minimale Version, die kopierbar und beweglich ist unter der Annahme, dass ValueT ist.

template<typename ValueT, typename ChildT> 
class MyUnion 
{ 
    public: 
    // Accessors, with ref qualifiers. 
    bool have_value() const { return mHaveValue; } 
    ValueT & get_value() & { return mValue; } 
    ValueT && get_value() && { return std::move(mValue); } 
    ValueT const & get_value() const & { return mValue; } 
    ChildT * & get_child() & { return mChild; } 
    ChildT * && get_child() && { return mChild; } 
    ChildT * const & get_child() const & { return mChild; } 

    // Constructors. Default, copy, and move. 

    MyUnion() { 
     this->init_child(nullptr); 
    } 

    MyUnion(const MyUnion & other) { 
     if (other.have_value()) { 
     this->init_value(other.get_value()); 
     } else { 
     this->init_child(other.get_child()); 
     } 
    } 

    MyUnion(MyUnion && other) { 
     if (other.have_value()) { 
     this->init_value(std::move(other.get_value())); 
     } else { 
     this->init_child(std::move(other.get_child())); 
     } 
    } 

    // Move assignment operator is easier, do that first. 
    // Note that if move ctors can throw, you can get a UB with this. 
    // So in most correct code, you would either ban such objects from 
    // appearing in your union, or try to make backup copies in order 
    // to recover from the exceptions. In this code, I will just 
    // assume that moving your object doesn't throw. 
    // In that case, it's just deinitialize self, then use code from 
    // move ctor. 

    MyUnion & operator = (MyUnion && other) { 
     this->deinitialize(); 
     if (other.have_value()) { 
     this->init_value(std::move(other.get_value())); 
     } else { 
     this->init_child(std::move(other.get_child())); 
     } 
     return *this; 
    } 

    // Copy ctor basically uses "copy and swap", but instead of 
    // swap, we use move assignment. This is exception safe, if 
    // move assignment is. 
    MyUnion & operator = (const MyUnion & other) { 
     MyUnion temp{other}; 
     *this = std::move(temp); 
     return *this; 
    } 

    // Dtor simply calls deinitialize. 
    ~MyUnion() { this->deinitialize(); } 

    private: 
    union { 
     ChildT* mChild; 
     ValueT mValue; 
    }; 
    bool mHaveValue; 

    // these next three methods are private helpers for you. 
    // the users of your class should not mess with these things, 
    // or UB is quite likely! 
    void deinitialize() { 
     if (mHaveValue) { 
     mValue.~ValueT(); 
     } else { 
     // pointer type has no dtor. But if you actually *own* the child, 
     // then you should call delete here I guess. 
     // Or, replace with `std::unique_ptr` and call 
     // that guys dtor. RAII is your friend, you can thank me later. 
     } 
    } 

    // Initialize the value, using perfect forwarding. 
    // Only do this if mValue is not currently initialized! 
    template <typename ... Args> 
    void init_value(Args && ... args) { 
     new (&mValue) ValueT(std::forward<Args>(args)...); 
     mHaveValue = true; 
    } 

    // Here, mChild is a raw pointer, so it doesn't make sense to 
    // make a similar initialization. But if you change it to be an RAII 
    // object, then you should probably do a similar pattern to above, 
    // with perfect forwarding. 
    void init_child(ChildT * c) { 
     mChild = c; 
     mHaveValue = false; 
    } 
}; 

Hinweis: Sie müssen nicht normalerweise Ihre eigene diskriminiert Union so rollen. Häufig ist es besser, eine vorhandene Bibliothek wie boost::variant oder einen der in den Kommentaren genannten Typen expected zu verwenden.Aber, wie dies Ihre eigene kleine diskriminiert Union so

  • nicht so schwer
  • eine gute Übung
  • manchmal eine gute Idee, wenn es an einer API-Grenze angezeigt werden muss oder etwas

In vielen Fällen ist die Verwendung einer Union überhaupt eine unnötige Optimierung, und Sie wären in Ordnung mit nur einem struct. Es wird mehr Speicher benötigt, um das Objekt zu repräsentieren, aber das ist selten von Bedeutung und es könnte für Ihr Team leichter zu verstehen/zu vereinfachen sein.

+0

Ich zeigte nur die wichtigsten Teile des Codes, die ich ändern wollte - es gab kein Speicherleck vorher, also keine Sorgen :) Meine Implementierung entpuppte sich ähnlich wie das, was Sie zur Verfügung gestellt haben, aber ich könnte ein paar Tricks von Ihnen stehlen! Es fühlt sich jedoch so an, als hätten Sie den wichtigsten Teil verpasst: Das Ziel war, keinen Speicher mit "neu" zu belegen. Ansonsten stimme ich Ihrer Lösung zu. – pingul

+0

Nun, mein Code verwendet nirgendwo neu. Wenn Sie nicht möchten, dass "ChildT" ein Zeiger ist, dann machen Sie es in meinem Beispiel zu einem Wert wie "ValueT". –

+0

Oh, ich bin verwirrt wie das Zeug funktioniert. Was bedeutet 'new (& mValue) ValueT (std :: vorwärts (args) ...);' tun? – pingul

0

Nein, Sie können nicht memcpy etwas, was nicht trivial kopierbar ist - und ist sicherlich nicht.

Um auf das nicht-triviale Mitglied dieser Vereinigung zuzugreifen, müssen Sie zuerst einen Operator placement new aufrufen - andernfalls wird der Konstruktor nicht aufgerufen und bleibt nicht initialisiert.

Ich finde grundsätzlich die Verwendung von nicht-trivialen Typen in Gewerkschaften in der Regel eine zweifelhafte Praxis, aber nicht jeder stimmt mir zu.