[Boost-bugs] [Boost C++ Libraries] #6149: as_literal used on string container making unnecessary expensive copy

Subject: [Boost-bugs] [Boost C++ Libraries] #6149: as_literal used on string container making unnecessary expensive copy
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2011-11-19 16:43:26

#6149: as_literal used on string container making unnecessary expensive copy
------------------------------------------------+---------------------------
 Reporter:  Roman Komary <RomanKomary@…>        |       Owner:  neilgroves
     Type:  Bugs                                |      Status:  new       
Milestone:  To Be Determined                    |   Component:  range     
  Version:  Boost 1.48.0                        |    Severity:  Problem   
 Keywords:                                      |  
------------------------------------------------+---------------------------
 The following problem is not only an optimization problem since it can
 slow down applications. It is also not limited to boost 1.48.0. It did
 exist in previous versions already, but is still present in 1.48.0.

 boost/range/as_literal.hpp uses range_detail::is_char_ptr() to distinguish
 if a passed T is a character pointer or convertible to character pointer.

 {{{
 template< class T >
 inline long is_char_ptr( T /* r */ )
 }}}

 is problematic when T is a string container. An expensive copy of the
 passed string object is created and immediately deleted again. You might
 think it should be subject to compiler optimization in release builds. But
 I encountered it at least in a project release build with optimization
 enabled on Visual Studio 2005 Professional, seen in the disassembly.

 A solution should be easy by preventing calling is_char_ptr() directly and
 instead doing an indirection over a template class:

 {{{
 namespace range_detail
 {

 template< class T >
 struct check_char_ptr :
     public boost::integral_constant<
             bool,
             sizeof(boost::range_detail::is_char_ptr( * (T*) 0 )) ==
 sizeof(bool)
            >
 {
     typedef BOOST_DEDUCED_TYPENAME boost::mpl::if_c< value, bool, long
 >::type choice_type;
 };

 }

 template< class Range >
 inline boost::iterator_range< BOOST_DEDUCED_TYPENAME
 boost::range_iterator< Range >::type >
 as_literal( Range& r )
 {
     return boost::range_detail::make_range( r,
 range_detail::check_char_ptr< Range >::choice_type() );
 }

 template< class Range >
 inline boost::iterator_range< BOOST_DEDUCED_TYPENAME
 boost::range_iterator< const Range >::type >
 as_literal( const Range& r )
 {
     return boost::range_detail::make_range( r,
 range_detail::check_char_ptr< Range >::choice_type() );
 }

 template< class Char, std::size_t sz >
 inline boost::iterator_range< Char* >
 as_literal( Char (&r) [sz] )
 {
     return boost::range_detail::make_range( r,
 range_detail::check_char_ptr< Char* >::choice_type() );
 }

 template< class Char, std::size_t sz >
 inline boost::iterator_range< const Char* >
 as_literal( const Char (&r) [sz] )
 {
     return boost::range_detail::make_range( r,
 range_detail::check_char_ptr< Char* >::choice_type() );
 }
 }}}

 That's the solution I am using right now. Neither debug nor release build
 (only checked in VS2005) includes a call to range_detail::is_char_ptr(),
 seen in the disassembly.
 It still is using range_detail::is_char_ptr() to reflect the previous
 behaviour also in terms of convertability of T to a character pointer
 type.

 If the use of mpl is not desirable, it should be easy to write it manually
 with template specialization.

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/6149>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.