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

Subject: Re: [boost] [PREDEF] Review for the Boost.Predef library by Rene Riviera
From: Bjorn Reese (breese_at_[hidden])
Date: 2012-02-26 10:06:59

I have a couple of comments about Boost.Predef:

Undefined vs null
=================
You define macros for the pertinent platform (OS, CPU, compiler,
standard) and set their value to the appropriate version number. If
no version number could be determined, 0.0.1 is used instead.

But you also define macros for all non-pertinent platforms and set
their value to 0.0.0.

I know that you are doing this to allow the compiler (rather than the
preprocessor) to use the macros. However, this design decision makes
the by far most common use case (#ifdef MACRO) more difficult in order
to support a rare use case (if (MACRO)). This is not how developers are
used to working with platform macros, and it will be an eternal source
of confusion.

I encourage you to reconsider this design decision, and instead only
define macros that are relevant for the platform. This is more in line
with the expectations of your future users.

Version layout
==============
You encode the version number as decimal numbers in the range from 0
to 1000000000. If you encode them as hexadecimal numbers instead, you
can utilize the full range from 0 to 2^32.

Extension macros
================
Some macros, such as __GNUC__, _MSC_VER, and _WIN32 serve a dual
purpose. They are partly used to identify a platform, and partly to
enable certain extensions in header files. Other compilers will
therefore define these macros as well when they want access to these
extensions. For instance clang defines __GNUC__ (and related macros).

If you wish BOOST_CXX_GNUC only to be defined for GCC, then you need
to exclude all other compilers that may set the __GNUC__ macro. For
example:

#if defined(__GNUC__) && !defined(__clang__)

As your predef/compiler/gcc.h file has to know the clang macro too, you
might as well put all compiler macros into a single file (as discussed
earlier in the thread.)

C++ version
===========
Your choice of using 0.0.YYYY as the version layout for C++ standards
is quite unusual. I understand that you are doing it for technical
reasons (the patch number is the only entry that can contain four
digits numbers), but I still think that a more intuitive solution is
needed.

One solution could be to use an encoding similar to your
BOOST_PREDEF_MAKE_YYYY_MM_DD macro.

Another solution could be to define an explicit macro for each supported
standard. E.g.

#if __cplusplus - 0 >= 199711L
# define BOOST_LANGUAGE_CXX98 1
#endif

Indentation of preprocessor directives
======================================
Old preprocessors will only recognize directives if these is a hash (#)
in the first column of the line. The most portable way of indenting
directives is therefore to put the hash sign in the first column and
then put whitespaces between the hash sign and the rest of the
directive. E.g. "#  if" instead of "  #if".

Else if
=======
You use the #elif directive in some files. Notice that older Borland
preprocessors will fail with that directive. The portable way to do
else-ifs is the more tedious alternative:

#if MACRO
#else
# if MACRO2
# endif
#endif

Apropos else-if, in compiler/greenhills.h you are using the incorrect
"#else if defined" construct.

Credit
======
As the main author of the predef.sourceforge.net pages, it is pretty
clear to me that you have extracted much information from those pages,
so I think that a reference to those pages may be in order.