Re: [boost] review requiest: config_06.zip

From: John Maddock (John_Maddock_at_[hidden])
Date: 2001-08-02 06:30:32

>There's a boost/test directory in the zip file that shouldn't be there.

Yep, it's an artifact from the way I built the zip, doesn't do any harm
though..

>Why the word "SYSTEM" in "BOOST_SYSTEM_HAS_STDINT_H"? It seems that this 
>is a property of the C standard library implementation, not the "system". 
>Perhaps there's some confusion here because Unix systems tend to include C

>standard library implementations, so on Unix this is part of the "system".

Good question, that's the way it is at present - changing it means changing
boost code, although it's just a search and replace right?

>In my opinion BOOST_REGEX_USE_C_LOCALE and BOOST_REGEX_USE_CPP_LOCALE 
>should be documented inside the regex library. If they can be generalized 
>for use outside the regex library, then their names should be changed. I 
>am a bit concerned about having specific configuration options for the 
>specific Boost libraries in the default user.hpp. Doesn't seem quite right

>to me.

Yes I take your point about that. I don't know what the corrext answer is -
it seems right to me that user settable options should all be in one place
- but as you say there may be problems with that.

>Since select_platform_config.hpp has to use double quotes around boost/xxx

>includes, why not switch to "boost/xxx" instead of <boost/xxx> throughout?
>  This macro issue seems like another point in favor of "" includes
instead 
>of <> includes -- perhaps enough to tip the scales?

Well I changed just as much as I had to, but as you say this may well tip
the scales in favour of "" over <>.

>For features that are not yet correct in any known version of a compiler, 
>I think it's better to assume that future versions have the same 
>limitations by default, rather than assuming that any newer version of the

>compiler is fully ISO compliant. Here's an example.
>
>       #   if __MWERKS__ <= 0x2401  // 6.2
>       #     define BOOST_NO_MEMBER_TEMPLATE_FRIENDS
>       #     define BOOST_WEAK_FUNCTION_TEMPLATE_ORDERING
>       #   endif
>
>The above should not be inside an ifdef at all. This code assumes that any

>newer version of the compiler would fix both of the problems. I think it's

>better to assume that any newer version is like the older versions. In 
>either case, once actual information about the newer version shows up, we 
>can update the configuration file.

That runs contrary to current config.hpp practice, personally I don't care
one way or another, any other opinions?

>I get the idea that for the most part, "platform" means "standard C 
>library implementation". Although this could be considered part of the 
>platform on Unix, it's not that way on non-Unix systems, I don't think. 
>For example, we define things like BOOST_NO_SWPRINTF and 
>BOOST_SYSTEM_HAS_STDINT_H in there, even though the issue is the standard 
>C library, not the "platform" per se.

It's a problem this one, when the C lib is tied to the compiler I've put
options like  BOOST_NO_SWPRINTF in the compiler section, where it's tied to
the OS then it goes uder the platform config.  In general though the
platform config describes properties of the platform that are compiler
independent.

>- config.htm
>Some defines have equal signs in them (example: "#define 
>BOOST_COMPILER_CONFIG=").
>Says <boost.cstdint.hpp> instead of <boost/cstdint.hpp> in a few places.

Oops OK, fixed.

>- config.hpp
>Typo: suffic code

Fixed.

>- user.hpp
>Typo: ISO complient

Fixed

>- select_platform_config.hpp
>Typo: "linix" is a macro
>Comment "cygwin is not win32" appears in the WIN32 section.

Got it.

>- suffix.hpp
>I think the leading underscore in "_BOOST_NO_STD_MIN_MAX" is a mistake.

Yep.

>- metrowerks.hpp
>It should be "Metrowerks CodeWarrior C++ version ", not "Metrowerks 
>codewarrior ".

Yep.

>- mpw.hpp
>It should be "MPW SCpp version ", not "MPW SCpp ".
>It should be "MPW MrCpp version ", not "MPW MrCpp ".

Yep.

>- visualc.hpp:
>It should be "Microsoft Visual C++ version ", not "Microsoft Visual C++ 
>Version ".

Yep.

>- msl.hpp
>It should be "Metrowerks Standard Library version ", not "MSL standard 
>library version ".

yep.

>- metrowerks.hpp
>It seems strange to check for the existence of <stdint.h> by checking the 
>compiler version. It would be more correct to check the C library version 
>by checking the value of __MSL__ after including at least one C library 
>header (<cstddef> is good candidate, I think). In particular, there's a 
>mode on OS X where Metrowerks compiler and C++ library are used with a BSD

>Unix C library. However, the BSD library does also include <stdint.h>, so 
>the current version will work even though it's strange.

I don't have access to that platform, so tell me the way it should look,
that's what I'll go with...

>- select_platform_config.hpp
>When compiling with gcc under Mac OS X, "macintosh" is not defined. We can

>check for "__APPLE__" or "__APPLE_CC__" to handle this case.

Got it.

>- linux.hpp
>I'm a bit puzzled by the defines for __extension__, __const__, 
>__volatile__, __signed__, __typeof__ and __inline__. What specific code in

>Boost does these help? In particular, I wonder what the use of having 
>__extension__ defined is. Defining the others doesn't seem particularly 
>specific to Linux -- there are other platforms where gcc is often used but

>occasionally another compiler is used that would probably benefit from 
>this hack (depending on what it's for exactly).

It's so that non-gcc compilers can handle stdint.h on linux (which uses
__extension__ prior to int64_t etc), as far as I know, linux is the only
platform that uses gcc specific keywords in system headers... this needs
more testing as I think I noted in the reviewers notes.

>- mac.hpp
>It's not correct to say that this is "pre-X". For example, when compiling 
>Mac-OS-X-only code with Metrowerks CodeWarrior (even in the "Mach-O mode" 
>which makes it use BSD headers), you'll still end up including mac.hpp 
>because "macintosh" will be defined. And also I propose using this even 
>for the Mac OS X case.

Yes, I just went with what Daryle gave me here.

>The following macros are missing when using Metrowerks CodeWarrior under 
>Mac OS X in "Mach-O mode" (not using MSL C library) or when using gcc 
>under Mac OS X and need to be defined. One way to check for both of these 
>is to check the __MACH__ define in mac.hpp after making the change to 
>include mac.hpp when "__APPLE__" is defined too.

     BOOST_HAS_PTHREADS
     BOOST_HAS_UNISTD_H
     BOOST_NO_CWCHAR
     BOOST_NO_CWCTYPE
     BOOST_NO_INTRINSIC_WCHAR_T
     BOOST_NO_STDC_NAMESPACE
     BOOST_NO_SWPRINTF
     BOOST_SYSTEM_HAS_STDINT_H

Got it.

>Hope this is helpful.

You bet, thanks!

- John Maddock
http://ourworld.compuserve.com/homepages/john_maddock/