How to write operator= for anonymous union with non-trivial members with virtual methods
Asked Answered
R

1

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.

Ruttish answered 15/10, 2018 at 16:13 Comment(5)
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.Glary
@Glary 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.Ruttish
Yes there was indeed language that specified the behavior is disallowed. See 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.Glary
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.Mongolism
@IgorTandetnik fair enough. So there's no other way except do a placement new to construct the object?Ruttish
R
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> :)

Ruttish answered 18/10, 2018 at 10:50 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.