Re: [Boost-users] [PREDEF] Review for the Boost.Predef library by Rene Riviera

Subject: Re: [Boost-users] [PREDEF] Review for the Boost.Predef library by Rene Riviera
From: Mathias Gaunard (mathias.gaunard_at_[hidden])
Date: 2012-02-29 13:35:50

On 02/18/2012 03:08 PM, Joel Falcou wrote:

> ================
> Writing a Review
> ================
> The reviews and all comments should be submitted to the developers list,
> and the email should have "[PREDEF] Review" at the beginning of the
> subject line to make sure it's not missed.
> Please explicitly state in your review whether the library should be
> accepted.

I do not think that this library should be accepted in Boost, and I 
think a fairly important overhaul would be required to get it to a 
satisfying state.

> The general review checklist:
>
> - What is your evaluation of the design?

Checking for different compilers in independent files simply doesn't 
work due to some compilers emulating others.
We have talked about __GNUC__, but there are several compilers that 
define _MSC_VER as well.

The library should use, like Boost.Config and Boost.Detect, a chain 
if/elif to ensure that all values are mutually exclusive.

Other issues in the design include the way macros are defined even if 
the associated element is not detected. This is fairly confusing for the 
user.

The design is not sufficiently good to make Boost.Config rely on it.

> - What is your evaluation of the implementation?

Implementation does not do what is expected, as was demonstrated by 
Artyom's tests.

It detects x86-64 as Itanium, wrongly detects the standard library 
(detects two of them at the same time, while only one should be 
possible), detects clang as gcc, etc.

Not robust enough to become the most basic piece of the Boost 
infrastructure, being included with every other Boost header.

> - What is your evaluation of the documentation?

Documentation was good, but misled me to believe the library didn't do 
things as it does.

It would probably be a good idea to say in what conditions macros are 
defined. The documentation made me assume all macros were 
mutually-exclusive: clearly this is not the case at all, which is why 
this library is not suitable for inclusion.

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

This library, if done right, would be very useful to correctly deal with 
frontend-specific, backend-specific, library-implementation-specific, 
architecture-specific or OS-specific code.

I therefore strongly encourage people to submit more proposals on the 
subject.

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

I did not try to use the library.
I read the documentation and skimmed through the code.

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

A quick reading.

> - Are you knowledgeable about the problem domain?

Yes.

> And finally, every review should answer this question:
>
> - Do you think the library should be accepted as a Boost library?

No.