I am trying to create a simple variant as a learning exercise.
I want to do this without dynamically allocating memory, as is specified by the c++ specification for std::variant.
To simplify things, my variant can only take two values.
Here is my implementation:
//variant style class for two types
template<typename T1, typename T2>
class Either {
using Bigest = std::conditional<sizeof(T1) >= sizeof(T2), T1, T2>;
using ByteArray = std::array<std::byte, sizeof(Bigest)>;
ByteArray val;
std::optional<std::type_index> containedType;
public:
Either() : containedType(std::nullopt) {}
template<typename T>
Either(const T& actualVal) : containedType(typeid(T)) { //ToDo check T is one of correct types
ByteArray* ptr = (ByteArray*)&actualVal;
val = *ptr;
}
class BadVariantAccess {};
template<typename T>
inline T& getAs() const {
if(containedType == typeid(T)) {
T* ptr = (T*)val.data();
return *ptr;
}
else throw BadVariantAccess();
}
};
However, when I test this I get an incorrect number after trying to get the value:
int main() {
Either<int,float> e = 5;
std::cout << e.getAs<int>() << std::endl;
return 0;
}
Returns a random number (e.g 272469509).
What is the problem with my implementation, and how can I fix it?
Your program exhibits Undefined Behavior for various reasons, so it's a bit meaningless to attempt to explain why you observe the given behaviour. But here are some serious issues with your code:
It's not safe to alias any memory as std::array<std::byte, N>. This is a violation of the Strict Aliasing Rule, the only exceptions to which are pointers to char and std::byte. When doing ByteArray* ptr = (ByteArray*)&actualVal; val = *ptr;, you are invoking std::array's copy constructor and passing an instance that does not exist. At this point, literally anything could happen. You should instead be either copying bytes one-by-one (for trivial types), or using placement new to copy-construct an object in your byte-based storage.
Your storage is not aligned, and this can cause crashes or serious performance penalties at runtime, depending on your target platform. I'm not immediately sure whether this can cause Undefined Behaviour but you should certainly address this if you want to continue with such low-level memory management.
Your copy constructor does not check the actual size in memory of the type being assigned. If, on some platform, you have sizeof(int) > sizeof(float), then when you copy-construct an Either<int, float> from a float, you will read bytes past the end of the float and easily cause Undefined Behavior. You must take the size of the type being assigned into account
If you plan to store anything but trivial types (i.e. std::string or std::vector, not just primitives), you'll need to make calls to the appropriate copy/move constructor/assignment operators. For constructors (default, move, copy), you'll need to use placement new to construct live objects in the pre-allocated storage. Furthermore, you'll need to use type-erasure to store some function that will destroy the contained object as the correct type. A lambda inside a std::function<void(std::byte*)> could be very useful here, which simply calls the destructor: [](std::byte* data){ (*reinterpret_cast<T*>(data)).~T(); } This would have to be assigned anytime you store a new type. Note that this sort of situation is just about the only time you would ever manually want to call a destructor.
I strongly urge you to do some careful reading about how to do such low-level memory management correctly. It's really easy to do incorrectly and not have any idea until you get plagued with bizarre bugs much later.
As was kindly pointed out by @MilesBudnek, using Biggest = std::conditional<sizeof(T1) >= sizeof(T2), T1, T2> will give the type trait, and so an instance of Biggest will actually be an instance of a specialization of the struct std::conditional and not T1 or T2. You probably meant std::conditional_t<...> or std::conditional<...>::type; This likely has the effect that your Either class will only ever allocate a single byte, which is obviously incorrect.