[range] as_array broken
Consider the output of the following program: #include <iostream> #include <algorithm> #include <boost/range.hpp> #include <boost/range/as_array.hpp> #include <boost/range/as_literal.hpp> void print_char(char ch) { std::cout << ch << '\t' << (int)ch << '\n'; } template<typename Range> void print_range(Range const &rng) { std::for_each(boost::begin(rng), boost::end(rng), print_char); } int main() { print_range(boost::as_literal("hello")); std::cout << '\n'; print_range(boost::as_array("hello")); std::cout << '\n'; return 0; } It prints the following: h 104 e 101 l 108 l 108 o 111 h 104 e 101 l 108 l 108 o 111 There is no difference between treating a string literal as an array or as a null-terminated string. There should be. When treated as an array, the null-terminator should be treated as any other element in the array. The problem comes from range/detail/implementation_help.hpp: template< class T, std::size_t sz > inline T* array_end( T BOOST_RANGE_ARRAY_REF()[sz], char_or_wchar_t_array_tag ) { return boost_range_array + sz - 1; } It seems Boost.Range is still treating arrays of char and wchar_t specially. IIRC, we decided long ago that it shouldn't. This is on HEAD, BTW. -- Eric Niebler Boost Consulting www.boost-consulting.com
Eric Niebler wrote:
Consider the output of the following program:
It seems Boost.Range is still treating arrays of char and wchar_t specially. IIRC, we decided long ago that it shouldn't.
This is on HEAD, BTW.
Fixed in HEAD. A long time ago we decided there was no time to fix it for 1.34. -Thorsten
Thorsten Ottosen wrote:
Eric Niebler wrote:
Consider the output of the following program:
It seems Boost.Range is still treating arrays of char and wchar_t specially. IIRC, we decided long ago that it shouldn't.
This is on HEAD, BTW.
Fixed
Thanks. You might want to add a test for this as well <nudge>. While we're at it, this trivial code fails to compile on msvc-7.1: int rg[5] = {}; boost::begin(rg); Clearly, it's a compiler bug. But the work-around is simple enough. In range/begin.hpp, the begin() overloads have the following signatures: template< class T > typename range_iterator<T>::type begin( T& r ) template< class T > typename range_iterator<const T>::type begin( const T& r ) If there were defined as follows: template< class T > typename range_mutable_iterator<T>::type begin( T& r ) template< class T > typename range_const_iterator<T>::type begin( const T& r ) ... the problem would go away, I think. Same is true for end(), size(), etc... HTH, -- Eric Niebler Boost Consulting www.boost-consulting.com
Eric Niebler wrote:
Thorsten Ottosen wrote:
Eric Niebler wrote:
Consider the output of the following program:
It seems Boost.Range is still treating arrays of char and wchar_t specially. IIRC, we decided long ago that it shouldn't.
This is on HEAD, BTW. Fixed
Thanks. You might want to add a test for this as well <nudge>.
While we're at it, this trivial code fails to compile on msvc-7.1:
int rg[5] = {}; boost::begin(rg);
Clearly, it's a compiler bug. But the work-around is simple enough. In range/begin.hpp, the begin() overloads have the following signatures:
template< class T > typename range_iterator<T>::type begin( T& r )
template< class T > typename range_iterator<const T>::type begin( const T& r )
If there were defined as follows:
template< class T > typename range_mutable_iterator<T>::type begin( T& r )
template< class T > typename range_const_iterator<T>::type begin( const T& r )
Another(more generic?) workaround seems known. template< class T > typename range_iterator< typename add_const<T>::type >::type some_algo(T const& rng) { // ... } I prefer a macro to document this bug... http://tinyurl.com/y2wkq3 Well, this bug seems not only involved in Boost.Range. VC++7.1 users would need this workaround; everywhere 'T' is deduced as array type. Regards, -- Shunsuke Sogame
shunsuke wrote:
Eric Niebler wrote:
Another(more generic?) workaround seems known.
template< class T > typename range_iterator< typename add_const<T>::type >::type some_algo(T const& rng) { // ... }
I prefer a macro to document this bug... http://tinyurl.com/y2wkq3 Well, this bug seems not only involved in Boost.Range. VC++7.1 users would need this workaround; everywhere 'T' is deduced as array type.
Oh man, I hate that compiler (and I love it too). Shunsuke, may I include your code in the boost distribution? If so, would the range library be an ok place to put it? And is the name PSTADE_DEDUCED_CONST(T) really a good name ? (What about BOOST_RANGE_CONST_TYPE?) -Thorsten
Thorsten Ottosen wrote:
shunsuke wrote:
Eric Niebler wrote:
Another(more generic?) workaround seems known.
template< class T > typename range_iterator< typename add_const<T>::type >::type some_algo(T const& rng) { // ... }
I prefer a macro to document this bug... http://tinyurl.com/y2wkq3 Well, this bug seems not only involved in Boost.Range. VC++7.1 users would need this workaround; everywhere 'T' is deduced as array type.
Oh man, I hate that compiler (and I love it too).
Shunsuke, may I include your code in the boost distribution? Of course you can.
If so, would the range library be an ok place to put it? I don't know for sure. Anyway it's not so complicated macro; wherever it is placed.
And is the name PSTADE_DEDUCED_CONST(T) really a good name ? (What about BOOST_RANGE_CONST_TYPE?) I of course follow your naming; I can't use english well :-)
Please note the workaround is not '::boost::add_const' but 'boost::add_const'. IIRC, the leftmost '::' would confuse VC++7.1 in weird situation. Regards, -- Shunsuke Sogame
Eric Niebler wrote:
Thorsten Ottosen wrote:
This is on HEAD, BTW.
Fixed
Thanks. You might want to add a test for this as well <nudge>.
Will don when I get some more time.
While we're at it, this trivial code fails to compile on msvc-7.1:
int rg[5] = {}; boost::begin(rg);
Clearly, it's a compiler bug. But the work-around is simple enough. In range/begin.hpp, the begin() overloads have the following signatures:
template< class T > typename range_iterator<T>::type begin( T& r )
template< class T > typename range_iterator<const T>::type begin( const T& r )
If there were defined as follows:
template< class T > typename range_mutable_iterator<T>::type begin( T& r )
template< class T > typename range_const_iterator<T>::type begin( const T& r )
.... the problem would go away, I think. Same is true for end(), size(),
Hm ... I don't recall vc7.1 having any problems with this before. AFAICT, it has to do with mpl: ..\..\..\boost\range\iterator.hpp(33) : error C2039: 'type' : is not a member of 'boost::mpl::eval_if_c<C,F1,F2>' with [ C=false, F1=boost::range_const_iterator<boost::remove_const<wchar_t [20]>::ty pe>, F2=boost::range_mutable_iterator<wchar_t [20]> ] -Thorsten
Thorsten Ottosen wrote:
Eric Niebler wrote:
Thorsten Ottosen wrote:
This is on HEAD, BTW. Fixed
Thanks. You might want to add a test for this as well <nudge>.
Will don when I get some more time.
Thanks.
While we're at it, this trivial code fails to compile on msvc-7.1:
int rg[5] = {}; boost::begin(rg);
Clearly, it's a compiler bug. But the work-around is simple enough. In range/begin.hpp, the begin() overloads have the following signatures:
template< class T > typename range_iterator<T>::type begin( T& r )
template< class T > typename range_iterator<const T>::type begin( const T& r )
If there were defined as follows:
template< class T > typename range_mutable_iterator<T>::type begin( T& r )
template< class T > typename range_const_iterator<T>::type begin( const T& r )
.... the problem would go away, I think. Same is true for end(), size(),
Hm ... I don't recall vc7.1 having any problems with this before. AFAICT, it has to do with mpl:
..\..\..\boost\range\iterator.hpp(33) : error C2039: 'type' : is not a member of 'boost::mpl::eval_if_c<C,F1,F2>' with [ C=false, F1=boost::range_const_iterator<boost::remove_const<wchar_t [20]>::ty pe>, F2=boost::range_mutable_iterator<wchar_t [20]> ]
This is not an mpl-specific problem. As Shunsuke correctly observed, it has to do with your use of "T const" in the begin() function's return type, when T is deduced by msvc-7.1 as an array type. Please either try my suggestion above, or Shunsuke's (to use "add_const<T>::type" instead of "T const") to confirm that it does indeed fix the problem. Thanks. -- Eric Niebler Boost Consulting www.boost-consulting.com
participants (3)
-
Eric Niebler -
shunsuke -
Thorsten Ottosen