I've started working through bimap in preparation for a review, but I'm having a few problems with the source: Gcc on linux, passes all the tests (tried AMD64 and IA64), good :-) Intel 9.1 on Linux, and HP aCC on HP-UX fails to build quite a few tests. These compilers are both based on a recent release of the highly regarded EDG front-end, and may indicate issues with the source. The HP aCC test run produced a 17Mb log file of errors and warnings (!) which is um, rather alarming so I'll contact Matias by private email with the logs. On Win32, everything builds with msvc, but I'm seeing access violations in: test_bimap_assign test_bimap_property_map with all of VC7.1, VC8, Intel 8.1 and 9.1. There are a few warnings as well that look suspicious to me: c:\data\boost\develop\boost\boost\multi_index\detail\ord_index_node.hpp(127) : warning C4312: 'type cast' : conversion from 'boost::multi_index::detail::uintptr_type' to 'void *' of greater size c:\data\boost\develop\boost\boost\multi_index\detail\ord_index_node.hpp(132) : warning C4311: 'type cast' : pointer truncation from 'void *' to 'boost::multi_index::detail::uintptr_type' c:\data\boost\develop\boost\boost\multi_index\detail\ord_index_node.hpp(159) : warning C4312: 'type cast' : conversion from 'const boost::multi_index::detail::uintptr_type' to 'void *' of greater size And finally, the serialization test fails to build with VC8 with: c:\data\boost\bimap\libs\bimap\test\test_bimap_serialization.cpp(53) : error C2872: 'bimap' : ambiguous symbol could be 'c:\data\boost\bimap\boost\bimap\bimap.hpp(149) : boost::bimap::bimap' or 'boost::bimap' c:\data\boost\bimap\libs\bimap\test\test_bimap_serialization.cpp(53) : error C2872: 'bimap' : ambiguous symbol could be 'c:\data\boost\bimap\boost\bimap\bimap.hpp(149) : boost::bimap::bimap' or 'boost::bimap' I note from the docs that these compilers are all listed as supported, so I'm guessing that either recent changes to Bimap, or to Boost cvs have broken something. Anyway I thought I'd bring these up in the hopes that there are some easy fixes, before other reviewers hit the same issues. I'll get back to reading the docs now :-) Regards, John.
On 2/22/07, John Maddock <john@johnmaddock.co.uk> wrote:
I've started working through bimap in preparation for a review, but I'm having a few problems with the source:
I am going to look at this issues and prepare a new version as soon as possible. We can continue with the interface review in the meanwhile. Best Regards Matias
Matias Capeletto wrote:
On 2/22/07, John Maddock <john@johnmaddock.co.uk> wrote:
I've started working through bimap in preparation for a review, but I'm having a few problems with the source:
I am going to look at this issues and prepare a new version as soon as possible. We can continue with the interface review in the meanwhile.
Thanks Matias, I don't think these are showstoppers, but I wanted to flag them up before too many folks stumbled over them. John.
John Maddock ha escrito:
I've started working through bimap in preparation for a review, but I'm having a few problems with the source:
[...]
There are a few warnings as well that look suspicious to me:
c:\data\boost\develop\boost\boost\multi_index\detail\ord_index_node.hpp(127) : warning C4312: 'type cast' : conversion from 'boost::multi_index::detail::uintptr_type' to 'void *' of greater size c:\data\boost\develop\boost\boost\multi_index\detail\ord_index_node.hpp(132) : warning C4311: 'type cast' : pointer truncation from 'void *' to 'boost::multi_index::detail::uintptr_type' c:\data\boost\develop\boost\boost\multi_index\detail\ord_index_node.hpp(159) : warning C4312: 'type cast' : conversion from 'const boost::multi_index::detail::uintptr_type' to 'void *' of greater size
Are you using /Wp64? If so, I think the warnings are a case of the compiler being too dumb about 32-to-64 correctness checks, please see the first part of: http://lists.boost.org/boost-users/2006/03/18075.php If you're not using /Wp64 please let me know. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
Joaquín Mª López Muñoz wrote:
Are you using /Wp64? If so, I think the warnings are a case of the compiler being too dumb about 32-to-64 correctness checks, please see the first part of:
http://lists.boost.org/boost-users/2006/03/18075.php
If you're not using /Wp64 please let me know.
Ah, yes that would be it, apologies for the noise. Would it be an idea to suppress these with some #pragma's if they've been investigated and found harmless? Cheers, John.
John Maddock ha escrito:
Joaquín Mª López Muñoz wrote:
Are you using /Wp64? If so, I think the warnings are a case of the compiler being too dumb about 32-to-64 correctness checks, please see the first part of:
http://lists.boost.org/boost-users/2006/03/18075.php
If you're not using /Wp64 please let me know.
Ah, yes that would be it, apologies for the noise.
Would it be an idea to suppress these with some #pragma's if they've been investigated and found harmless?
It can be a good idea, but I need to make sure that #pragma warning(push) and #pragma warning(pop) can be used here in such a manner as to not suppress this otherwise useful warning in user code. Would you volunteer to try some tests in your environment (I don't have VC7.1 or 8.0) before commiting such a change? Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
Joaquín Mª López Muñoz ha escrito:
John Maddock ha escrito:
Joaquín Mª López Muñoz wrote:
Are you using /Wp64? If so, I think the warnings are a case of the compiler being too dumb about 32-to-64 correctness checks, please see the first part of:
http://lists.boost.org/boost-users/2006/03/18075.php
If you're not using /Wp64 please let me know.
Ah, yes that would be it, apologies for the noise.
Would it be an idea to suppress these with some #pragma's if they've been investigated and found harmless?
It can be a good idea, but I need to make sure that #pragma warning(push) and #pragma warning(pop) can be used here in such a manner as to not suppress this otherwise useful warning in user code. Would you volunteer to try some tests in your environment (I don't have VC7.1 or 8.0) before commiting such a change?
I mean tests that I'd send to you, don't want you to write the modification yourself, or course. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
Joaquín Mª López Muñoz wrote:
Would it be an idea to suppress these with some #pragma's if they've been investigated and found harmless?
It can be a good idea, but I need to make sure that #pragma warning(push) and #pragma warning(pop) can be used here in such a manner as to not suppress this otherwise useful warning in user code. Would you volunteer to try some tests in your environment (I don't have VC7.1 or 8.0) before commiting such a change?
How about the patch below then - turns off the warnings only for your code. Note that the suppression of the std::equal warning will suppress the warning for any further uses of that template instance as well (since a template is instantiated only once). John. $ cvs diff -u boost/multi_index johnmaddock@boost.cvs.sourceforge.net's password: cvs diff: Diffing boost/multi_index Index: boost/multi_index/ordered_index.hpp =================================================================== RCS file: /cvsroot/boost/boost/boost/multi_index/ordered_index.hpp,v retrieving revision 1.14 diff -u -r1.14 ordered_index.hpp --- boost/multi_index/ordered_index.hpp 19 May 2006 06:51:20 -0000 1.14 +++ boost/multi_index/ordered_index.hpp 23 Feb 2007 10:04:46 -0000 @@ -1092,7 +1092,15 @@ const ordered_index<KeyFromValue1,Compare1,SuperMeta1,TagList1,Category1>& x, const ordered_index<KeyFromValue2,Compare2,SuperMeta2,TagList2,Category2>& y) { +#ifdef BOOST_MSVC +// suppress std lib security warning: +#pragma warning(push) +#pragma warning(disable:4996) +#endif return x.size()==y.size()&&std::equal(x.begin(),x.end(),y.begin()); +#ifdef BOOST_MSVC +#pragma warning(pop) +#endif } template< cvs diff: Diffing boost/multi_index/detail Index: boost/multi_index/detail/ord_index_node.hpp =================================================================== RCS file: /cvsroot/boost/boost/boost/multi_index/detail/ord_index_node.hpp,v retrieving revision 1.7 diff -u -r1.7 ord_index_node.hpp --- boost/multi_index/detail/ord_index_node.hpp 19 May 2006 06:48:22 -00001.7 +++ boost/multi_index/detail/ord_index_node.hpp 23 Feb 2007 10:04:47 -0000 @@ -120,6 +120,14 @@ struct parent_ref { +#ifdef BOOST_MSVC +// This code casts pointers to an integer type that has been +// computed to be large enough to hold the pointer, however +// the metaprogramming logic isn't spotted by the VC++ code +// analyser that issues a long list of warnings: +#pragma warning(push) +#pragma warning(disable:4312 4311) +#endif parent_ref(uintptr_type* r_):r(r_){} operator ordered_index_node_impl*()const @@ -158,6 +166,9 @@ { return (ordered_index_node_impl*)(void*)(parentcolor_&~uintptr_type(1)); } +#ifdef BOOST_MSVC +#pragma warning(pop) +#endif ordered_index_node_impl*& left(){return left_;} ordered_index_node_impl* left()const{return left_;}
John Maddock ha escrito:
Joaquín Mª López Muñoz wrote:
Would it be an idea to suppress these with some #pragma's if they've been investigated and found harmless?
It can be a good idea, but I need to make sure that #pragma warning(push) and #pragma warning(pop) can be used here in such a manner as to not suppress this otherwise useful warning in user code. Would you volunteer to try some tests in your environment (I don't have VC7.1 or 8.0) before commiting such a change?
How about the patch below then - turns off the warnings only for your code. Note that the suppression of the std::equal warning will suppress the warning for any further uses of that template instance as well (since a template is instantiated only once).
Oh, you already did the job! Thank you very much. I applied your C4312/C4311 patch, I'd be grateful if you can double check that eveything's OK now. As for the other C4996 patch, I decided *not* to use it because: 1. I'd have to apply it at numerous other locations where std::equal is used (even though as you say one pragma serves all uses, it might be the case that boost/multi_index/ordered_index.hpp is not included in a particular program.) 2. This is a manifestation of the infamous C++ "safe mode". The position of boost.org wrt to this rather irritating feature is to explicitly *not* support it and urge the user to avoid it by defining _SCL_SECURE_NO_DEPRECATE, as exposed in the "Supported Compilers" section of http://boost.org Thank you, Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
participants (3)
-
Joaquín Mª López Muñoz -
John Maddock -
Matias Capeletto