11
\$\begingroup\$

I am a C# game developer currently learning C++ and this is my second big-ish project (the first one being a vector implementation). This time I opted for implementing a mot-a-mot any implementation based on the specifications of the ISO/IEC N4830 standard draft.

My objective is to familiarize myself with the modern C++ techniques.

The code can be found on Github. Here is the link.

Thank you in advance for taking the time to read my code.

#pragma once
/*
    At any point, an <any> stores one of the following types:
        1. Big types
        2. Small types
        3. Trivial types

    In the 1st case, <any> will dynamically allocate memory on the heap for the object
    In the 2nd case, <any> will store the object inside any itself
    In the 3rd case, <any> will store the object inside any itself and no destructor shall be called for the object. Additionally, the copy and moves will be treated differently
*/
#include <utility>
#include <initializer_list>
#include <type_traits>

class bad_any_cast : public std::bad_cast
{
    const char* what() const noexcept override
    {
        return "bad_any_cast";
    }
};

constexpr size_t small_space_size = 8 * sizeof(void*);

template<class T>
using any_is_small = std::bool_constant<std::is_nothrow_move_constructible_v<T>
                                     && sizeof(T) <= small_space_size
                                     && alignof(T) <= alignof(void*)>; // Check if alignment shouldnt be % == 0

enum class any_representation
{
    Small,
    Big,
};

template<class T, class... Args>
void Construct(void* destination, Args&&... args) noexcept
{
    new(destination) T(std::forward<Args>(args)...);
}

struct any_big
{
    template <class T>
    static void Destroy(void* target) noexcept
    {
        std::destroy_at(static_cast<T*>(target));

        _aligned_free(target);
    }

    template<class T>
    static void* Copy(const void* source)
    {
        return new T(*static_cast<const T*>(source));
    }

    template<class T>
    static void* Type()
    {
        return (void*)&typeid(T);
    }

    void (*_destroy)(void*);
    void* (*_copy)(const void*);
    void* (*_type)();
};

struct any_small
{
    template <class T>
    static void Destroy(void* target)
    {
        if constexpr (!std::is_trivially_copyable_v<T>)
        {
            std::destroy_at(static_cast<T*>(target));
        }
    }

    template<class T>
    static void Copy(void* destination, const void* what)
    {
        if constexpr (std::is_trivially_copyable_v<T>)
        {
            *static_cast<T*>(destination) = *static_cast<const T*>(what);
        }
        else
        {
            Construct<T>(static_cast<T*>(destination), *static_cast<const T*>(what));
        }
    }

    template<class T>
    static void Move(void* destination, void* what) noexcept
    {
        if constexpr (std::is_trivially_copyable_v<T>)
        {
            *static_cast<T*>(destination) = *static_cast<T*>(what);
        }
        else
        {
            Construct<T>(static_cast<T*>(destination), std::move(*static_cast<T*>(what)));
        }
    }

    template<class T>
    static void* Type()
    {
        return (void*) & typeid(T);
    }

    void (*_destroy)(void*);
    void (*_copy)(void*, const void*);
    void (*_move)(void*, void*);
    void* (*_type)();
};

template<class T>
any_big any_big_obj = { &any_big::Destroy<T>, &any_big::Copy<T>, &any_big::Type<T> };

template<class T>
any_small any_small_obj = { &any_small::Destroy<T>, &any_small::Copy<T>, &any_small::Move<T>, &any_small::Type<T> };

class any
{
public:
    constexpr any() noexcept
        :_storage{},
        _representation{}
    {
    }

    any(const any& other) noexcept
        :_storage{},
        _representation{}
    {
        if (!other.has_value())
        {
            return;
        }

        switch (other._representation)
        {
        case any_representation::Big:
            _storage.big_storage.handler = other._storage.big_storage.handler;
            _storage.big_storage.storage = other._storage.big_storage.handler->_copy(other._storage.big_storage.storage);
            _representation = any_representation::Big;
            break;
        case any_representation::Small:
            _storage.small_storage.handler = other._storage.small_storage.handler;
            other._storage.small_storage.handler->_copy(&_storage.small_storage.storage, &other._storage.small_storage.storage);
            _representation = any_representation::Small;
            break;
        }
    }

    any(any&& other) noexcept
        :_storage{},
        _representation{}
    {
        if (!other.has_value())
        {
            return;
        }

        switch (other._representation)
        {
        case any_representation::Big:
            _storage.big_storage.handler = other._storage.big_storage.handler;
            _storage.big_storage.storage = other._storage.big_storage.storage;
            _representation = any_representation::Big;
            break;
        case any_representation::Small:
            _storage.small_storage.handler = other._storage.small_storage.handler;
            other._storage.small_storage.handler->_move(&_storage.small_storage.storage, &other._storage.small_storage.storage);
            _representation = any_representation::Small;
            break;
        }
    }

    template<class T, typename VT = std::decay_t<T>, typename = std::enable_if_t<!std::is_same_v<VT, any> // can use conjunction and negation for short circuit but it's too hard too 
                                                                               && std::is_copy_constructible_v<VT>>> // check if VT is a specialization of in_place_type_t
    any(T&& value)
    {
        emplace<VT>(std::forward<T>(value));
    }

    template<class T, class... Args, typename VT = std::decay_t<T>, typename = std::enable_if<std::is_copy_constructible_v<VT>
                                                                                           && std::is_constructible_v<VT, Args...>>>
    explicit any(std::in_place_type_t<T>, Args&&... args)
    {
        emplace<VT>(std::forward<T>(args)...);
    }

    template<class T, class U, class...Args, typename VT = std::decay_t<T>, typename = std::enable_if_t<std::is_copy_constructible_v<VT> && 
                                                                                                        std::is_constructible_v<VT, std::initializer_list<U>&, Args...>>>
    explicit any(std::in_place_type_t<T>, std::initializer_list<U> il, Args&&... args)
    {
        emplace<VT>(il, std::forward<Args>(args)...);
    }

    ~any()
    {
        reset();
    }

    any& operator=(const any& rhs)
    {
        any(rhs).swap(*this);

        return *this;
    }

    any& operator=(any&& rhs) noexcept
    {
        any(std::move(rhs)).swap(*this);
        return *this;
    }

    template<class T, typename VT = std::decay_t<T>, typename = std::enable_if_t<!std::is_same_v<VT, any>
                                                                                && std::is_copy_constructible_v<VT>>>
    any& operator=(T&& rhs)
    {
        any tmp(rhs);

        tmp.swap(*this);

        return *this;
    }

    template<class T, typename VT = std::decay_t<T>, class... Args, typename = std::enable_if_t<std::is_copy_constructible_v<VT>
                                                                                             && std::is_constructible_v<VT, Args...>>>
    std::decay_t<T>& emplace(Args&&... args)
    {
        reset();

        return emplace_impl<VT>(any_is_small<T>{}, std::forward<Args>(args)...);
    }

    template<class T, class U, typename VT = std::decay_t<T>, class... Args, typename = std::enable_if_t<std::is_copy_constructible_v<VT>
                                                                                                      && std::is_constructible_v<VT,std::initializer_list<U>&, Args...>>>
    std::decay_t<T>& emplace(std::initializer_list<U> il, Args&&... args)
    {
        reset();

        return emplace_impl<VT>(any_is_small<T>{}, il, std::forward<Args>(args)...);
    }

    void reset() noexcept
    {
        if (!has_value())
        {
            return;
        }

        switch (_representation)
        {
        case any_representation::Big:
            _storage.big_storage.handler->_destroy(_storage.big_storage.storage);
            _storage.big_storage.handler = nullptr;
            break;
        case any_representation::Small:
            _storage.small_storage.handler->_destroy(&_storage.small_storage.storage);
            _storage.small_storage.handler = nullptr;
            break;
        }
    }

    void swap(any& rhs) noexcept
    {
        any tmp;
        tmp._storage = rhs._storage;
        tmp._representation = rhs._representation;

        rhs._storage = _storage;
        rhs._representation = _representation;

        _storage = tmp._storage;
        _representation = tmp._representation;
    }

    bool has_value() const noexcept
    {
        switch (_representation)
        {
        case any_representation::Big:
            return _storage.big_storage.handler != nullptr;
        case any_representation::Small:
            return _storage.small_storage.handler != nullptr;
        default:
            return false;
        }
    }
    const std::type_info& type() const noexcept
    {
        if (has_value())
        {
            switch (_representation)
            {
                case any_representation::Big:
                    return *static_cast<const std::type_info*>(_storage.big_storage.handler->_type());
                case any_representation::Small:
                    return *static_cast<const std::type_info*>(_storage.small_storage.handler->_type());
            }
        }
        else
        {
            return typeid(void);
        }
    }

    template<class T>
    T* get_val() noexcept
    {
        return static_cast<T*>(get_val_impl(any_is_small<T>{}));
    }

private:
    template<class T, class... Args>
    std::decay_t<T>& emplace_impl(std::true_type, Args&&... args) // any_is_trivial, any_is_small
    {
        // small any
        _storage.small_storage.handler = &any_small_obj<T>;
        Construct<T>(static_cast<void*>(&_storage.small_storage.storage), std::forward<Args>(args)...);
        _representation = any_representation::Small;
        return reinterpret_cast<T&>(_storage.small_storage.storage);
    }

    template<class T, class... Args>
    std::decay_t<T>& emplace_impl(std::false_type, Args&&... args) // any_is_trivial, any_is_small
    {
        // big any
        _storage.big_storage.handler = &any_big_obj<T>;
        _storage.big_storage.storage = _aligned_malloc(sizeof(T), alignof(T));
        Construct<T>(_storage.big_storage.storage, std::forward<Args>(args)...);
        _representation = any_representation::Big;
        return reinterpret_cast<T&>(_storage.big_storage.storage);
    }

    void* get_val_impl(std::true_type) noexcept
    {
        return (static_cast<void*>(&_storage.small_storage.storage));
    }

    void* get_val_impl(std::false_type) noexcept
    {
        return (_storage.big_storage.storage);
    }

    struct big_storage_t
    {
        void* storage;
        any_big* handler;
    };

    struct small_storage_t
    {
        typedef std::aligned_storage_t<small_space_size, std::alignment_of_v<void*>> internal_storage_t;
        internal_storage_t storage;
        any_small* handler;
    };

    struct storage
    {
        union
        {
            small_storage_t small_storage;
            big_storage_t big_storage;
        };

        std::type_info* _typeInfo;
    };

    storage _storage;
    any_representation _representation;
};

inline void swap(any& x, any& y) noexcept 
{
    x.swap(y);
}

template<class T, class... Args>
any make_any(Args&&... args)
{
    return any{std::in_place_type<T>, std::forward<Args>(args)...};
}

template<class T, class U, class... Args>
any make_any(std::initializer_list<U> il, Args&&... args)
{
    return make_any(il, std::forward<Args>(args));
}

template<class T>
T any_cast(const any& operand)
{
    static_assert(std::is_constructible_v<T, const std::remove_cv_t<std::remove_reference_t<T>>&>);

    const auto storagePtr = any_cast<std::remove_cv_t<std::remove_reference_t<T>>>(&operand);

    if (!storagePtr)
    {
        throw bad_any_cast({});
    }

    return static_cast<T>(*storagePtr);
}

template<class T>
T any_cast(any& operand)
{
    static_assert(std::is_constructible_v<T, std::remove_cv_t<std::remove_reference_t<T>>&>);

    const auto storagePtr = any_cast<std::remove_cv_t<std::remove_reference_t<T>>>(&operand);

    if (!storagePtr)
    {
        throw bad_any_cast({});
    }

    return static_cast<T>(*storagePtr);
}

template<class T>
T any_cast(any&& operand)
{
    static_assert(std::is_constructible_v<T, std::remove_cv_t<std::remove_reference_t<T>>>);

    const auto storagePtr = any_cast<std::remove_cv_t<std::remove_reference_t<T>>>(&operand);

    if (!storagePtr)
    {
        throw bad_any_cast({});
    }

    return static_cast<T>(std::move(*storagePtr));
}

template<class T>
const T* any_cast(const any* operand) noexcept
{
    if (operand != nullptr && operand->type() == typeid(T))
    {
        return /*const_cast*/ operand->get_val<T>();
    }

    return nullptr;
}

template<class T>
T* any_cast(/*const*/ any* operand) noexcept
{
    if (operand != nullptr && operand->type() == typeid(T))
    {
        return /*const_cast*/ operand->get_val<T>();
    }

    return nullptr;
}

Edit: A more up-to-date version of this implenetantion can be found here

\$\endgroup\$
6
\$\begingroup\$

You've got some issues with your perfect forwarding and move semantics. You wrote:

template<class T, typename VT = std::decay_t<T>,
         typename = std::enable_if_t<!std::is_same_v<VT, any>
                                   && std::is_copy_constructible_v<VT>>>
any& operator=(T&& rhs)
{
    any tmp(rhs);
    tmp.swap(*this);
    return *this;
}

Here T&& rhs is a forwarding reference, so you ought to forward it:

    any tmp(std::forward<T>(rhs));

And again:

template<class T, typename VT = std::decay_t<T>, class... Args,
         typename = std::enable_if_t<std::is_copy_constructible_v<VT>
                                  && std::is_constructible_v<VT, Args...>>>
std::decay_t<T>& emplace(Args&&... args)
{
    reset();
    return emplace_impl<VT>(any_is_small<T>{}, std::forward<Args>(args)...);
}

Here you are correctly forwarding Args&&... args; but look closely at your enable_if condition! You're asking whether VT is constructible from Args.... But when you actually pass args... to emplace_impl<VT>, you end up trying to construct VT from Args&&...! So you should be testing

         class = std::enable_if_t<std::is_copy_constructible_v<VT>
                               && std::is_constructible_v<VT, Args&&...>>>

instead. (And notice that I personally prefer template<class> over template<typename> — it means the same thing, it's just less to write and less to read.)


Since enum class any_representation is part of your struct layout and thus part of your ABI, you should add an explicit underlying type so that you know how many bytes it'll take up in your struct.

enum class any_representation : unsigned char { Small, Big };

template<class T, class... Args>
void Construct(void* destination, Args&&... args) noexcept
{
    new(destination) T(std::forward<Args>(args)...);
}

noexcept? Really? What if it throws? This is a good argument to never put noexcept anywhere except on move-constructors (to avoid the vector pessimization) and perhaps swap. Certainly you should be very careful every time you apply it; don't just scatter it willy-nilly like constexpr!


template<class T>
any_big any_big_obj = { &any_big::Destroy<T>, &any_big::Copy<T>, &any_big::Type<T> };

template<class T>
any_small any_small_obj = { &any_small::Destroy<T>, &any_small::Copy<T>, &any_small::Move<T>, &any_small::Type<T> };

Why do you have two different layouts for these structs (any_small has 4 function pointers where any_big has only 3)? The whole point of any is that you're type-erasing everything about the original VT except for its affordances. If "small" VTs require different affordances than "big" VTs, you're probably doing something incorrect somewhere.


std::type_info* _typeInfo;

This field is completely unused, I think. Why is it here? (More importantly, how could you improve your workflow to avoid submitting code reviews for dead code?)


typedef std::aligned_storage_t<small_space_size, std::alignment_of_v<void*>> internal_storage_t;
internal_storage_t storage;

I strongly recommend against using std::aligned_storage_t in C++11-and-later; its behavior is not portable from one library to the next. Instead, you can use C++11's built-in support for alignment:

alignas(void*) char storage[small_space_size];

inline void swap(any& x, any& y) noexcept 
{
    x.swap(y);
}

Prefer to use the Hidden Friend idiom here: make this function findable only via ADL.


void swap(any& rhs) noexcept
{
    any tmp;
    tmp._storage = rhs._storage;
    tmp._representation = rhs._representation;

    rhs._storage = _storage;
    rhs._representation = _representation;

    _storage = tmp._storage;
    _representation = tmp._representation;
}

First of all, if this code were correct, you should just be using std::swap(_storage, rhs._storage); std::swap(_representation, rhs._representation);, not writing out the three-step dance by hand.

However, this code is utterly wrong. Try it (on libstdc++ or libc++) with

any a1 = std::list<int>{1,2,3};
any a2 = std::list<int>{4,5,6};
a1.swap(a2);
std::list<int> r = any_cast<const std::list<int>&>(a1);

You can't just memswap the bytes of two std::lists and expect them to come through unscathed; std::list is not trivially relocatable. To move a1's contents over into a2, and vice versa, you must at some point invoke std::list's move-constructor. Your code never invokes the move-constructor, so I know it's wrong.

Godbolt seems to show that you've got problems elsewhere in the code, too. I haven't tried to track them down.

This goes to show: you should also write unit tests for your code, and test it, before submitting it for review!


return make_any(il, std::forward<Args>(args));

This line gives an error on Clang, because you forgot the ... after (args).

\$\endgroup\$
  • \$\begingroup\$ is_constructible_v<T, Args...> is identical to is_constructible_v<T, Args&&...> because is_constructible is defined in terms of declval. Unless you are talking about incomplete types ... \$\endgroup\$ – L. F. Oct 8 '19 at 10:23
  • \$\begingroup\$ Thank you for the review. I'm not sure about the difference between std::is_constructible_v<VT, Args&&...>> and std::is_constructible_v<VT, Args...>> though. I know that Construct should not have been marked as noexcept. My cppcore code analysis kept screaming I should mark it as noexcept for some reason and I did but it was the wrong call. any_big doesn't have a move method because I work with pointers so in my mind I can just swap/assign the pointers that point to the allocated object on the heap. \$\endgroup\$ – Marius T Oct 8 '19 at 11:41
  • \$\begingroup\$ Interestingly enough, if I do change the swap method to work with std::swap(storage...) and and std::swap(representation...) your example works fine. You were right to say I shouldn't "three-step dance by hand". I don't see why I should (move) construct the objects in a swap operation though. I do have unit tests for my code but perhaps these are not enough: github.com/TheSica/any/blob/master/any/TestAny.cpp I will work on fixing the errors/warnings that I get from different compilers. Thank you again for your review! \$\endgroup\$ – Marius T Oct 8 '19 at 11:44
  • \$\begingroup\$ @MariusT: Those look like a good start on tests! However, indeed, I don't see any tests of swap for non-trivially relocatable types. If you're on GCC/libstdc++, then even std::string (for a small string) would have exposed the problem with swap. \$\endgroup\$ – Quuxplusone Oct 8 '19 at 14:59
  • \$\begingroup\$ "any_big doesn't have a move method because I work with pointers so in my mind I can just swap/assign the pointers" — Right, what it means to "move" an any_big is pretty trivial. But you still have code somewhere that describes what you do to a storage when you need to "move" it, in the any_big case. You just chose to write that code inline rather than attach it to void (*move)(void*, void*). If you attach it to void (*move)(void*, void*), then your big_handler and small_handler get the same layout, so you're one step closer to removing that switch from your move-constructor. \$\endgroup\$ – Quuxplusone Oct 8 '19 at 15:04
2
\$\begingroup\$
  1. Regarding your typeid, you have to include the header <typeinfo> before being allowed to use it.
  2. In your any_big and any_small: Why does the function static void* Type<T>() return a void*? You could return a const std::type_info& instead
  3. Your any_big_obj and any_small_obj are variables defined in the header-file. If you include your any.h in two cpp-files and instantiate the variable templates with the same type, you will get a linker error. Two avoid that, mark the variables as either const or inline (or both, which I suggest)

    inline const any_big any_big_obj = ...
    
  4. In any_small::Destroy: The if constexpr check should be !std::is_trivially_destructible_v<T>

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for?Browse other questions tagged or ask your own question.