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: Rene Rivera (grafikrobot_at_[hidden])
Date: 2012-02-28 02:09:17

On 2/26/2012 9:06 AM, Bjorn Reese wrote:
> 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.

Right.

> I know that you are doing this to allow the compiler (rather than the
> preprocessor) to use the macros.

Actually, that's not the core reason..

> 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 guess that depends on the developers. The reasons the macros are 
always defined, to zero when not detected, is to reduce the amount of 
checks when writing conditionals. At least in Boost, the common use case 
goes something like this:

#if defined(_some_compiler_) && (_some_compiler_ < 123)
//...
#endif

Or the longer equivalent:

#ifdef _some_compiler_
#if (_some_compiler_ < 123)
...
#endif
#endif

This is because many times we are interested in a particular version 
number not just if it's one platform vs. another.

> 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.

It's a good thing it's easy to change when one has a single version 
definition macro :-) At the time of choosing the 2/2/5 split it wasn't a 
problem as I wasn't including some of the predefs that could make use of 
say a bigger major number, i.e. the ones that are dates. If we switched 
to using hex and keep at least the current allowed range it would have 
to be at least FF/FE/1FFFF, i.e. a decimal range of 
[0,255]-[0,127]-[0,131071]. Which gives some slightly larger ranges in 
each number, but not enough to get a year into the major number. But it 
might be worth it just for the patch number range increase.

> 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).

Right.

> If you wish BOOST_CXX_GNUC only to be defined for GCC,

Which is the distinction between *actual* and *emulated* I mentioned in 
the other post tonight.

> 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.)

That doesn't follow from the argument. It is possible to instead have 
the clang.h file both define the Clang compiler, and undef all the other 
*emulation* predefs. Or more useful, define separate emulation predefs 
also. It's even possible to arrange it so that this works regardless of 
inclusion order of the gcc.h and clang.h headers. This has the benefit 
of keeping the knowledge about the real compiler in the real compilers' 
header only. Making it likely to be maintained accurately.

> 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

Yes, I'm not that happy with the choice of 0.0.YYYY either. And I'm 
leaning towards standardizing all the predefs with even partial dates to 
be the YYYY.MM.DD - 1970.0.0 format.

> 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".

Even if I've been programming for a long time.. I don't even remember 
having to deal with those. But if they are still sufficiently used, and 
worth supporting, I'm willing to adjust them.

> 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

At least I've run into that one a few times :-) And I'm also willing to 
adjust accordingly to increase portability.. But wouldn't this only be 
needed for the parts that would be considered by the boarland PP. I.e. 
the common parts and the borland header itself? (Assuming the others 
bail after the first #if)

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

Making a not of that, thanks.

OK, ran out of time for tonight :-( Other replies, after the C/C++UG 
meeting tomorrow night.

-- 
-- Grafik - Don't Assume Anything
-- Redshift Software, Inc. - http://redshift-software.com
-- rrivera/acm.org (msn) - grafik/redshift-software.com
-- 102708583/icq - grafikrobot/aim,yahoo,skype,efnet,gmail