Re: [boost] [detail/lightweight_mutex] Win32 threading selection in boost/detail/lightweight_mutex.hpp

Subject: Re: [boost] [detail/lightweight_mutex] Win32 threading selection in boost/detail/lightweight_mutex.hpp
From: Peter Dimov (pdimov_at_[hidden])
Date: 2008-12-17 12:03:57

joaquin (at) tid es:

> Hello,
>
> This is a followup of a discussion we had some time ago:
>
> http://lists.boost.org/Archives/boost/2007/09/127115.php
>
> Lines 31-40 of boost/detail/lightweight_mutex.hpp read like this:
>
> #if !defined(BOOST_HAS_THREADS)
> # include <boost/detail/lwm_nop.hpp>
> #elif defined(BOOST_HAS_PTHREADS)
> #  include <boost/detail/lwm_pthreads.hpp>
> #elif defined(WIN32) || defined(_WIN32) || defined(__WIN32__)
> #  include <boost/detail/lwm_win32_cs.hpp>
> #else
> // Use #define BOOST_DISABLE_THREADS to avoid the error
> #  error Unrecognized threading platform
> #endif
>
> I think the conditions for selecting boost/detail/lwm_win32_cs.hpp are 
> suboptimal because they're ruling out the case where the user's disabled 
> Pthreads in cygwin/mingw by explicitly defining BOOST_HAS_WINTHREADS, a 
> case covered by boost/config/platform/cygwin.hpp: in such a situation, 
> Win32 threading is available but none of WIN32 and similar are defined by 
> default (unless <windows.h> has been previously included, which makes
boost/detail/lightweight_mutex.hpp context dependent).

You're right, this is a bug. I didn't realize that defining 
BOOST_HAS_WINTHREADS manually was a supported configuration. (Aside: I 
wonder whether we need the test for the plain 'WIN32', it doesn't seem like 
a predefined macro and leads to the context dependence you mention. I just 
copied these conditions from select_platform_config.hpp.)

> Wouldn't it be better to change the selection code to the following?
>
> #elif defined(BOOST_HAS_WINTHREADS)
> #  include <boost/detail/lwm_win32_cs.hpp>
> #else

This is (I suspect) not done to avoid the old issue with /Za defining 
BOOST_DISABLE_WIN32 defining BOOST_DISABLE_THREADS undefining 
BOOST_HAS_WINTHREADS. On the other hand, in this specific case /Za would 
lead to the (incorrect) selection of lwm_nop and the win32_cs branch would 
never be reached, so it would "work" (that is, fail in the same way). :-)