2016-06-28 3 views
2

私は次のようなデータ構造を変換しようとしている:C++共用体をコピーするにはどうすればいいですか?

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

ValueTは、それが最初のように実装された理由である、両方のPOD(intfloat、など)とVec3などの非自明なもの、std::stringすることができ動的に割り当てられたメモリへのポインタ。しかし、C++ 11では、クラスに直接値を格納することができます。私は、このさを探していた結果:

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

これを変更するには、コンパイラがコピーコンストラクタが欠落していると文句を言うので、私も

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

、理想的には移動コンストラクタを実装することができます。これまでコンパイラは私のためにこれらを実装していました。 PODを使って、私はmemcpyか何か似たようなことをすることができます - 私は今、同じものを使用して正しい結果を期待できますか?

+1

いいえ、一般的に、あなたは 'memcpy'を使うことはできません。これは正確に 'std :: string'のようなクラスを「自明ではない」ものにします(正確であるためには簡単にコピーすることはできません)。どの組合員が現在アクティブであるかを知るには何らかの方法が必要で、それに応じてコピーコンストラクタを書く必要があります。代わりに、['boost :: variant'](http://www.boost.org/doc/libs/1_61_0/doc/html/variant.html)のようなものを使用してください。 –

+0

' ValueT'はコピー&移動も必要ですコンストラクタ/代入などがあります。これは、 'MyUnion'がいつでもコピー&構築することができるので、これは非常に簡単になります。 PODでも初期化コピーと 'std :: move'と一緒に働くafaik –

+0

@XerenNarcyコピーするメンバーを知っているように、メンバーをコピーする方法はあまり問題ではありません。 –

答えて

2

まず、mValueが動的に割り当てられたメモリへのポインタだった場合、このクラスのデフォルトのコピーコンストラクタは非常に安全ではありませんでした。

オブジェクトを削除するのはどのコピーが原因ですか?両方とも同じに見え、共有ポインタはありません。だから私はあなたがそれを漏らしたと仮定します。 (あなたはおそらくマネージャークラスをいくつか持っていたかもしれませんが、あなたはそれを組合で価値あるものにする方法を尋ねることはありませんでした)p

ほとんどの場合、必要なのは、どのメンバーが現在初期化されているかを示す追加のフラグを格納することです。

ValueTであると仮定して、コピー可能かつ移動可能な最小限のバージョンを提供します。これは、「区別されたユニオン」と呼ばれます。

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; 
    } 
}; 

注:通常、このような識別されたユニオンをロールする必要はありません。多くの場合、boost::variantのような既存のライブラリや、コメントに記載されているexpectedのいずれかのタイプのライブラリを使用する方がよいでしょう。しかし、このようなあなた自身の小さな判別組合を作ることは

  • ないというハード
  • 良い運動です
  • 時にはそれがAPIの境界か何か

に表示する必要がある場合は良いアイデア組合を使用する多くのケースでは、不要な最適化が行われており、structであれば問題ありません。オブジェクトを表現するためにより多くのメモリが必要になりますが、それは重要ではなく、チームのメンテナンスが容易/理解しやすくなります。

+0

私は変更したいコードの原則部分のみを示しました。メモリリークはありませんでしたので、心配はありませんでした。実装はあなたの提供するものと似ていましたが、いくつかの罠を盗むかもしれません。しかし、あなたは最も重要な部分を見逃してしまったように感じます。目標は、 'new'でヒープメモリを割り当てる必要がないことでした。それ以外は私はあなたのソリューションに完全に同意します。 – pingul

+0

まあ、私のコードは新しい場所を使用しません。 'ChildT'をポインタにしたくないのであれば、私の例では' ValueT'のような値型にしてください。 –

+0

ああ、私はものがどのように動作するのか混乱する。 'new(&mValue)ValueT(std :: forward (args)...);とは何ですか?' do? – pingul

0

いいえ、あなたはmemcpyとはっきりとコピーできないものはありませんか?std::stringはもちろんありません。

さらに、このユニオンの自明でないメンバーにアクセスするには、最初にプレースメントnew演算子を呼び出す必要があります。そうでなければ、コンストラクタは呼び出されず、未初期化のままです。

私は基本的に、組合の中では些細なものではない使い方を見ていますが、誰もが私に同意するわけではありません。

関連する問題