Re: [boost] Review: Config system

From: Jens Maurer (Jens.Maurer_at_[hidden])
Date: 2001-08-28 15:51:53

Here are my review comments:

I'd like to see the new config system accepted into boost.

As a historical remark, when we added boost/config.hpp a long
time ago, it was intended as a stop-gap measure.  We had
longish discussions how to deal with configuration best
(e.g. use GNU configure), and the simplest thing was a
boost/config.hpp until a better, agreed-upon configuration
mechanism comes along.  I believe we've reached that point
now.

On a different topic discussed here, unknown compiler versions
should be assumed to be fully compliant.  If they aren't,
people will complain here, and we will know thusly that a new
version of some compiler has appeared and we can make appropriate
corrections.

Detailed remarks:

 - Please reformat source code files to < 80 chars line length,
at least the comments.

 - I'd like the header file names for the compiler configuration
better reflect the *compiler*.  In particular:
  tru64.hpp -> compaq_cxx.hpp       (similar to intel_icl)
  sun.hpp -> sunpro_cc.hpp
(The others are ok, though sgi.hpp -> sgi_mipspro.hpp would be an
option.)
 - select_compiler_config.hpp around line 52
avoid "(excluding Intel/EDG front end)" since a lot of others
also define _MSC_VER.  The explanation below is enough.#
 - I find the source layout of select_compiler_config.hpp not
very parsable; possibly add an empty line before each #elif?
 - Also, make sure ----------- lines end consistently without "//".

 - In select_platform_config.hpp, please change the copyright year
from 1999 to 2001.
 - For the platforms, the header file names should reflect the
operating system.  In particular:
  sun.hpp -> solaris.hpp
  be.hpp -> beos.hpp
  map.hpp -> macos.hpp
 - select_stdlib_config.hpp:  Why must "stlport" come first? Please add
a line of explanation.

 - I'm missing a select_cpu_config.hpp (to switch between x86, SPARC,
Alpha, whatever).  For example, Solaris runs on x86 and SPARC, and
those two CPUs have different endianess (boost/limits.hpp likes to
know).   Linux runs on a whole bunch of CPUs.   BSD also.

 - cstdint.hpp must be properly merged with the current cstdint.hpp,
which has a "ULL" fix.

 - suffix.hpp:  Please remove BOOST_NMEMBER_TEMPLATES now, it's
no longer referenced in the boost sources and it's in "will be
removed soon" state since ages.

 - suffix.hpp:  Please move EDG stuff to compiler/edg.hpp;
individual compiler config files should include edg.hpp if
they're EDG-based. (SGI, Alpha cxx, Comeau, Intel).
 - Is this same approach also applicable to Unix-like
platforms, i.e. should they include a platform/unix.hpp?
Then the Unix stuff in suffix.hpp could be removed.  Sounds
more orthogonal to me.

 - suffix.hpp has a bunch of empty lines at the end.

 - Please put the boost copyright boilerplate at the top of
each compiler/*.hpp.  Same for platform/*.cpp and stdlib/*.cpp.
 - Could we merge intel_icl.hpp and intel_icc.hpp?  They're
the same compiler on different platforms, effectively. 
 - compiler/true64.hpp has a misspelled filename even if my
above suggestion for renaming is not followed.
 - compiler/mpw.hpp:  Please indent consistently with the rest
of compiler/*.hpp.
 - compiler/vacpp.hpp:  Needs conditionalization on version
and fixed BOOST_COMPILER #define.
 - comiler/visualc.hpp:  Please merge the 1300 (VC7.1) case
with the 1200 (VC6) case by taking advantage of
"#if _MSC_VER <= 1300" or similar.
 - platform/be.hpp:  Doesn't need the #ifdef BOOST_DISABLE_THREADS,
because suffix.hpp will take care of undefined BOOST_HAS_THREADS
in that case.

config.htm:

 - I like the transition from BOOST_NO_HASH to BOOST_HAS_HASH
(similar for slist).  It's conceptually the right thing to do.
 - Regarding <stdint.h>:  Please read my detailed comments
at the beginning of boost/stdint.h why I had serious trouble
with Linux' <stdint.h>.
 - Assuming that new compiler/libraries/platforms are fully
compliant is ok and consistent with the philosophy of boost.
 - It's nice to see documentation on the boost conformance
macros.

Jens Maurer