
Hi, The failures of iterator_range and sub_range.cpp seem to be fixed trivially. Wrap string literals with as_literal. Am I right? Here is a patch. http://svn.boost.org/trac/boost/ticket/1302 Regards, -- Shunsuke Sogame

shunsuke wrote:
The failures of iterator_range and sub_range.cpp seem to be fixed trivially. Wrap string literals with as_literal. Am I right? Here is a patch. http://svn.boost.org/trac/boost/ticket/1302
This patch might fix this particular failure, but I don't think it should be applied, as it only masks the real problem. The failure comes from the fact, that range currently is not able to correctly handle char[] types. For example: str = "hello world" rr = make_iterator_range( str.begin(), str.begin() + 5 ); BOOST_CHECK( rr == "hello" ); This fails because rr (length 5) is compared to a char array (length 6), as the terminating null character is not correctly handled. In boost/range/detail/implementation_help.hpp there is code that should deal with this, but it is commented out: template< class T, std::size_t sz > inline const T* array_end( const T BOOST_RANGE_ARRAY_REF()[sz] ) { /* typedef BOOST_RANGE_DEDUCED_TYPENAME boost::mpl::if_c< is_same<char,T>::value || is_same<wchar_t,T>::value, char_or_wchar_t_array_tag, int >::type tag; return array_end<T,sz>( boost_range_array, tag() ); */ return boost_range_array + sz; } If I enable all the deactivated code in this file, all range tests pass for me on Tru64/CXX. Thorsten, is there a reason that prevents you from enabling the deactivated code? If you lack the time, do you want me the commit the modified header? (See attached patch.) Markus Index: implementation_help.hpp =================================================================== --- implementation_help.hpp (revision 39918) +++ implementation_help.hpp (working copy) @@ -57,7 +57,6 @@ return const_cast<Char*>( str_end( s, s ) ); } - /* template< class T, std::size_t sz > inline T* array_end( T BOOST_RANGE_ARRAY_REF()[sz], int ) { @@ -81,32 +80,25 @@ { return boost_range_array + sz - 1; } - */ template< class T, std::size_t sz > inline T* array_end( T BOOST_RANGE_ARRAY_REF()[sz] ) { - /* typedef BOOST_RANGE_DEDUCED_TYPENAME boost::mpl::if_c< is_same<char,T>::value || is_same<wchar_t,T>::value, char_or_wchar_t_array_tag, int >::type tag; return array_end<T,sz>( boost_range_array, tag() ); - */ - return boost_range_array + sz; } template< class T, std::size_t sz > inline const T* array_end( const T BOOST_RANGE_ARRAY_REF()[sz] ) { - /* typedef BOOST_RANGE_DEDUCED_TYPENAME boost::mpl::if_c< is_same<char,T>::value || is_same<wchar_t,T>::value, char_or_wchar_t_array_tag, int >::type tag; return array_end<T,sz>( boost_range_array, tag() ); - */ - return boost_range_array + sz; } ///////////////////////////////////////////////////////////////////// @@ -119,7 +111,6 @@ return str_end( s ) - s; } - /* template< class T, std::size_t sz > inline std::size_t array_size( T BOOST_RANGE_ARRAY_REF()[sz], int ) { @@ -143,31 +134,24 @@ { return sz - 1; } - */ template< class T, std::size_t sz > inline std::size_t array_size( T BOOST_RANGE_ARRAY_REF()[sz] ) { - /* typedef BOOST_RANGE_DEDUCED_TYPENAME boost::mpl::if_c< is_same<const char,T>::value || is_same<const wchar_t,T>::value || is_same<char,T>::value || is_same<wchar_t,T>::value, char_or_wchar_t_array_tag, int >::type tag; return array_size<T,sz>( boost_range_array, tag() ); - */ - return sz; } template< class T, std::size_t sz > inline std::size_t array_size( const T BOOST_RANGE_ARRAY_REF()[sz] ) { - /* typedef BOOST_RANGE_DEDUCED_TYPENAME boost::mpl::if_c< is_same<char,T>::value || is_same<wchar_t,T>::value, char_or_wchar_t_array_tag, int >::type tag; return array_size<T,sz>( boost_range_array, tag() ); - */ - return sz; } } // namespace 'range_detail'

Markus Schöpflin wrote:
Here is a patch. http://svn.boost.org/trac/boost/ticket/1302
This patch might fix this particular failure, but I don't think it should be applied, as it only masks the real problem.
The failure comes from the fact, that range currently is not able to correctly handle char[] types. For example:
str = "hello world" rr = make_iterator_range( str.begin(), str.begin() + 5 ); BOOST_CHECK( rr == "hello" );
This fails because rr (length 5) is compared to a char array (length 6), as the terminating null character is not correctly handled.
See "Warning" in http://www.boost.org/libs/range/doc/intro.html In Boost1.34(or below), "hello" and (char const *)"hello" is a range whose size is 5. In Boost1.35, "hello" is a range whose size is 6, containing the trailing null. And (char const *)"hello" is no longer a range. Boost1.34(or below) behavior was rejected, because it was different from other array types behavior. BTW, see ticket: http://svn.boost.org/trac/boost/ticket/1309 This is more important for msvc-7.1 users. If these two patches applied, I think all the regressions will pass. Regards, -- Shunsuke Sogame

shunsuke schrieb:
Markus Schöpflin wrote:
Here is a patch. http://svn.boost.org/trac/boost/ticket/1302 This patch might fix this particular failure, but I don't think it should be applied, as it only masks the real problem.
The failure comes from the fact, that range currently is not able to correctly handle char[] types. For example:
str = "hello world" rr = make_iterator_range( str.begin(), str.begin() + 5 ); BOOST_CHECK( rr == "hello" );
This fails because rr (length 5) is compared to a char array (length 6), as the terminating null character is not correctly handled.
See "Warning" in http://www.boost.org/libs/range/doc/intro.html In Boost1.34(or below), "hello" and (char const *)"hello" is a range whose size is 5. In Boost1.35, "hello" is a range whose size is 6, containing the trailing null. And (char const *)"hello" is no longer a range.
Boost1.34(or below) behavior was rejected, because it was different from other array types behavior.
I wasn't aware of this, thanks for the pointer. So probably the test should be patched, and the commented out code removed. Nevertheless I think it's problematic to introduce such a change which makes previously valid code now invalid, especially because it still compiles without any problems, it just changes the observable behavior. And it doesn't fit the principle of least surprise, IMHO. The code above just looks right, and yet it is wrong.
BTW, see ticket: http://svn.boost.org/trac/boost/ticket/1309 This is more important for msvc-7.1 users.
If these two patches applied, I think all the regressions will pass.
Markus
participants (3)
-
Markus Schöpflin
-
Markus Schöpflin
-
shunsuke