views:

129

answers:

3

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!

+3  A: 

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
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.
Joe Gauterin
@sbi - 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.
Joe Gauterin
@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
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
@Johannes: Thanks for pointing out these brainfarts. `<sigh>` At least I already had my standard disclaimer in there...
sbi
+2  A: 

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.

Joe Gauterin
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
+4  A: 

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
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.
Joe Gauterin
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.
Joe Gauterin
I suppose wrapping the `std::numeric_limits<T>::is_signed` in a function template might remove the warning: `template<typename T> bool is_signed(const T}` - 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
MSalters
@Joe: does it help if you change it to `return value < 0 ` ?
MSalters
sbi
smerlin
@MSalters: Switching the condition around causes it to compile cleanly at the highest warning level on VC9.
Joe Gauterin