5

I'm having a problem which I'm sure is simple to fix but I'm at a loss...

I have a template that performs the following code:

T value     = d;
if ( std::numeric_limits< T >::is_signed )
{
    if ( value < 0 )
    {
        *this += _T( "-" );
        value = -(signed)value;
    }
}

Now for, obvious reasons, GCC is giving me a warning (comparison is always false due to limited range of data type) when this code is compiled for an unsigned type. I fully understand the reasoning behind this and I put in the numeric_limits check to see if I could get the compiler to shut up about it (it worked for MSVC). Alas under GCC I get the warning. Is there any way (short of disabling the warning which I don't even know if you can do with GCC) to fix this warning? The code will never get called anyway and I would assume the optimiser will compile it out as well but I can't get rid of the warning.

Can someone give me a solution to this?

Cheers!

Goz
  • 61,365
  • 24
  • 124
  • 204
  • 2
    This is VERY nasty when `int` is 16 bits. When `value` is a long, `value = -(signed /*int*/) value` truncates larger values. Multiply by `-1` instead, and let the optimizer figure it out. – MSalters Jun 29 '10 at 12:13

4 Answers4

5

Simpler solution:

template <typename T> inline bool isNegative(T value) {
  return std::numeric_limits< T >::is_signed && value < 0; // Doesn't trigger warning.
}

T value     = d;
if ( isNegative(value) ) // Doesn't trigger warning either.
{
    *this += _T( "-" );
    value = -1 * value;
}
MSalters
  • 173,980
  • 10
  • 155
  • 350
  • Unfortunately, while that fixes the warning on gcc, it introduces a new warning on MSVC because `isNegative` never uses the parameter `value` when called with an unsigned type `(warning C4100: 'value' : unreferenced formal parameter)`. Pleasing every compiler is difficult - it might just be better to disable the warning than to write code that compiles cleanly everywhere. – JoeG Jun 29 '10 at 12:37
  • To expand to that; I think every solution you'll find will either cause a compiler warning or else contain convoluted logic which will be more difficult to maintain. – JoeG Jun 29 '10 at 12:40
  • I suppose wrapping the `std::numeric_limits::is_signed` in a function template might remove the warning: `template bool is_signed(const T&){return std::numeric_limits::is_signed;}` - however, if it does so, it (as MSalters' version) does it __at the cost of turning a compile-time check in a run-time check__. In the end I'd probably turn off the warning in VC (`#pragma warn(push : 4100)`, IIRC) right before the offending line, and turn it back on (`#pragma warn(pop)`) afterwards. – sbi Jun 29 '10 at 13:25
  • @sbi: Mine is only theoretically a run-time check. Do the template instantiation in your head to see how it falls out; you end up with either `false && (unsigned)value < 0` or `true && (signed)value < 0`. Obviously the latter has a necessary run-time component. – MSalters Jun 29 '10 at 13:38
  • @Joe: does it help if you change it to `return value < 0 && std::numeric_limits< T >::is_signed;` ? – MSalters Jun 29 '10 at 13:41
  • @MSalters: Of course, compilers might optimize either one. But then they might figure out the call is bogus and give a warning. `:)` BTW, for `value < 0 && std::numeric_limits::is_signed` I smell a "condition always false" warning. – sbi Jun 29 '10 at 13:57
  • using a `*` instead of a `&&` should fix the warning for unsigned types under MSVC – smerlin Jun 29 '10 at 19:51
  • @MSalters: Switching the condition around causes it to compile cleanly at the highest warning level on VC9. – JoeG Jun 30 '10 at 07:25
3

Can someone give me a solution to this?

Nothing revolutionary, but I usually do this by overloading. Something along those lines: 

//Beware, brain-compiled code ahead!
template< bool B >
struct Bool { const static bool result = B; }

template< typename T >
void do_it(T& , Bool<false> /*is_signed*/)
{
  // nothing to do for unsigned types
}

template< typename T >
void do_it(T& value, Bool<true> /*is_signed*/)
{
    if ( value < 0 ) {
        *this += _T( "-" );
        value = -(signed)value;
    }
}

template< typename T >
void do_something(T& value)
{
  do_it(value, Bool<std::numeric_limits< T >::is_signed>() );
}

If you can use class templates instead of function templates, you can use specialization instead of overloading. (There is no function template partial specialization, which makes specializing function templates a bit more of a hassle.)

sbi
  • 219,715
  • 46
  • 258
  • 445
  • A bit overcomplicated - introducing a compile time wrapper around bool and an extra parameter for the call? Also, `value` needs to be passed by non-const reference, as its value gets changed. As it's making use of `this`, `T` is probably a template parameter of the class, not the function. – JoeG Jun 29 '10 at 11:18
  • When I commented, your solution wouldn't compile because you're using a const reference for a mutated value. I'm sorry if you interpret that as my being 'annoyed' with you. You might also note that I didn't down vote your answer. The edit I made to my comment was to add the final sentence. – JoeG Jun 29 '10 at 12:19
  • @Joe: Then I must have misread your comment and I apologize for my response. I removed my comment. (Your comment coincidented with what later turned out to be a removed up-vote, BTW, which at first I took as a down-vote.) Anyway, regarding the `Bool<>` type: I wouldn't dream trying to code C++ without that in my toolbox. It's always there and I just need to grab it when I need it. `:)` – sbi Jun 29 '10 at 13:27
  • This is what i would do too. It's easy and seems the most efficient way to do it. I think you need to swap `true` and `false` though and fix the syntax in the wrapper template's static const. – Johannes Schaub - litb Jun 29 '10 at 17:34
  • @Johannes: Thanks for pointing out these brainfarts. `` At least I already had my standard disclaimer in there... – sbi Jun 29 '10 at 19:15
3

See https://stackoverflow.com/a/8658004/274937 for the real solution, which allows suppressing -Wtype-limits warnings case-by-case. In short, just wrap each comparison, which generates the warning, into a dummy function.

Community
  • 1
  • 1
valyala
  • 11,669
  • 1
  • 59
  • 62
2

You can specialize your function like this:

template <bool S> 
void manipulate_sign(T&) {}

template <> 
void manipulate_sign<true>(T& value) {
  if ( value < 0 )
  {
    *this += _T( "-" );
    value = -(signed)value;
  }
}

//then call like this:
manipulate_sign<std::numeric_limits< T >::is_signed>();

Just disabling the warning might be better though.

JoeG
  • 12,994
  • 1
  • 38
  • 63
  • 1
    I would definitely wrap this function so the user doesn't have to use `numeric_limits` himself - `T` is all the information the function needs :) – Georg Fritzsche Jun 30 '10 at 09:19