Re: [boost] [align] Review - Impl Evaluation Part 1.

On Sat, Apr 12, 2014 at 2:21 AM, Mostafa wrote:

Should "alignment" be silently set to alignof(void *) if its more weak than alignof(void *), or should the restrictions on "alignment" be a precondition for calling this function and a BOOST_ASSERT check be made on it. Either way it should be noted in the documentation. (More importantly, does this silent behaviour also apply to the provided platform specific aligned_alloc functions?)

The intention is that aligned_alloc() should have the same requirements whether it uses my implementation, MSVC's, POSIX's, or any other. So if a particular implementation specific version has stronger requirements for the value of 'alignment', my inline aligned_alloc wrapper around that particular platform-specific function will relax those requirements (by rounding up 'alignment' if necessary).

align/detail/aligned_alloc_{android,msvc,posix,sunos}.hpp What happens when "size" parameter is 0? According to "boost::aligned_alloc" specs it should return a null pointer. This also seems to be the behaviour of aligned_alloc on both msvc and posix, but sunos maybe different (consult the table below).

The documentation should be written more clearly to reflect this; the intention is that if zero is passed, then the value returned could be a null pointer, or it could be non-null, but use of that pointer is undefined. It can only be passed to aligned_free() and nothing else.

Should "align" BOOST_ASSERT if "alignment < alignment_of::value" ? Since, per the doc, alignment shall be a fundamental alignment value or an extended alignment value, and the alignment of "void *" represents the weakest alignment requirement.

An alignment smaller than alignof(void*) is still a valid fundamental alignment, so align() should be usable with such values.

pointer allocate(size_type size, const_void_pointer hint) Is the preprocessor conditional code really needed? Can't it all be replaced by "a1.allocate(...)" since "a1" is required by the standard to define such a function?

While [allocator.requirements] implies that every allocator should provide an overload for allocate() that takes hint, [allocator.traits.members] 20.6.8.2/2 implies that for a given allocator, allocate(n, hint) may not be well-formed, and so allocator_traits<...>::allocate(n, hint) will call allocate(n) if allocate(n, hint) is not well-formed.

I don't see the point of "h1 = *(static_cast(hint) - 1);". Presumably the point of using this overloaded version of "allocate" is to allocate memory at memory address "hint", not construct an object there, since the starting hint address is not guaranteed to be aligned properly.

The inner allocator's allocate(..., hint) can assume that hint is a value previously returned by its allocate() (on an equivalent allocator object). The hint passed to the adaptor would not be that value - it would be the value returned by a call to allocate() on the adaptor. So: This just ensures that the hint passed to the inner allocator is the right value. Glen