15
\$\begingroup\$

This describes type util::any<std::size_t size, std::size_t align> - an implementation of std::any with a static size/no dynamic allocation. It closely follows the standard's specification for std::any.

A discriminated type that can contain values of different types but does not attempt conversion between them, i.e. 5 is held strictly as an int and is not implicitly convertible either to "5" or to 5.0. This indifference to interpretation but awareness of type effectively allows safe, generic containers of single values, with no scope for surprises from ambiguous conversions.

[Taken from the standard's description of std::any]

Review goals

  • Correctness: The lifetime of objects is managed manually. Are there any invalid states in my code? Can invalid states be generated through usage?

  • Optimization: There is a small size overhead required in order to enable type-erasure. Is there a way to make this code faster or require less memory without decreasing performance?

  • Design: Any design suggestions/comments are welcome. Most constraints are done through static_assert, this generates an informative error message, but does not highlight the actual error location; would it be better to simply remove functions through std::enable_if?

Usage

Instances of util::any<std::size_t, std::size_t> can hold any type that fits in the specified amount of static memory it has been created with. Access of the contained value is done through the static_any_cast<T> free functions.

Example

#include <cassert>
#include "static_any.h"

int main()
{
    util::static_any<4, 4> aa; // holds any type with a size <= 4, aligned as 4
    assert( aa.empty() );

    aa = 'A'; // holds a char
    aa = "abc"; // now holds char const[4];
    //aa = "abcd"; // error: char const[5] is too big.

    aa = 3.0f; // now holds a float
    try
    {
        util::static_any_cast<int>( aa ); // invalid cast; aa holds a float
    }
    catch ( util::bad_static_any_cast const& )
    {}
    assert( util::static_any_cast<float const&>( aa ) == 3.0f ); // view through const-ref

    util::static_any_cast<float&>( aa ) = 5.0f; // modify through reference cast
    assert( util::static_any_cast<float>( aa ) == 5.0f );
}

Implementation

All relevant parts of the code are commented. This is all in one file, but I've separated into sections for ease of review.

#ifndef UTIL_STATIC_ANY_H
#define UTIL_STATIC_ANY_H

#include <new>
#include <typeinfo>
#include <type_traits>

Implementation - class bad_static_any_cast

namespace util
{
    class bad_static_any_cast : public std::bad_cast
    {
        virtual char const* what() const noexcept override
        {
            return "bad_static_any_cast: casting to invalid type";
        }
    };
}

Implementation - class static_any

namespace util
{
    template <std::size_t size, std::size_t align = 8>
    class static_any;

    template <class StaticAny>
    struct is_static_any : std::false_type {};

    template <std::size_t size, std::size_t align>
    struct is_static_any<static_any<size, align>> : std::true_type {};

    template <std::size_t size, std::size_t align>
    class static_any
    {
    public:
        // [ destructor ]
        ~static_any()
        {
            if ( m_has_value )
            {
                storage_base_ptr()->destroy_contained_value();
            }
        }

        // [ constructors ]
        /**
        * effects: constructs an empty static_any.
        * postcondition: *this does not have a contained value.
        */
        static_any() noexcept
            : m_has_value{ false }
        {
        }

        /**
        * effects: constructs a static_any from another static_any by copying its
                   contained value.
        * requirements: the argument static_any must be smaller or equal in size.
        * throws: any exception thrown by the copy constructor of the contained value.
        */
        template <std::size_t other_size, std::size_t other_align>
        static_any( static_any<other_size, other_align> const& other )
            : m_has_value{ other.m_has_value }
        {
            static_assert( other_size <= size,
                "static_any error: cannot be constructed from a larger static_any" );

            if ( m_has_value )
            {
                other.storage_base_ptr()->copy_to( &m_memory );
            }
        }

        /**
        * effects: constructs a static_any from another static_any by moving its
                   contained value.
        * requires: the argument static_any must be smaller or equal in size.
        * throws: any exception thrown by the move constructor of the contained value.
        */
        template <std::size_t other_size, std::size_t other_align>
        static_any( static_any<other_size, other_align>&& other )
            : m_has_value{ other.m_has_value }
        {
            static_assert( other_size <= size,
                "static_any error: cannot be constructed from a larger static_any" );

            if ( m_has_value )
            {
                other.storage_base_ptr()->move_to( &m_memory );
                other.m_has_value = false;
            }
        }

        /**
        * effects: constructs a static_any containing a direct-initialized value of
                   type std::decay_t<T>.
        * requires: type std::decay_t<T> must be copy constructible.
        * throws: any exceptions thrown by the selected constructor of std::decay_t<T>.
        */
        template
        <
            class T,
            std::enable_if_t<!is_static_any<std::decay_t<T>>::value, int> = 0
        >
        static_any( T&& value )
            : m_has_value{ true }
        {
            static_assert( sizeof( T ) <= size,
                "incompatible type size; requires sizeof(T) <= size" );

            static_assert( std::is_copy_constructible<std::decay<T>>::value,
                "std::decay_t<T> must be copy constructible" );

            ::new ( &m_memory ) storage<std::decay_t<T>>( std::forward<T>( value ) );
        }

        // [ assignment ]
        /**
        * effects: static_any{rhs}.swap(*this);
                   no effects if an exception is thrown.
        * requires: the argument static_any must be smaller or equal in size.
        * returns: *this.
        * throws: any exception thrown by the copy constructor of the contained
                  value of the argument static_any.
        */
        template <std::size_t other_size, std::size_t other_align>
        static_any& operator=( static_any<other_size, other_align> const& rhs )
        {
            static_any{ rhs }.swap( *this );
            return *this;
        }

        /**
        * effects: static_any{std::move(rhs)}.swap(*this);
                   no effects if an exception is thrown.
        * requires: the argument static_any must be smaller or equal in size.
        * returns: *this.
        * throws: any exception thrown by the move constructor of the contained
                  value of the argument static_any.
        */
        template <std::size_t other_size, std::size_t other_align>
        static_any& operator=( static_any<other_size, other_align>&& rhs )
        {
            static_any{ std::move( rhs ) }.swap( *this );
            return *this;
        }

        /**
        * effects: static_any{std::forward<T>(rhs)}.swap(*this);
                   no effects if an exception is thrown.
        * requires: type std::decay_t<T> must be copy constructible.
        * returns: *this.
        * throws: any exception thrown by the selected constructor of std::decay_t<T>.
        */
        template
        <
            class T,
            std::enable_if_t<!is_static_any<std::decay_t<T>>::value, int> = 0
        >
        static_any& operator=( T&& value )
        {
            static_any{ std::forward<T>( value ) }.swap( *this );
            return *this;
        }

        // [ modifiers ]
        /**
        * effects: if *this has a contained value, destroy the contained value.
        * postcondition: *this does not have a contained value.
        */
        void clear()
        {
            if ( m_has_value )
            {
                storage_base_ptr()->destroy_contained_value();
                m_has_value = false;
            }
        }

        /**
        * effects: exchanges the contained values of *this and argument other.
        */
        void swap( static_any& other )
        {
            if ( this != &other )
            {
                if ( m_has_value && other.m_has_value )
                {
                    other.unsafe_swap( *this );
                }
                else if ( m_has_value )
                {
                    storage_base* storage_ptr{ storage_base_ptr() };

                    storage_ptr->move_to( &other.m_memory );
                    other.m_has_value = true;

                    storage_ptr->destroy_contained_value();
                    m_has_value = false;
                }
                else if ( other.m_has_value )
                {
                    storage_base* other_storage_ptr{ other.storage_base_ptr() };

                    other_storage_ptr->move_to( &m_memory );
                    m_has_value = true;

                    other_storage_ptr->destroy_contained_value();
                    other.m_has_value = false;
                }
            }
        }

        /**
        * effects: exchanges the contained values of *this and argument other without
                   checking for valid state (non-empty instances, self-assign, etc.).
        */
        template <std::size_t other_size, std::size_t other_align>
        void unsafe_swap( static_any<other_size, other_align>& other )
        {
            memory_t tmp;

            storage_base* storage_ptr{ storage_base_ptr() };
            storage_ptr->move_to( &tmp );
            storage_ptr->destroy_contained_value();

            storage_base* other_storage_ptr{ other.storage_base_ptr() };
            other_storage_ptr->move_to( &m_memory );
            other_storage_ptr->destroy_contained_value();

            reinterpret_cast<storage_base*>( &tmp )->move_to( &other.m_memory );
        }

        // [ observers ]
        /**
        * effects: indicates whether the static_any has a contained value or not.
        * returns: true if *this has no contained value, otherwise false.
        */
        bool empty() const noexcept
        {
            return !m_has_value;
        }

        /**
        * effects: accesses the std::type_info of the associated contained value.
        * returns: typeid(T) if *this has a contained value, otherwise typeid(void).
        */
        std::type_info const& type() const noexcept
        {
            static std::type_info const& void_type_info{ typeid( void ) };
            return m_has_value ? storage_base_ptr()->type() : void_type_info;
        }

    private:
        // [ friend declarations ]
        template <std::size_t other_size, std::size_t other_align>
        friend class static_any;

        template <class T, std::size_t size, std::size_t align>
        friend T static_any_cast( static_any<size, align>& operand );

        template <class T, std::size_t size, std::size_t align>
        friend T static_any_cast( static_any<size, align> const& operand );

        template <class T, std::size_t size, std::size_t align>
        friend T static_any_cast( static_any<size, align>&& operand );

        template <class T, std::size_t size, std::size_t align>
        friend T* static_any_cast( static_any<size, align>* operand ) noexcept;

        template <class T, std::size_t size, std::size_t align>
        friend T const* static_any_cast( static_any<size, align> const* operand ) noexcept;

        // [ helper types ]
        /**
        * base storage class
        */
        class storage_base
        {
        public:
            virtual ~storage_base() = default;

            storage_base( std::type_info const& type_info )
                : m_type_info{ type_info }
            {}

            virtual void copy_to( void* p_location ) const = 0;
            virtual void move_to( void* p_location ) = 0;
            virtual void destroy_contained_value() = 0;

            std::type_info const& type() const noexcept
            {
                return m_type_info;
            }

        private:
            std::type_info const& m_type_info;
        };

        /**
        * helper type used to determine the maximum size of *this' storage<T>
        */
        class storage_size_t : public storage_base
        {
            char filler[ size ];
        };

        /**
        * wrapper type to be created in static uninitialized memory
        * the contained value must be freed with a call to destroy_contained_value()
        */
        template <class T>
        class storage : public storage_base
        {
        public:
            template <class U>
            storage( U&& value )
                noexcept( std::is_nothrow_constructible<T, U&&>::value )
                : storage_base( typeid( T ) )
                , m_value( std::forward<U>( value ) )
            {
            }

            storage( storage const& ) = delete;
            storage( storage&& ) = delete;

            storage& operator=( storage const& ) = delete;
            storage& operator=( storage&& ) = delete;

            void copy_to( void* p_location ) const override
            {
                ::new ( p_location ) storage( m_value );
            }

            void move_to( void* p_location ) override
            {
                ::new ( p_location ) storage( std::move( m_value ) );
            }

            void destroy_contained_value() override
            {
                m_value.~T();
            }

            T m_value;

        private:
            ~storage() = default;
        };

        storage_base* storage_base_ptr()
        {
            return reinterpret_cast<storage_base*>( &m_memory );
        }

        storage_base const* storage_base_ptr() const
        {
            return reinterpret_cast<storage_base const*>( &m_memory );
        }

        using memory_t = std::aligned_storage_t<sizeof( storage_size_t ), align>;
        memory_t m_memory;
        bool m_has_value;
    };
}

Implementation - free functions

namespace util
{
    // [ non-member functions ]
    /**
    * effects: x.swap( y ).
    */
    template <std::size_t size, std::size_t align>
    void swap( static_any<size, align>& x, static_any<size, align>& y ) noexcept
    {
        x.swap( y );
    }

    /**
    * requires: std::is_reference<T>::value && std::is_copy_constructible<T>::value.
    * returns: *static_any_cast<std::remove_reference_t<T>>( &operand ).
    * throws: bad_static_any_cast if operand.type() != typeid( std::remove_reference_t<T> ).
    */
    template <class T, std::size_t size, std::size_t align>
    T static_any_cast( static_any<size, align>& operand )
    {
        static_assert(
            std::is_reference<T>::value || std::is_copy_constructible<T>::value,
            "any_cast requirement not met: T is a reference or copy constructible" );

        auto ret{ static_any_cast<std::remove_reference_t<T>>( &operand ) };
        if ( ret )
        {
            return *ret;
        }
        throw bad_static_any_cast{};
    }

    /**
    * requires: std::is_reference<T>::value && std::is_copy_constructible<T>::value.
    * returns: *static_any_cast<std::add_const_t<std::remove_reference_t<T>>>(&operand).
    * throws: bad_static_any_cast if operand.type() != typeid(std::remove_reference_t<T>).
    */
    template <class T, std::size_t size, std::size_t align>
    T static_any_cast( static_any<size, align> const& operand )
    {
        static_assert(
            std::is_reference<T>::value || std::is_copy_constructible<T>::value,
            "any_cast requirement not met: T is a reference or copy constructible" );

        auto ret
        {
            static_any_cast<std::add_const_t<std::remove_reference_t<T>>>( &operand )
        };

        if ( ret )
        {
            return *ret;
        }
        throw bad_static_any_cast{};
    }

    /**
    * requires: std::is_reference<T>::value && std::is_copy_constructible<T>::value.
    * returns: *static_any_cast<std::remove_reference_t<T>>(&operand).
    * throws: bad_static_any_cast if operand.type() != typeid(std::remove_reference_t<T>).
    */
    template <class T, std::size_t size, std::size_t align>
    T static_any_cast( static_any<size, align>&& operand )
    {
        static_assert(
            std::is_reference<T>::value || std::is_copy_constructible<T>::value,
            "any_cast requirement not met: T is a reference or copy constructible" );

        auto ret{ static_any_cast<std::remove_reference_t<T>>( &operand ) };
        if ( ret )
        {
            return *ret;
        }
        throw bad_static_any_cast{};
    }

    /**
    * returns: if operand != nullptr && operand->type() == typeid(T),
               a pointer to the object contained by operand, otherwise nullptr.
    */
    template <class T, std::size_t size, std::size_t align>
    T* static_any_cast( static_any<size, align>* operand ) noexcept
    {
        static std::type_info const& typeid_value = typeid( std::remove_reference_t<T> );

        using pointer = static_any<size, align>::template storage<T>*;

        return operand && operand->type() == typeid_value ?
            &static_cast<pointer>( operand->storage_base_ptr() )->m_value : nullptr;
    }

    /**
    * returns: if operand != nullptr && operand->type() == typeid(T),
               a pointer to const of the object contained by operand, otherwise nullptr.
    */
    template <class T, std::size_t size, std::size_t align>
    T const* static_any_cast( static_any<size, align> const* operand ) noexcept
    {
        static std::type_info const& typeid_value = typeid( std::remove_reference_t<T> );

        using pointer = static_any<size, align>::template storage<T> const*;

        return operand && operand->type() == typeid_value ?
            &static_cast<pointer>( operand->storage_base_ptr() )->m_value : nullptr;
    }
}
#endif // UTIL_STATIC_ANY_H
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

This isn't a thorough review, but I'll mention a few interesting points...


Your static_any is part of a noble tradition of in-place type-erased types. See also

Reading the code of these libraries will give you a lot of ideas for how to optimize the representation; for example, how to get rid of m_has_value. It may also help with the "SFINAE versus static_assert" question, which I don't address in this review but I agree that it's a good question to think about.


Your unsafe_swap function has a bug. Here's the code again:

    template <std::size_t other_size, std::size_t other_align>
    void unsafe_swap( static_any<other_size, other_align>& other )
    {
        memory_t tmp;

        storage_base* storage_ptr{ storage_base_ptr() };
        storage_ptr->move_to( &tmp );
        storage_ptr->destroy_contained_value();

        storage_base* other_storage_ptr{ other.storage_base_ptr() };
        other_storage_ptr->move_to( &m_memory );
        other_storage_ptr->destroy_contained_value();

        reinterpret_cast<storage_base*>( &tmp )->move_to( &other.m_memory );
    }

Notice that the calls to move_to and destroy_contained_value always come in pairs: first we do both on storage_ptr, then we do both on other_storage_ptr, then we do both on... oops! Red flag! We have forgotten to destroy the T object that was move-constructed into the memory of tmp on the first line. The missing destructor call could produce a memory leak, resource leak, or worse.

In an ideal world, the red flag of "faulty parallel construction" would have tipped you off to this bug. But I admit that I was particularly looking for this kind of bug because I'd already seen it happen in the wild, in the code of stdext::inplace_function. Type-erased move and swap are theoretically no harder than any other type-erased operation; yet empirical experience has shown me that in reality nobody gets them right on the first try.


As you might have guessed from that pull request, and from the observation that move and destroy must appear in pairs, IMHO it is good practice to combine them into a single operation, whose widely accepted name is relocate. However, you'll still need a type-erased destroy operation as well, to call in your actual destructor.


static std::type_info const& void_type_info{ typeid( void ) };
return m_has_value ? storage_base_ptr()->type() : void_type_info;

The above is so much more needlessly verbose than

return m_has_value ? storage_base_ptr()->type() : typeid(void);

Prefer the shorter version. (This applies to several places in your code.)


On the other hand, in one of your any_cast functions, you write:

auto ret{
    static_any_cast<std::add_const_t<std::remove_reference_t<T>>>( &operand )
};
if (ret) {
    return *ret;
}
throw bad_static_any_cast{};

(I've already taken the liberty of normalizing your indentation style. My editor window's vertical space is valuable; don't waste it!) Here, I think you've put the verbosity in the wrong place. I would have pulled out that confusing and ultimately "skim-over-able" type into a typedef, and collapsed all the value-space logic into a single line.

using U = std::add_const_t<std::remove_reference_t<T>>;
auto ret = static_any_cast<U>(&operand);
return ret ? *ret : throw bad_static_any_cast();

My vendetta against curly braces is purely personal preference; I wouldn't say you were wrong if you preferred auto ret{static_any_cast...} and throw bad_static_any_cast{}. I tend to use curlies only for code blocks and aggregate initializers; for things that are "intuitively" settings-equal or function-invocations, I personally prefer = or () respectively.


Speaking of static_any_cast and bad_static_any_cast: Do you think it's a good feature that these are spelled differently from the existing any_cast and bad_any_cast? I would naturally expect them to use the existing names, so that I could write

#ifdef FASTER
using any = util::static_any<32>;
#else
using any = std::any;
#endif
template<int> void any_cast();  // so dumb

and then

    any a = 42;
    printf("%d\n", any_cast<int>(a));

and it would Just Work. (The "so dumb" part is because C++ is stupid.)

So, IMO you should stop calling them static_any_cast and bad_static_any_cast and start calling them any_cast and bad_any_cast. In fact, I would strongly suggest that you should just use the standard std::bad_any_cast exception type instead of making up your own... at least, if you're in an environment where the standard one can be assumed (or detected) to exist.


Nit: Your default size_t Align = 8 could productively be replaced with size_t Align = alignof(std::aligned_storage_t<Size>). This prevents you from quietly overaligning the buffer of a static_any<4>.


I'm sure there's more, but this is enough for one review. :)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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