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

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). 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 Joaquín M López Muñoz Telefónica, Investigación y Desarrollo

joaquin (at) tid es:
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.)
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). :-)

Peter Dimov escribió:
You can have both checks (the extra check for WIN32 et al. is immaterial to the cygwin/mingw case): #elif defined(BOOST_HAS_WINTHREADS) ||\ defined(WIN32) || defined(_WIN32) || defined(__WIN32__) # include <boost/detail/lwm_win32_cs.hpp> #else Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
participants (2)
-
joaquin@tid.es
-
Peter Dimov