9

C++11 gaves us the ability to create anonymous unions with non-trivial members. That can be very useful sometimes - for example, if I want to create Holder class for some non-trivial object without default ctor.
Let's make this NonTrivial object more interesting by giving it a virtual method:

#include <stdint.h>
#include <stdio.h>

struct Base
{
    virtual void something() { printf("something\n"); }
};

struct NonTrivial : Base 
{
    explicit NonTrivial( int ) : a(1), b(2), c(3), d(4) { printf("NonTrivial\n"); }

    virtual void something() override { printf("something non trivial\n"); }

    int a;
    int b;
    int c;
    int d;
};

struct Holder
{
    Holder() : isNonTrivial(false), dummy(0x77) {}

    Holder( NonTrivial n) : isNonTrivial(true), nonTrivial( n ) {}

    bool isNonTrivial;
    union
    {
        int dummy;
        NonTrivial nonTrivial;
    };

    Holder & operator=( const Holder & rhs )
    {
        isNonTrivial = rhs.isNonTrivial;

        if( isNonTrivial ) 
            nonTrivial = rhs.nonTrivial;

        return *this;
    }
};

int main() {

    Holder holder_1;
    NonTrivial n(1);

    Holder holder_2( n );

    holder_1 = holder_2;

    holder_2.nonTrivial.something();
    holder_1.nonTrivial.something();

    return 0;

}

This just works. However, this works only because compiler doesn't actually make a virtual call here. Let's force it:

Base * ptr = &holder_1.nonTrivial;

ptr->something(); 

This produces a segfault.
But why? I did more or less the obvious thing - checked if holder holds a non-trivial object and if so - copied it.
After reading the assembly I saw that this operator= doesn't actually copy vtable pointer from rhs.nonTrivial. I assume that this happens because operator= for NonTrivial should only be called on fully-constructed object and fully-constructed object should already have its vtable pointer initialized - so why bother and copy it?

Questions:

  1. Is my thinking correct?
  2. How should operator= look like to create a full copy of nonTrivial object? I have two ideas - delete operator= entirely and force the user to use copy ctor - or use placement new instead of nonTrivial = rhs.nonTrivial - but maybe there are some other options?

P.S. I'm aware of std::optional and the like, I'm trying to understand how to do it myself.

Amomum
  • 6,217
  • 8
  • 34
  • 62
  • It would be unreasonable for `operator=` to unconditionally copy a vptr b/c the arguments vptr is not necessarily the one belonging to that class. A branch to check if the vptr is initialized correctly seems unfortunate in the general case. Unless the standard specifies that this behavior is disallowed... I would suspect that this is a bug since vtables aren't specified anywhere anyways. – Lawrence Oct 15 '18 at 22:13
  • @Lawrence I tested this on two compilers with similar results, so I don't think that's a bug. I suppose that since Holder holds NonTrivial by value and not by pointer, it must be the exact type, so vtables must be the same. – Amomum Oct 15 '18 at 22:31
  • Yes there was indeed language that specified the behavior is disallowed. See https://en.cppreference.com/w/cpp/language/union: ` If members of a union are classes with user-defined constructors and destructors, to switch the active member, explicit destructor and placement new are generally needed`. Similar language can be found in the standard I believe. – Lawrence Oct 15 '18 at 23:23
  • 1
    You call a member function (namely, `operator=()`) on an object that was never constructed, whose lifetime has never started. That's undefined behavior, of course. – Igor Tandetnik Oct 16 '18 at 01:10
  • @IgorTandetnik fair enough. So there's no other way except do a placement new to construct the object? – Amomum Oct 16 '18 at 09:00

1 Answers1

1

If anybody will stumble upon this question looking for a quick answer, here's how I solved this issue using placement new:

template< typename T, typename ... Args >
void inplace_new( T & obj, Args && ... args )
{
    auto * t = &obj;

    t = new(t) T{ args... };
}

Holder & operator=( const Holder & rhs )
{
    isNonTrivial = rhs.isNonTrivial;

    if( isNonTrivial ) 
        inplace_new( nonTrivial, rhs.nonTrivial );

    return *this;
}

Don't forget #include <new> :)

Amomum
  • 6,217
  • 8
  • 34
  • 62