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

Subject: Re: [boost] [align] Review - Impl Evaluation Part 1.
From: Glen Fernandes (glen.fernandes_at_[hidden])
Date: 2014-04-12 05:58:48

On Sat, Apr 12, 2014 at 2:21 AM, Mostafa <mostafa_working_away_at_[hidden]> 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<void*>::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<const CharPtr*>(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