[Boost-bugs] [Boost C++ Libraries] #4666: boost::variant gives several warnings (fix suggestions included)

Subject: [Boost-bugs] [Boost C++ Libraries] #4666: boost::variant gives several warnings (fix suggestions included)
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2010-09-21 09:57:38

#4666: boost::variant gives several warnings (fix suggestions included)
---------------------------------------------------+------------------------
 Reporter:  Lieven de Cock <killerbot@…>           |       Owner:  ebf     
     Type:  Bugs                                   |      Status:  new     
Milestone:  To Be Determined                       |   Component:  variant 
  Version:  Boost 1.44.0                           |    Severity:  Cosmetic
 Keywords:                                         |  
---------------------------------------------------+------------------------
 I am using several compiler options with gcc to increase the warning
 level.
 Especially 3 of them are raising warnings :

 1) -Wshadow
 Too warn when a variable is shadowing/hiding another variable/method
 This occurs in boostvariant/variant.hpp

 {{{
 void indicate_which(int which)
     {
         which_ = static_cast<which_t>( which );
     }

     void indicate_backup_which(int which)
     {
         which_ = static_cast<which_t>( -(which + 1) );
     }
 }}}
 the local which is shadowing the member method which.

 My suggestion to fix would be :


 {{{
     void indicate_which(int which_in)
     {
         which_ = static_cast<which_t>( which_in );
     }

     void indicate_backup_which(int which_in)
     {
         which_ = static_cast<which_t>( -(which_in + 1) );
     }

 }}}

 So I just renamed to local to which_in

 2) -Wswitch-default
 warn when a switch statement doesn't have a default case.
 This occurs at : boost/variant/detail/visitation_impl.hpp

 {{{
     // ...switch on the target which-index value...
     switch (logical_which)
     {

     // ...applying the appropriate case:
 #   define BOOST_VARIANT_AUX_APPLY_VISITOR_STEP_CASE(z, N, _) \
     case (Which::value + (N)): \
         return visitation_impl_invoke( \
               internal_which, visitor, storage \
             , static_cast<BOOST_PP_CAT(T,N)*>(0) \
             , no_backup_flag, 1L \
             ); \
     /**/

     BOOST_PP_REPEAT(
           BOOST_VARIANT_VISITATION_UNROLLING_LIMIT
         , BOOST_VARIANT_AUX_APPLY_VISITOR_STEP_CASE
         , _
         )

 #   undef BOOST_VARIANT_AUX_APPLY_VISITOR_STEP_CASE

     }

 }}}

 My suggestion is to add the following just in front of the end brace of
 the switch :

 {{{
     default: break;
 }}}

 3) -Wundef
 warns if an undefined identifier is evaluated in an #if directive
 This occurs at : boost/mpl/has_xxx.hpp
 It occurs 4 times (that I ran into, within all the ifdef stuff it might
 occur a few times more).

 {{{
 #   if !BOOST_MPL_HAS_XXX_NO_EXPLICIT_TEST_FUNCTION
 #     define BOOST_MPL_HAS_MEMBER_REJECT(args, member_macro) \
         template< typename V > \
         static boost::mpl::aux::no_tag \
         BOOST_MPL_HAS_MEMBER_INTROSPECTION_TEST_NAME(args)(...); \
       /**/
 #   else
 #     define BOOST_MPL_HAS_MEMBER_REJECT(args, member_macro) \
         static boost::mpl::aux::no_tag \
         BOOST_MPL_HAS_MEMBER_INTROSPECTION_TEST_NAME(args)(...); \
       /**/
 #   endif

 }}}
 The question is : BOOST_MPL_HAS_XXX_NO_EXPLICIT_TEST_FUNCTION, does it
 need to have a special value (for example : not 0), or is it just a matter
 of beind defined or not.
 In the latter case, this are my suggestions on how to fix :

 line 344 --> #   if not defined
 (BOOST_MPL_HAS_XXX_NO_EXPLICIT_TEST_FUNCTION)
 line 357 --> #   if not defined (BOOST_MPL_HAS_XXX_NO_WRAPPED_TYPES)
 line 386 --> #   if not defined
 (BOOST_MPL_HAS_XXX_NO_EXPLICIT_TEST_FUNCTION)
 line 458 --> #  if defined (BOOST_MPL_HAS_XXX_NEEDS_TEMPLATE_SFINAE)


 What do you think ?

 kind regards,
 Lieven

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