Modify

Ticket #4649 (closed Bugs: fixed)

Opened 4 years ago

Last modified 3 years ago

token_functions.hpp (220) : fix for warning C4127 conditional expression is constant

Reported by: Andrew Macgregor <aamacgregor@…> Owned by: jsiek
Milestone: To Be Determined Component: tokenizer
Version: Boost 1.44.0 Severity: Problem
Keywords: isspace Cc:

Description

MSVC gives a compiler warning for the following piece of code (starting at line 220) in token_functions.hpp:


if (sizeof(char_type) == 1)

return std::isspace(c) != 0;

else

return std::iswspace(c) != 0;


The warning is obvious:

warning C4127: conditional expression is constant

The warning is causing automated project builds to fail when 'fail on warning' and /W4 (maximum warnings) are set.

Please can you make the run-time check for sizeof(char_type)==1 into a compile-time check? A solution could be to change:


if (sizeof(char_type) == 1)

return std::isspace(c) != 0;

else

return std::iswspace(c) != 0;


for something along the lines of


return check_isspace<traits, sizeof(char_type)>::value_for(c);


where check_isspace is defined as follows:


template <typename traits, size_t char_type_size> struct check_isspace {

typedef typename traits::char_type char_type;

static bool value_for(char_type c) { return std::iswspace(c)!=0; }

};

template <typename traits> struct check_isspace<traits, 1> {

typedef typename traits::char_type char_type;

static bool value_for(char_type c) { return std::isspace(c)!=0; }

};


Attachments

token_functions.patch Download (2.0 KB) - added by rvalde@… 3 years ago.
4649-mtc.patch Download (2.0 KB) - added by marshall 3 years ago.
4649-mtc-2.patch Download (2.1 KB) - added by marshall 3 years ago.

Change History

comment:1 Changed 4 years ago by Andrew Macgrego <aamacgregor@…>

I just realised that the problem also affects ispunct too.

Perhaps the following solution?

Add:

#if !defined(BOOST_NO_CWCTYPE)

  struct is_space
  {
    template <typename char_type>
    static bool wchar_func(char_type c) { return std::iswspace(c) != 0; }

    template <typename char_type>
    static bool char_func(char_type c) { return std::isspace(c) != 0; } 
  };

  struct is_punct
  {
    template <typename char_type>
    static bool wchar_func(char_type c) { return std::iswpunct(c) != 0; }

    template <typename char_type>
    static bool char_func(char_type c) { return std::ispunct(c) != 0; } 
  };

  template <typename function_pair, typename traits, size_t char_type_size>
  struct check
  {
    typedef typename traits::char_type char_type;
    static bool value_for(char_type c) { return function_pair::wchar_func(c); }
  };

  template <typename function_pair, typename traits>
  struct check<function_pair, traits, 1>
  {
    typedef typename traits::char_type char_type;
    static bool value_for(char_type c) { return function_pair::char_func(c); }
  };
#endif

And replace:

  if (sizeof(char_type) == 1)
    return std::isspace(c) != 0;
  else
    return std::iswspace(c) != 0;

with

  return check<is_space, traits, sizeof(char_type)>::value_for(c);

and replace

  if (sizeof(char_type) == 1)
    return std::ispunct(c) != 0;
  else
    return std::iswpunct(c) != 0;

with

  return check<is_punct, traits, sizeof(char_type)>::value_for(c);

comment:2 Changed 3 years ago by rvalde@…

Looking to submit a bug for this issue, and I found this one. The issue hasn't been solved for 1.45 yet. I'll attached my solution, for if someone likes it.

Changed 3 years ago by rvalde@…

comment:3 Changed 3 years ago by marshall

I attempted to apply your patch; it did not apply cleanly. I merged it in by hand, and found that clang TOT complained about an extra 'typename'.

With that taken out, it compiles and passes the tests using gcc 4.2.1 and clang TOT.

Adjusted patch attached.

Changed 3 years ago by marshall

comment:4 Changed 3 years ago by anonymous

Thanks Marshall, sorry for the inconveniences caused by a bad patch.

I think the adjusted patch is missing something. There's a remaining const expression "if (sizeof(char)==1)" in traits_extension::ispunct that should be removed.

    static bool ispunct(char_type c)
    {
#if !defined(BOOST_NO_CWCTYPE)
      return traits_extension_details<traits, sizeof(char_type)>::ispunct(c); 
#else
      return static_cast< unsigned >(c) <= 255 && std::ispunct(c) != 0;
#endif
    }

comment:5 Changed 3 years ago by marshall

Well, crud. You are correct. Updated patch attached.

Changed 3 years ago by marshall

comment:6 Changed 3 years ago by marshall

(In [71006]) Applied patch; Refs #4649

comment:7 Changed 3 years ago by marshall

  • Status changed from new to closed
  • Resolution set to fixed

(In [71185]) Merge tokenizer fixes to release. Fixes #4649

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.