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.
1 Answer 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.
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.
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 ofthis
will not change, this guarantees alignment. In other words, not assigningm_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.
You should add
noexcept
specifications to your functions. They affect efficiency of some operations (like storing in astd::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; }
It would probably be a good idea to introduce a way to query
Align
out of analigned_wrapper
object, e.g.static const std::size_t alignment = Align;
You don't need
reinterpret_cast
to convert an object pointer tovoid*
, astatic_cast
is sufficient for that. Don't use stronger casts than necessary.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)...);
16
looks like a rather arbitrary number for the default template argument forAlign
(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 assizeof(void*)
.There is no reason to use
std::aligned_storage
for the type ofm_storage
. You don't actually care about alignment ofm_storage
at all (which is emphasised by you passing1
to itsAlign
parameter). All you need is a buffer of suitable size, and for thisusigned char[sizeof(T) + Align -1]
(or astd::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.You might want to swap the order of the data members, it might prevent need for padding between an oddly-sized
m_storage
and alignedm_object
.You might consider overloading the pointer operators
*
and->
for your class as syntactic sugar forget()
, it simplifies usage. Other generic classes which store a single object sometimes do so as well; an example isboost::optional
.
Explore related questions
See similar questions with these tags.