Re: [boost] Boost.Align review begins today

Subject: Re: [boost] Boost.Align review begins today
From: Lars Viklund (zao_at_[hidden])
Date: 2014-04-19 06:38:02

On Fri, Apr 11, 2014 at 08:48:02PM -0700, Ahmed Charles wrote:
> The review of Boost.Align by Glen Fernandes commences today, Friday, 11 April, and concludes Sunday, 20 April.
> Please always state in your review whether you think the library should be accepted as a Boost library.

I believe that the Align library should be accepted into Boost without
any conditional requirements from my side.

> Additionally, please consider giving feedback on the following general topics:
> 
> - What is your evaluation of the design?

The aligned allocation/destruction functions are similiar in shape to
the platform specific functions with similiar names.

Judging by the documentation, the allocators and functions have as
natural interfaces as possible, which is good.

> - What is your evaluation of the implementation?

I've skimmed the implementation and no apparent madness stands out.

> - What is your evaluation of the documentation?

Documentation as it currently stands needs some Boostification, but I
believe that has been addressed already. I find the standardese-like
reference readable.

> - What is your evaluation of the potential usefulness of the library?

This library is very useful to anyone that ever has needed to deal with
alignment, saving them from adapting and reinventing the facilities
present in the library.

> - Did you try to use the library? With what compiler? Did you have any problems?

I ran the bundled tests with a set of compilers with no additional
flags, with Align implanted into a modular Boost checkout.

Works perfectly:
GCC 4.6.3, Intel 13.0, Intel 13.1

Compiler errors:
PGI 13.10, PGI 14.3 (needed to modify Boost.Build to avoid linker failures)
  A lot of ambiguities like
  "aligned_allocator_adaptor_test.cpp", line 31: error: "allocator" is ambiguous

Intel 12.0
  Breaks in Boost.Config on __int128, so not really Align's fault.

Runtime errors:
PSC 5, may be due to site customizations of the toolchain so may be
disregarded, I have to look more into it.
  is_aligned_test.cpp fails intermittently for sizes 128, 64, and sometimes 32.
  is_aligned_test.cpp(44): test 'boost::alignment::is_aligned(32, &s[0])' failed in function 'int main()'
  is_aligned_test.cpp(49): test 'boost::alignment::is_aligned(64, &s[0])' failed in function 'int main()'
  is_aligned_test.cpp(54): test 'boost::alignment::is_aligned(128, &s[0])' failed in function 'int main()'

  Fails 'align_test.cpp' assertions on lines 23, 25, 31, 32, 33, 40, 42; once for 64 and once for 128 bytes.

Sadly SunStudio and VisualAge are out of my reach at the moment,
checking out modular Boost on those filesystems would take days.

I can provide logs on request.

> - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?

An evening skimming and reflecting on the documentation and intended
semantics, a morning running the test suites on any compilers I could
reach.

> - Are you knowledgeable about the problem domain?

I've reinvented most facilities of this library in more or less horrible
ways in the past.

-- 
Lars Viklund | zao_at_[hidden]