Re: [boost] boost::mpl::for_each and value_initialized
Peter Foelsche wrote:
in boost_1_41_0 there is still an unconditional call to memset in value_initialized()
Fernando Cacciola and I originally added the memset call within value_initialized() for MSVC only, because MSVC's implementation of value-initialization was known to incorrectly leave some bytes uninitialized, in some very relevant use cases. https://svn.boost.org/trac/boost/changeset/39157/trunk/boost/utility/value_i... Thereby we also added extra unit tests, which were passed by MSVC by then, because of the memset. But unfortunately they failed on other compilers (GCC, Borland/Codegear and Sun), because of similar value-initialization issues. That's when we thought it was wise to just have the memset call there for all compilers: https://svn.boost.org/trac/boost/changeset/41942/trunk/boost/utility/value_i... Do I understand correctly that you want the memset call to be skipped for those compiler versions that have implemented value-initialization entirely correctly? Do you know exactly which compiler versions do implement value-initialization correctly? Personally I do appreciate that the behavior of value_initialized is now exactly the same for all compilers. But I do understand you're concerned about the performance penalty of the memset call, for those compilers that have implemented value-initialization correctly. If so, can you please create a ticket, preferably including a patch? Kind regards, Niels -- Niels Dekker http://www.xs4all.nl/~nd/dekkerware Scientific programmer at LKEB, Leiden University Medical Center
"Niels Dekker - address until 2010-10-10" <niels_address_until_2010-10-10@xs4all.nl> wrote in message news:4716CDF08A964D95A470AB95BEEFD53B@lumcnet.prod.intern...
Do I understand correctly that you want the memset call to be skipped for those compiler versions that have implemented value-initialization entirely correctly? Do you know exactly which compiler versions do implement value-initialization correctly?
I'm only concerned with boost::mpl::for_each This function calls a function object by passing an object of the matching type. The contents of this object usually do not matter -- and in case they do, the programmer can write a default constructor to initialize this object. So there is no need in boost::mpl::for_each to call value_initialized. Another solution would be to pass a zero pointer to an object of this type. Anyway -- this argument is only used to select the correct template method of the function object passed. Peter
On Wed, 13 Jan 2010 11:11:20 -0600, Niels Dekker - address until 2010-10-10 <niels_address_until_2010-10-10@xs4all.nl> wrote:
Peter Foelsche wrote:
in boost_1_41_0 there is still an unconditional call to memset in value_initialized()
Fernando Cacciola and I originally added the memset call within value_initialized() for MSVC only, because MSVC's implementation of value-initialization was known to incorrectly leave some bytes uninitialized, in some very relevant use cases. https://svn.boost.org/trac/boost/changeset/39157/trunk/boost/utility/value_i... Thereby we also added extra unit tests, which were passed by MSVC by then, because of the memset. But unfortunately they failed on other compilers (GCC, Borland/Codegear and Sun), because of similar value-initialization issues. That's when we thought it was wise to just have the memset call there for all compilers: https://svn.boost.org/trac/boost/changeset/41942/trunk/boost/utility/value_i...
Do I understand correctly that you want the memset call to be skipped for those compiler versions that have implemented value-initialization entirely correctly? Do you know exactly which compiler versions do implement value-initialization correctly?
Personally, I believe the assumption that by default compilers get this wrong is incorrect and should be revised. IOW, I think we should revert to the approach of applying the workaround conditionally for the specific compilers known to be buggy, and just those -- especially if we have tests to discover these bugs should they return. As for the specific compiler versions: - GCC bugs #30111 and #33916 has been fixed in 4.4 and 4.2.4 correspondingly [1], [2]. - MSVC bug #100744, despite being marked as "Closed, Won't Fix" [3], has actually been fixed in VC9 [4]. - Borland bug #51854 is also reported as being fixed in build "12.0.3140.16150" [5] [1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30111#c10 [2] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33916#c12 [3] https://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?Feedba... [4] https://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?Feedba... [5] http://qc.embarcadero.com/wc/qcmain.aspx?rc=51854 -- Aleksey Gurtovoy MetaCommunications Engineering
Do I understand correctly that you want the memset call to be skipped for those compiler versions that have implemented value-initialization entirely correctly? Do you know exactly which compiler versions do implement value-initialization correctly?
Aleksey Gurtovoy wrote:
[...] I think we should revert to the approach of applying the workaround conditionally for the specific compilers known to be buggy, and just those -- especially if we have tests to discover these bugs should they return.
As for the specific compiler versions:
- GCC bugs #30111 and #33916 has been fixed in 4.4 and 4.2.4 correspondingly [1], [2].
So GCC 4.2.4 still needs the memset, but GCC >= 4.4.0 can do without it, right?
- MSVC bug #100744, despite being marked as "Closed, Won't Fix" [3], has actually been fixed in VC9 [4].
I'm sorry, MSVC bug #100744 is still there, even though #335987 has been fixed. VC9 deals with POD types correctly, but may not do so for non-POD aggregate class types.
- Borland bug #51854 is also reported as being fixed in build "12.0.3140.16150" [5]
Looks promising. Do you know how to do a compile-time check if Borland/Codegear build >= 12.0.3140.16150? Anyway, I wouldn't mind if the memset call in utility/value_init.hpp would become conditional, for example as follows: #ifdef BOOST_VALUE_INITIALIZATION_NEEDS_MEMSET std::memset(&x, 0, sizeof(x)); #endif What do you think? And would it be okay to you to have such a macro, BOOST_VALUE_INITIALIZATION_NEEDS_MEMSET, defined in boost/config/compiler/, for those specific compiler versions?
[1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=30111#c10 [2] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33916#c12 [3] https://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?Feedba... [4] https://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?Feedba... [5] http://qc.embarcadero.com/wc/qcmain.aspx?rc=51854
Thanks! Kind regards, Niels -- Niels Dekker http://www.xs4all.nl/~nd/dekkerware Scientific programmer at LKEB, Leiden University Medical Center
On Fri, 22 Jan 2010 04:06:40 -0600, Niels Dekker - address until 2010-10-10 <niels_address_until_2010-10-10@xs4all.nl> wrote:
Aleksey Gurtovoy wrote:
[...] I think we should revert to the approach of applying the workaround conditionally for the specific compilers known to be buggy, and just those -- especially if we have tests to discover these bugs should they return.
As for the specific compiler versions:
- GCC bugs #30111 and #33916 has been fixed in 4.4 and 4.2.4 correspondingly [1], [2].
So GCC 4.2.4 still needs the memset, but GCC >= 4.4.0 can do without it, right?
Yep.
- MSVC bug #100744, despite being marked as "Closed, Won't Fix" [3], has actually been fixed in VC9 [4].
I'm sorry, MSVC bug #100744 is still there, even though #335987 has been fixed. VC9 deals with POD types correctly, but may not do so for non-POD aggregate class types.
Hmm, you are right, it doesn't.
- Borland bug #51854 is also reported as being fixed in build "12.0.3140.16150" [5]
Looks promising. Do you know how to do a compile-time check if Borland/Codegear build >= 12.0.3140.16150?
I don't, but may be somebody more familiar with C++ Builder can help us.
Anyway, I wouldn't mind if the memset call in utility/value_init.hpp would become conditional, for example as follows:
#ifdef BOOST_VALUE_INITIALIZATION_NEEDS_MEMSET std::memset(&x, 0, sizeof(x));
#endif
What do you think? And would it be okay to you to have such a macro, BOOST_VALUE_INITIALIZATION_NEEDS_MEMSET, defined in boost/config/compiler/, for those specific compiler versions?
Sounds good to me. John? -- Aleksey Gurtovoy MetaCommunications Engineering
Anyway, I wouldn't mind if the memset call in utility/value_init.hpp would become conditional, for example as follows:
#ifdef BOOST_VALUE_INITIALIZATION_NEEDS_MEMSET std::memset(&x, 0, sizeof(x)); #endif
Fernando (the author of value_init) mailed me that it's okay to him if I add an #ifdef like that to utility/value_init.hpp. Still I think it would be useful to have the issue officially reported. Aleksey, were you already planning to open a ticket on this issue? If so, please do! And please feel free to assign the ticket to niels_dekker :-) Of course, <boost/config/compiler/...> also needs to have BOOST_VALUE_INITIALIZATION_NEEDS_MEMSET added to various compiler versions: MSVC (all versions) Borland/Codegear build < 12.0.3140.16150 Sun (possibly all versions, I don't know exactly) GCC < 4.4.0
What do you think? And would it be okay to you to have such a macro, BOOST_VALUE_INITIALIZATION_NEEDS_MEMSET, defined in boost/config/compiler/, for those specific compiler versions?
Aleksey Gurtovoy wrote:
Sounds good to me. John?
Hope it's okay to John as well. Kind regards, Niels -- Niels Dekker http://www.xs4all.nl/~nd/dekkerware Scientific programmer at LKEB, Leiden University Medical Center
If I understand correctly, the only purpose of the initialized argument is to select the correct template method? Recently I had another problem with mpl::for_each, in that I was trying to use it to iterate over a sequence of tag types. For my application it was most convenient to leave these tag types incomplete (macros could declare them more than once), using them only for template logic. Of course mpl::for_each then failed because it was trying to instantiate incomplete types. I guess that David Abrahams' trick of transforming the sequence to wrap<T> could have worked (it didn't occur to me), but it feels kludgy. I prefer to minimize compile times and avoid the value_initialized business. My solution was to write "for_each_type", which is just mpl::for_each without the value_initialized argument. Maybe this should also exist in the MPL? It seems both approaches have drawbacks. mpl::for_each does not work for some types (without sequence transformation), and it creates an instance that normally is not used. "for_each_type" does not work for normal function objects - though I am not convinced that is a typical use case. For anyone who's interested, I've appended "for_each_type" below. This is a "clean" implementation, that assumes a well-behaved compiler. -Patrick #include <boost/mpl/is_sequence.hpp> #include <boost/mpl/begin_end.hpp> #include <boost/mpl/next_prior.hpp> #include <boost/mpl/deref.hpp> #include <boost/mpl/assert.hpp> #include <boost/mpl/aux_/unwrap.hpp> #include <boost/type_traits/is_same.hpp> template< bool done = true > struct for_each_type_impl { template<typename Iterator, typename LastIterator, typename F> static void execute(F) {} }; template<> struct for_each_type_impl<false> { template<typename Iterator, typename LastIterator, typename F> static void execute(F f) { typedef typename boost::mpl::deref<Iterator>::type arg; boost::mpl::aux::unwrap(f, 0).template operator()<arg>(); typedef typename boost::mpl::next<Iterator>::type iter; for_each_type_impl<boost::is_same<iter, LastIterator>::value> ::template execute<iter, LastIterator, F>(f); } }; template<typename Sequence, typename F> void for_each_type(F f) { BOOST_MPL_ASSERT(( boost::mpl::is_sequence<Sequence> )); typedef typename boost::mpl::begin<Sequence>::type first; typedef typename boost::mpl::end<Sequence>::type last; for_each_type_impl< boost::is_same<first, last>::value >::template execute<first, last, F>(f); } On Tue, Jan 26, 2010 at 7:27 AM, Niels Dekker - address until 2010-10-10 < niels_address_until_2010-10-10@xs4all.nl> wrote:
Anyway, I wouldn't mind if the memset call in utility/value_init.hpp
would become conditional, for example as follows:
#ifdef BOOST_VALUE_INITIALIZATION_NEEDS_MEMSET std::memset(&x, 0, sizeof(x)); #endif
Fernando (the author of value_init) mailed me that it's okay to him if I add an #ifdef like that to utility/value_init.hpp. Still I think it would be useful to have the issue officially reported. Aleksey, were you already planning to open a ticket on this issue? If so, please do! And please feel free to assign the ticket to niels_dekker :-)
Of course, <boost/config/compiler/...> also needs to have BOOST_VALUE_INITIALIZATION_NEEDS_MEMSET added to various compiler versions:
MSVC (all versions)
Borland/Codegear build < 12.0.3140.16150 Sun (possibly all versions, I don't know exactly) GCC < 4.4.0
What do you think? And would it be okay to you to have such a macro,
BOOST_VALUE_INITIALIZATION_NEEDS_MEMSET, defined in boost/config/compiler/, for those specific compiler versions?
Aleksey Gurtovoy wrote:
Sounds good to me. John?
Hope it's okay to John as well.
Kind regards,
Niels -- Niels Dekker http://www.xs4all.nl/~nd/dekkerware<http://www.xs4all.nl/%7End/dekkerware> Scientific programmer at LKEB, Leiden University Medical Center
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Tue, 26 Jan 2010 09:27:40 -0600, Niels Dekker - address until 2010-10-10 <niels_address_until_2010-10-10@xs4all.nl> wrote:
Anyway, I wouldn't mind if the memset call in utility/value_init.hpp would become conditional, for example as follows:
#ifdef BOOST_VALUE_INITIALIZATION_NEEDS_MEMSET std::memset(&x, 0, sizeof(x)); #endif
Fernando (the author of value_init) mailed me that it's okay to him if I add an #ifdef like that to utility/value_init.hpp. Still I think it would be useful to have the issue officially reported. Aleksey, were you already planning to open a ticket on this issue? If so, please do! And please feel free to assign the ticket to niels_dekker :-)
Done: https://svn.boost.org/trac/boost/ticket/3869 Thanks, -- Aleksey Gurtovoy MetaCommunications Engineering
"Niels Dekker - address until 2010-10-10" <niels_address_until_2010-10-10@xs4all.nl> wrote in message news:42DCB3BF36D94D4D9714434725145288@lumcnet.prod.intern...
- MSVC bug #100744, despite being marked as "Closed, Won't Fix" [3], has actually been fixed in VC9 [4].
I'm sorry, MSVC bug #100744 is still there, even though #335987 has been fixed. VC9 deals with POD types correctly, but may not do so for non-POD aggregate class types.
but if this is so, shouldn't the proposed patch atleast be modified to: #ifdef BOOST_VALUE_INITIALIZATION_NEEDS_MEMSET if ( !( ( _MSC_VER >= 1500 ) && ( is_pod<T>::value ) ) ) std::memset(&x, 0, sizeof(x)); #endif ? -- "That men do not learn very much from the lessons of history is the most important of all the lessons of history." Aldous Huxley
participants (5)
-
Aleksey Gurtovoy -
Domagoj Saric -
Niels Dekker - address until 2010-10-10 -
Patrick Mihelich -
Peter Foelsche