Re: [boost] Review: Config system

From: John Maddock (John_Maddock_at_[hidden])
Date: 2001-08-23 05:57:46

>I would also like to see a presentation of the required changes to other 
>Boost libraries as part of the review (not all the changes needed to take 
>advantage of Config, but the ones required to be compatible with it). I 
>don't think the current Config system proposal is compatible with the 
>other libraries as-is because of defines that were moved into the Config 
>system headers. (Highly motivated reviewers can obtain this information 
>from CVS, I guess.)

I'm not sure that I understand you here - why wouldn't it be compatible? 
OK I admit regex needs to change (my problem), and stdint.hpp needs to
change (BOOST_SYSTEM_HAS_STDINT_H renamed - changes in the cvs), but that's
it.

>The EDG front-end lines from the top of <boost/config/suffix.hpp> should 
>go in a separate <boost/config/edg.hpp> file, for the same reason that we 
>have separate compiler, platform, and stdlib files.

Too be honest I didn't know what to do with this - it's a config section
that's common to several compilers - it is not a compiler in it's own
right, I guess <boost/config/compiler/edg.hpp> could be included by those
compilers that use the edg front end, but I'm not entirely sure which those
are...

>Since BOOST_HAS_THREADS tells whether a thread implementation that the 
>Threads library supports is available, the BOOST_HAS_THREADS lines should 
>be moved into the Threads library and out of the Config library -- this 
>define is both about the capabilities of the platform and about the 
>capabilities of the Thread library, so it does not belong in Config. 
>(BOOST_HAS_PTHREADS, BOOST_HAS_BETHREADS, BOOST_HAS_WINTHREADS, and the 
>like should of course remain in the Config library.)

I disagree - for several reasons:

On a practical note, discovering whether the compiler/platform has threads
is specific to that compiler/platform, it's easier to configure this inside
the existing preprocessor logic in the config system, than it is to repeat
that logic within the thread lib just for one macro.

On usage: last time I checked the thread library generates a #error if
BOOST_HAS_THREADS is not defined - so you need to check for
BOOST_HAS_THREADS before including any thread lib header, obviously you
can't do that if BOOST_HAS_THREADS only gets defined internally in the
thread lib :-)

Two other libs currently have their own thread synch support: pool and
regex, in the short term, these are likely to use BOOST_HAS_THREADS (regex
will anyway), in the long run probably changing to use the new thread lib.

>Since BOOST_NO_WREGEX tells whether the regex library provides 
>wide-character regular expressions, the BOOST_NO_WREGEX lines should be 
>moved into the Regex library and out of the Config library. We might even 
>want to consider renaming this to be in the BOOST_REGEX namespace, 
>although I like the current name.

On reflection I think I agree: there's likely to be a
boost/regex/config.hpp and boost/regex/user.hpp for informational and user
regex-specific macros (not basic configuration though, that should be
centralised).  BTW I also like the current name (obviously!)

>BOOST_DISABLE_THREADS should not be defined just because BOOST_HAS_THREADS

>is not defined. BOOST_DISABLE_THREADS should prevent BOOST_HAS_THREADS 
>from being defined even if the thread library can support threads on this 
>platform, but that should be its only function and there's no need to set 
>it when the threads implementation is not compatible with a particular 
>platform. Doing this will only encourage people to use 
>BOOST_DISABLE_THREADS directly, and make it slightly less clear what the 
>purpose of BOOST_DISABLE_THREADS is. We might even want to consider 
>renaming BOOST_HAS_THREADS and BOOST_DISABLE_THREADS to be in the 
>BOOST_THREAD namespace, although I like the current names.

OK, I also like the current name.

>The real documentation, not just the review notes, should mention what 
>happens on platforms that are not auto-detected and how unreleased 
>versions of compilers and standard libraries are handled. (Also note the 
>spelling error: "complient" should be "compliant".)

Yep.

>I suggest we take the "option (b)" for the question of "how to handle as 
>yet unreleased versions of compilers/standard libraries". We should assume

>that future versions will have all the defects that the current version 
>has rather than assuming that any new release fixes all defects. This 
>would be a default, by the way. If someone had a good reason to predict 
>that a particular new version will fix a bug, it seems fine to be 
>optimistic for that particular case, but not as a general policy.

It means some refactoring, but I'm easy either which way - any other
opinions?

>I believe that BOOST_HAS_THREADS is an informational macro, like 
>BOOST_NO_WREGEX. It tells you if the Threads library supports threads. The

>documentation does not do it this way. 

Kind of, really it's probably compiler/lib/platform dependent.

>And the documentation claims that 
>BOOST_NO_WREGEX is in <boost/regex.hpp>, when it's actually in 
><boost/config.hpp> (but shouldn't be as I suggest above).

Yep, as above.

>There a spelling error: "occurances" rather than "occurrences" in 
><boost/config/suffix.hpp>.

Thanks.

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