Boost C++ Libraries: Ticket #2548: Let's fix the logical constness of value_initialized!

attachment set

niels_dekker — Fri, 28 Nov 2008 19:26:34 GMT

attachment → value_initialized_constness.patch

owner, status changed

niels_dekker — Fri, 20 Feb 2009 20:24:56 GMT

owner changed from Fernando Cacciola to niels_dekker
status new → assigned

Fernando Cacciola allowed me to fix the issue :-) See also "[boost] [utility] A fix for the const issue of value_initialized", http://lists.boost.org/Archives/boost/2009/02/148489.php

cc set

niels_dekker — Sun, 22 Feb 2009 19:10:58 GMT

cc niels_address_until_2010-10-10@… added

Committed to the trunk, two days ago: changesets [51355] and [51356]

So far, I haven't seen any regression failure because of these commits, but I'll keep an eye at http://www.boost.org/development/tests/trunk/developer/utility_.html

niels_dekker — Tue, 24 Feb 2009 18:17:34 GMT

Hmmm... My commits ([51355] and [51356]) have caused a compile time regression failure of Intel Version 9.0 Build 20060222Z (see below).

I still have to figure out how to work around this issue. Hopefully the commits don't need to be reverted. Any help is appreciated!

http://www.boost.org/development/tests/trunk/output/Sandia-intel-9-0-boost-bin-v2-libs-utility-test-value_init_test-test-intel-linux-9-0-debug.html says: Compile [2009-02-24 12:19:24 UTC]: fail

"/usr/local/intel/cc/9.0-032/bin/icpc" -c -xc++ -O0 -g -Ob0 -w1 -fPIC -DBOOST_ALL_NO_LIB=1 -I".." -c -o "/var/scratch/boost/results/boost/bin.v2/libs/utility/test/value_init_test.test/intel-linux-9.0/debug/value_init_test.o" "../libs/utility/test/../value_init_test.cpp"

../libs/utility/test/../value_init_test.cpp(258): error: more than one operator "==" matches these operands:

built-in operator "pointer == pointer" built-in operator "pointer == pointer" operand types are: boost::value_initialized<NonPOD *> == NonPOD *const

BOOST_CHECK ( x == z ) ;

detected during instantiation of "bool test(const T &, const T &) [with T=NonPOD *]"

../libs/utility/test/../value_init_test.cpp(265): error: more than one operator "==" matches these operands:

built-in operator "pointer == pointer" built-in operator "pointer == pointer" operand types are: const boost::value_initialized<NonPOD *> == NonPOD *const

BOOST_CHECK ( x_c == z ) ;

detected during instantiation of "bool test(const T &, const T &) [with T=NonPOD *]"

compilation aborted for ../libs/utility/test/../value_init_test.cpp (code 2)

Daniel James — Sat, 27 Jun 2009 12:20:07 GMT

This should either be fixed or reverted. If it's a bug with Intel 9 then it would probably be best to use a BOOST_WORKAROUND macro for the compilers which have this bug. And you'll need to update the version number in the documentation, since 1.39 has the old version.

niels_dekker — Sun, 28 Jun 2009 21:39:15 GMT

Daniel, thanks for your feedback. Yes, I would still like the issue to be fixed. It's now more than four months after committing the fix to the trunk, [51355], and I haven't heard any compaint. (I did announce the fix at the Boost dev mailing list.) Intel 9 is the only compiler on the regression site that doesn't compile implicit conversions from value_initialized<T> to reference-to-T anymore, because of the fix. It's no problem to Intel 10 and 11, and any other compiler included with the regression tests.

There are various possible workarounds for Intel 9:

Do nothing. Intel 9 users can still retrieve a reference to the value_initialized data by using boost::get, or value_initialized::data()
Revert the fix, but only for Intel 9 (using BOOST_WORKAROUND). Leave the fix in there for other compilers.
Offer to Intel 9 users only the const-version of the conversion operator (using BOOST_WORKAROUND)
Offer to Intel 9 users only the non-const-version of the conversion operator (using BOOST_WORKAROUND)

I do prefer workaround "d.", which allows Intel 9 users to convert from value_initialized<T> to T&, but protects const value_initialized<T> objects against such a conversion. As attached: intel_9_value_init_conversion-operator.patch Please let me know if you think this patch is fine to you.

PS Thanks also for reminding me to update the version number in the documentation!

attachment set

niels_dekker — Sun, 28 Jun 2009 21:40:05 GMT

attachment → intel_9_value_init_conversion-operator.patch

Daniel James — Sun, 28 Jun 2009 22:43:56 GMT

I'm really not the person to ask, I'm mainly trying to get some of the 'core' libraries to be consistent on trunk and release in response to some recent mails on the development list.

My normal preference would be to preserve the old behaviour for broken compilers (option b, I think) but the old behaviour looks wrong. I don't know why it was written that way so I don't feel qualified to express a preference. So I'm fine with your patch. No one else seems to care so I would suggest that you go ahead with what you think best. You've already asked for feedback on the development list, so I don't think you have to do it again, unless you feel it's necessary.

Thanks for sorting this out.

niels_dekker — Mon, 29 Jun 2009 18:19:27 GMT

You're welcome, Daniel.

I have a problem with "option b", as it would make the behaviour of value_initialized compiler-dependent. So I sticked with "option d", by applying intel_9_value_init_conversion-operator.patch: changeset [54502]. Hopefully it will lower the number of Intel 9 compile errors from 2 to 1, as the compiler should now be able to accept BOOST_CHECK(x == z). If so, I'll mark the other Intel 9 compile error as an "expected failure", and do a merge to the release branch, when the time is right.

PS I also updated the documentation, as you suggested: changeset [54503]

niels_dekker — Wed, 01 Jul 2009 09:10:38 GMT

So I marked the Intel 9 failure: changeset [54560]

Unfortunately the number of Intel 9 compile errors is not reduced by the workaround I added with changeset [54502]. Instead, it has gone up from 2 to 33! Before committing the workaround, Intel 9 apparently only failed to convert from value_initialized<T> to T when T was a pointer. While the workaround disables conversion from any const value_initialized<T>, for Intel 9. But it does enable conversion from any non-const value_initialized<T>, including when T is a pointer. So there's clearly a trade-off here.

Sandia-intel-9-0-boost-bin-v2-libs-utility-test-value_init_test-test-intel-linux-9-0-debug.html now reports:

Test output: Sandia-intel-9.0 - utility - value_init_test / intel-linux-9.0

Rev 54533 / Tue, 30 Jun 2009 10:33:23 +0000 Report Time: Tue, 30 Jun 2009 20:15:05 +0000

Compile [2009-06-30 12:11:26 UTC]: fail

"/usr/local/intel/cc/9.0-032/bin/icpc" -c -xc++ -O0 -g -w1 -Ob0 -fPIC -DBOOST_ALL_NO_LIB=1 -I".." -c -o "/var/scratch/boost/results/boost/bin.v2/libs/utility/test/value_init_test.test/intel-linux-9.0/debug/value_init_test.o" "../libs/utility/test/../value_init_test.cpp"

../libs/utility/test/../value_init_test.cpp(261): error: no operator "==" matches these operands

operand types are: const int == const boost::value_initialized<int>

BOOST_CHECK ( y == x_c ) ;

detected during instantiation of "bool test(const T &, const T &) [with T=int]"

../libs/utility/test/../value_init_test.cpp(265): error: no operator "==" matches these operands

operand types are: const boost::value_initialized<int> == const int

BOOST_CHECK ( x_c == z ) ;

detected during instantiation of "bool test(const T &, const T &) [with T=int]"

[ snip ]

../libs/utility/test/../value_init_test.cpp(283): error: no operator "==" matches these operands

operand types are: const AggregatePODStructWrapper == const boost::value_initialized<const AggregatePODStructWrapper>

BOOST_CHECK ( y == cx_c ) ;

detected during instantiation of "bool test(const T &, const T &) [with T=AggregatePODStructWrapper]"

compilation aborted for ../libs/utility/test/../value_init_test.cpp (code 2)

niels_dekker — Sat, 04 Jul 2009 12:22:30 GMT

I just asked for feedback from Boost users: [Boost-users] [utility] Anyone here using Intel 9.0 compiler + value_initialized?, http://lists.boost.org/boost-users/2009/07/49498.php

niels_dekker — Thu, 09 Jul 2009 08:17:07 GMT

Fernando Cacciola mailed me that it seems okay to him to just leave implicit conversion from value_initialized<T*> broken for Intel 9.0, instead of having it patched. Which also makes sense to me because it appears a rather rare use case, and it's an old compiler version anyway. So I committed changeset [54832], which reverted intel_9_value_init_conversion-operator.patch

status changed; resolution set

niels_dekker — Sun, 04 Oct 2009 14:19:07 GMT

status assigned → closed
resolution → fixed

FYI, Yesterday I merged the fix from the trunk to the release branch: changeset [56547]. Related merge of value_init_test: Changeset [56543]. I also informed people at the [boost] mailing list: "[boost] [utility] Fix for const issue of value_initialized merged to release", http://lists.boost.org/Archives/boost/2009/10/156798.php

Looks like the fix will be included with Boost 1.41.0 :-)