Hi Thomas,
Please go ahead. It's your call whether to simply revert or accept Richard's patch. The way I look at it is that the patch has higher risk but a nice testcase, that makes it even to me.
Well, it turns out that reverting isn't as straightforward as I figured. The problem is that the previous version used two separate headers "none_t.hpp" and "none.hpp", with only the first being included by "optional.hpp", forcing users to include "none.hpp" manually. That separation was simply intended to allow Borland users to work around the PCH problem. But only so much. A nice feature of the "new" implementation (the one that caused all this trouble) is that you no longer need to include "none.hpp" manually anymore if you use optional<>. But I really hated the idea of going back to 2 headers, so I spent quite some time trying to make an informed decision. First, I searched the list archives for threads about 'none'. Interestingly enough, I found this: http://thread.gmane.org/gmane.comp.lib.boost.devel/146009/focus=146126 There you can see the proposal for the alternative implementation, which eventually I put in place and casused all this, but, you can also see that at _that_ time, I was much much more lucid that in the recent days and figured that it would cause the issue Richard just found. Furthermore, in that thread there is a different implementation proposed by Anthony Williams which, incidentally, is the same Steven just posted here (that code is slightly more elaborated, but the principle is the same). You can also read that I agreed at the time with Anthony's implementation (...but then my clone forgot about it and picked the wrong one to commit ;) Then I tried out Anthony's version (attached here) with the Optional testsuite but also with a new specific test (derived from Richard's post here, see attached). The tests pass with toolsets msvc-8.00 and msvc-7.1 in WinXP. I can test it with gcc (4.4.1) in a Debian Linux Box, but not today (8pm already here). And probably not until next Monday. I installed the command line version of borland 5.5.1, but when I run "bjam --toolset=borland", bjam crashes, so I couldn't test there. Can those participating in this thread take a look at the new implementation, test if you can, and report back so we can decide if it is safe to commit? TIA ======================================================================= // Copyright (C) 2003, Fernando Luis Cacciola Carballal. // Copyright (C) 2007, Anthony Williams // // Distributed under the Boost Software License, Version 1.0. // (See accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) // // See http://www.boost.org/lib/optional/ for documentation. // // You are welcome to contact the author at: // fernando.cacciola@gmail.com // #ifndef BOOST_NONE_17SEP2003_HPP #define BOOST_NONE_17SEP2003_HPP namespace boost { namespace detail { class none_helper; } inline void none(detail::none_helper); namespace detail { class none_helper { private: none_helper( none_helper const& ) {} friend void boost::none(none_helper); }; } typedef void (*none_t)(detail::none_helper); inline void none(detail::none_helper) {} } #endif none_test.cpp: ======================================= #include <boost/optional/optional.hpp> // Test that none.hpp is included with <optional.hpp> // Left undefined to cause a linker error if this overload is incorrectly selected. void verify_no_implicit_conversion_to_int ( int i ) ; void verify_no_implicit_conversion_to_int ( boost::optional<int> const& ) {} int main() { verify_no_implicit_conversion_to_int( boost::none ); } =======================================