Config patch to fix bug #802 --- BOOST_DISABLE_THREADS and /Za on MSVC
Since the boost thread headers can now be used without including <windows.h>, can I commit the following patch to boost/config/suffix.hpp in order to stop BOOST_DISABLE_THREADS being defined just because the user specified /Za on the command line? There was a discussion about this back in May, but I don't remember there being a definite outcome. Anthony Index: boost/config/suffix.hpp =================================================================== --- boost/config/suffix.hpp (revision 41433) +++ boost/config/suffix.hpp (working copy) @@ -190,16 +190,6 @@ # endif // -// If Win32 support is turned off, then we must turn off -// threading support also, unless there is some other -// thread API enabled: -// -#if defined(BOOST_DISABLE_WIN32) && defined(_WIN32) \ - && !defined(BOOST_DISABLE_THREADS) && !defined(BOOST_HAS_PTHREADS) -# define BOOST_DISABLE_THREADS -#endif - -// // Turn on threading support if the compiler thinks that it's in // multithreaded mode. We put this here because there are only a // limited number of macros that identify this (if there's any missing
Anthony Williams wrote:
Since the boost thread headers can now be used without including <windows.h>, can I commit the following patch to boost/config/suffix.hpp in order to stop BOOST_DISABLE_THREADS being defined just because the user specified /Za on the command line?
There was a discussion about this back in May, but I don't remember there being a definite outcome.
That would almost certainly break Regex and maybe other libraries as well :-( I would prefer not to make this change to Boost.Config for 1.35, but we should try to find a decent way of handling this. Any other ideas? Thanks, John.
John Maddock <john <at> johnmaddock.co.uk> writes:
Anthony Williams wrote:
Since the boost thread headers can now be used without including <windows.h>, can I commit the following patch to boost/config/suffix.hpp in order to stop BOOST_DISABLE_THREADS being defined just because the user specified /Za on the command line?
There was a discussion about this back in May, but I don't remember there being a definite outcome.
That would almost certainly break Regex and maybe other libraries as well
Why? The only uses of BOOST_DISABLE_THREADS that I can see are in the smart_ptr detail headers and in xpressive. In neither case will the code break if this is not defined on a multi-threaded build, but /Za is specified. There's more uses of BOOST_HAS_THREADS, but looking at them (including those in the regex headers), it seems to me that BOOST_HAS_THREADS really ought to be defined for a multi-threaded build, even if /Za is specified, otherwise the code won't be thread-safe. Code that depends on <windows.h> will break if /Za is specified, but that's a separate issue to BOOST_DISABLE_THREADS. I'm not suggesting we should be able to *build* boost with /Za, just use it.
I would prefer not to make this change to Boost.Config for 1.35, but we should try to find a decent way of handling this.
I was thinking that we didn't fix this in 1.34, so we ought to try and fix it in 1.35. Anthony
Anthony Williams wrote:
There's more uses of BOOST_HAS_THREADS, but looking at them (including those in the regex headers), it seems to me that BOOST_HAS_THREADS really ought to be defined for a multi-threaded build, even if /Za is specified, otherwise the code won't be thread-safe.
Code that depends on <windows.h> will break if /Za is specified, but that's a separate issue to BOOST_DISABLE_THREADS. I'm not suggesting we should be able to *build* boost with /Za, just use it.
I would prefer not to make this change to Boost.Config for 1.35, but we should try to find a decent way of handling this.
I was thinking that we didn't fix this in 1.34, so we ought to try and fix it in 1.35.
Well, how about you run the full regressions tests with /Za both with and without the config changes and see what happens? Regards, John.
John Maddock <john <at> johnmaddock.co.uk> writes:
Anthony Williams wrote:
There's more uses of BOOST_HAS_THREADS, but looking at them (including those in the regex headers), it seems to me that BOOST_HAS_THREADS really ought to be defined for a multi-threaded build, even if /Za is specified, otherwise the code won't be thread-safe.
Code that depends on <windows.h> will break if /Za is specified, but that's a separate issue to BOOST_DISABLE_THREADS. I'm not suggesting we should be able to *build* boost with /Za, just use it.
I would prefer not to make this change to Boost.Config for 1.35, but we should try to find a decent way of handling this.
I was thinking that we didn't fix this in 1.34, so we ought to try and fix it in 1.35.
Well, how about you run the full regressions tests with /Za both with and without the config changes and see what happens?
OK, I've done that. As expected, you can't build boost with /Za, but if you build boost without /Za, and build the tests with /Za (with or without the patch) then lots of tests do run successfully. Unfortunately, lots of other tests fail to compile. Some (such as libs/thread/test/test_tss.cpp) include <windows.h> in the test, but others don't compile because the boost headers change if /Za is specified, and some functionality is no longer available. There are a few tests that pass without the patch and fail to compile with the patch, but there are only a handful of these. I'll investigate further. Anthony
participants (2)
-
Anthony Williams -
John Maddock