Wrapping types with alignment requirements

Question

Visual Studio 2013 still doesn't support the alignas keyword in C++11. This causes some problems with alignment of types in various situations. Thankfully the compiler will generate an error when it can't guarantee the alignment or at least give a warning.

To solve these errors and warnings I'm implementing a wrapper type that will wrap another type with some alignment restriction regardless of how the wrapper is aligned. Consider two wrapper objects A and B that themselves have arbitrary alignment. Let the wrapped datum be a and b respectively. Then A=B will assign a=b while still keeping a aligned regardless of how B was aligned because b in itself is aligned.

I'm looking for a review on the code on the account of syntax and possible weaknesses in the design and implementation.

template< typename T, std::size_t Align = 16>
class aligned_wrapper{
public:
    typedef T value_type;

    template<typename... Args>
    aligned_wrapper(Args&&... args){
        std::size_t space = sizeof(m_storage);
        auto raw_ptr = reinterpret_cast<void*>(&m_storage);
        auto ptr = std::align(Align, sizeof(T), raw_ptr, space);
        new (ptr)T(std::forward<Args>(args)...);
        m_object = reinterpret_cast<T*>(ptr);
    }

    ~aligned_wrapper(){
        get().~T();
    }

    aligned_wrapper& operator = (const aligned_wrapper& that){
        get() = that.get();
        return *this;
    }

    template<typename U>
    aligned_wrapper(const aligned_wrapper<U>& that)
        : aligned_wrapper(that.get())
    {}

    template<typename U>
    aligned_wrapper(aligned_wrapper<U>&& that)
        : aligned_wrapper(std::move(that.get()))
    {}

    inline T& get(){ return *m_object; }
    inline const T& get() const { return *m_object; }

    void swap(aligned_wrapper& that){
        using std::swap;
        swap(get(), that.get());
    }

    friend void swap(aligned_wrapper& lhs, aligned_wrapper& rhs){
        lhs.swap(rhs);
    }
private:
    std::aligned_storage_t<sizeof(T) + Align - 1, 1> m_storage;
    T* m_object{ nullptr };
};

Although it may look a bit funky, one of the key ideas is that once the aligned_wrapper is constructed, m_object will never change for that instance as the address of this will not change, this guarantees alignment. In other words, not assigning m_object in the assignment operator is intentional.

Answer 1

1. The class is missing a copy constructor and a move constructor. These can never be created by instantiating a template. Right now, the compiler will generate a defaulted copy constructor for you (and no move constructor, because you have a user-defined destructor). Since you obviously don't want that, you have to define the copy (and probably move as well) constructor yourself.

2. You're also missing a move assignment operator which you should probably add. Otherwise, your class wouldn't be fully usable with move-only types.

3. Your code is missing comments (there aren't any). You even had to explain an invariant in the question:

   ... once the aligned_wrapper is constructed, m_object will never change for that instance as the address of this will not change, this guarantees alignment. In other words, not assigning m_object in the assignment operator is intentional.

   This type of information must go into comments. The next maintainer (which can be you in 6 moths' time) will need knowledge like this to understand the code.

4. You should add noexcept specifications to your functions. They affect efficiency of some operations (like storing in a std::vector), and since this is a very generic class, you should not limit its use in any way. An example for the assignment operator would be:

   aligned_wrapper& operator = (const aligned_wrapper& that) noexcept(std::is_nothrow_copy_assignable<T>::value) {
       get() = that.get();
       return *this;
   }
   

5. It would probably be a good idea to introduce a way to query Align out of an aligned_wrapper object, e.g.

   static const std::size_t alignment = Align;
   

6. You don't need reinterpret_cast to convert an object pointer to void*, a static_cast is sufficient for that. Don't use stronger casts than necessary.

7. You can simplify the assignment to m_object and make it more obvious by assigning the result of the placement-new into it:

   m_object = new (ptr) T(std::forward<Args>(args)...);
   

8. 16 looks like a rather arbitrary number for the default template argument for Align (it's actually a magic number). There is even no guarantee that 16 is a valid alignment value. I would probably choose something less arbitrary such as sizeof(void*).

9. There is no reason to use std::aligned_storage for the type of m_storage. You don't actually care about alignment of m_storage at all (which is emphasised by you passing 1 to its Align parameter). All you need is a buffer of suitable size, and for this usigned char[sizeof(T) + Align -1] (or a std::array) would work just as well.

   As a side note, be aware that std::aligned_storage_t was only introduced in C++14. Visual Studio 2013 supports some C++14 bits, but using it makes your code non-C++11-compliant.

10. You might want to swap the order of the data members, it might prevent need for padding between an oddly-sized m_storage and aligned m_object.

11. You might consider overloading the pointer operators * and -> for your class as syntactic sugar for get(), it simplifies usage. Other generic classes which store a single object sometimes do so as well; an example is boost::optional.