Best way to eliminate warnings ?

[Gianni Mariani]

I have a couple of template methods that take any integer type however,
the first "if" statement becomes a constant expression when T is an
unsigned type.


#include <limits>

template <typename T>
static T bin_to_gray( const T & value )
{

if ( value >= 0 ) // << warning - "allways true" when T unsigned
{
return value ^ ( value >> 1 );
}
else
{
return ( ( ~value ) ^ ( ( ~value ) >> 1 ) )
+ std::numeric_limits<T>::min();
}
}

Any portable way to eliminate the warning ?

 

[Dan W.]

I have a couple of template methods that take any integer type however,
the first "if" statement becomes a constant expression when T is an
unsigned type.


#include <limits>

template <typename T>
static T bin_to_gray( const T & value )
{

if ( value >= 0 ) // << warning - "allways true" when T unsigned
{
return value ^ ( value >> 1 );
}
else
{
return ( ( ~value ) ^ ( ( ~value ) >> 1 ) )
+ std::numeric_limits<T>::min();
}
}

Any portable way to eliminate the warning ?

How about specializing for unsigned's

template <>
static unsigned long
bin_to_gray<unsigned long>(unsigned long const & val)
{
return val^(val>>1);
}
template <>
static unsigned short
bin_to_gray<unsigned short>(unsigned short const & val)
{
return val^(val>>1)
}

and so on for unsigned char/long long

I'm sure that using a numeric trait would be better, but I haven't
used traits myself before.

cheers!

 

[Alf P. Steinbach]

I have a couple of template methods that take any integer type however,
the first "if" statement becomes a constant expression when T is an
unsigned type.


#include <limits>

template <typename T>
static T bin_to_gray( const T & value )
{

if ( value >= 0 ) // << warning - "allways true" when T unsigned
{
return value ^ ( value >> 1 );
}
else
{
return ( ( ~value ) ^ ( ( ~value ) >> 1 ) )
+ std::numeric_limits<T>::min();
}
}

Any portable way to eliminate the warning ?

The expression in the else-block is not portable, so portability is a moot
issue.

However, you can use std::numeric_limits<T>::is_signed to portably specialize
your function for signed types.

For example (off the cuff),


template< bool, typename T1, typename T2 > struct Choice;
template< typename T1, typename T2 > struct Choice<true, T1, T2>{ typedef T1 T; };
template< typename T1, typename T2 > struct Choice<false, T1, T2>{ typedef T2 T; };



struct SignedType{};
struct UnsignedType{};

template< typename T >
struct Signedness
{
enum{ isSigned = numeric_limits<T>::is_signed };
typedef Choice<isSigned, SignedType, UnsignedType>::T Type;
};

template< class SignednessType > struct GrayConverter;

template<> struct GrayConverter<SignedType>
{
template< typename T >
T to_gray( T value ) ...
};

template<> struct GrayConverter<UnsignedType> ...

template <typename T>
static inline T bin_to_gray( T value )
{
return GrayConverter<Signedness<T>::Type>::to_gray( value );
}


The Choice machinery is for a more readable definition of GrayConverter; you
could alternatively templatize GrayConverter directly on a bool value.

 

[John Ericson]

I have a couple of template methods that take any integer type however,
the first "if" statement becomes a constant expression when T is an
unsigned type.


#include <limits>

template <typename T>
static T bin_to_gray( const T & value )
{

if ( value >= 0 ) // << warning - "allways true" when T unsigned
{
return value ^ ( value >> 1 );
}
else
{
return ( ( ~value ) ^ ( ( ~value ) >> 1 ) )
+ std::numeric_limits<T>::min();
}
}

Any portable way to eliminate the warning ?

How about

template <typename T>
static T bin_to_gray( const T & value )
{

if ( !std::numeric_limits<T>::is_signed || value >= 0 )
//
{
return value ^ ( value >> 1 );
}
else
{
return ( ( ~value ) ^ ( ( ~value ) >> 1 ) )
+ std::numeric_limits<T>::min();
}
}

- -
Best regards, John E.

 

[Gianni Mariani]

The expression in the else-block is not portable, so portability is a moot
issue.

You mean 2's complement ? That's not as big an issue. Any suggestions ?

However, you can use std::numeric_limits<T>::is_signed to portably specialize
your function for signed types.

For example (off the cuff),

Yeah, I was afraid of this.

example snipped.

That's alot of code to fight warnings from *valid code* ... oh well.
The end result is here.

http://www.mariani.ws/~gianni/at_gray_code.h

 

[Gianni Mariani]

John Ericson wrote:
....

How about

template <typename T>
static T bin_to_gray( const T & value )
{

if ( !std::numeric_limits<T>::is_signed || value >= 0 )
//
{
return value ^ ( value >> 1 );
}
else
{
return ( ( ~value ) ^ ( ( ~value ) >> 1 ) )
+ std::numeric_limits<T>::min();
}
}

Yep - I tried that first too. I also tried

std::numeric_limits<T>::is_signed ? value >= 0 : true


I decided to go with the template verbage method

see:
http://www.mariani.ws/~gianni/at_gray_code.h

 

[Pete Becker]

I have a couple of template methods that take any integer type however,
the first "if" statement becomes a constant expression when T is an
unsigned type.

#include <limits>

template <typename T>
static T bin_to_gray( const T & value )
{

if ( value >= 0 ) // << warning - "allways true" when T unsigned
{
return value ^ ( value >> 1 );
}
else
{
return ( ( ~value ) ^ ( ( ~value ) >> 1 ) )
+ std::numeric_limits<T>::min();
}
}

Any portable way to eliminate the warning ?

Turn warnings off. There's nothing wrong with that code, and rewriting
it to satisfy some compiler writer's notion of good style is a waste of
time.

 

[Gianni Mariani]

Gianni Mariani wrote: ....


Turn warnings off. There's nothing wrong with that code, and rewriting
it to satisfy some compiler writer's notion of good style is a waste of
time.

Thanks, but that's not portable.

I'm on the contrary with that style comment, I think this particular
warning is important because it can catch errors.

Leaving warning on is also a problem because if you want to make use of
warning like this one, it will get lost in a sea of other warnings.

 

[Pete Becker]

Thanks, but that's not portable.

Nope. And working around idiosyncratic warnings isn't portable, either:
what one compiler likes another won't. So you have a choice: fight with
your compilers or tell them to shut up.

I'm on the contrary with that style comment, I think this particular
warning is important because it can catch errors.

What errors did it catch in this code? What do you gain from the extra
time you spend working around this warning and the extra code that you
end up writing?

Leaving warning on is also a problem because if you want to make use of
warning like this one, it will get lost in a sea of other warnings.

So shut it off.

 

[lilburne]

Thanks, but that's not portable.

I'm on the contrary with that style comment, I think this particular
warning is important because it can catch errors.

Leaving warning on is also a problem because if you want to make use of
warning like this one, it will get lost in a sea of other warnings.

I'd agree with you.

Warnings like this are indicative of potential problems with
the code, which is why we tend to ratchet up the warning
level of the compilers we use and turn them into compilation
errors.

In some cases the code that is warned about is OK, but it is
still worth spending the time to remove it, particularly if
the warning is coming from stuff in a header file because it
is likely to be generated in every compilation unit that
includes the header. Personally I'd rather not have to deal
with such headers.

 

[Dan W.]

Thanks, but that's not portable.

I'm on the contrary with that style comment, I think this particular
warning is important because it can catch errors.

Leaving warning on is also a problem because if you want to make use of
warning like this one, it will get lost in a sea of other warnings.

I'm just curious about what didn't you like about the solution I
offered you, namely, to write template specializations for unsigned
types. You want it portable? You got it. You want it simple? You got
it.

 

[Gianni Mariani]

Dan W. wrote:
....

I'm just curious about what didn't you like about the solution I
offered you, namely, to write template specializations for unsigned
types. You want it portable? You got it. You want it simple? You got
it.

It violates two of my rules :

"Don't artificially limit the utility of a class (library)".

By enumerating implementations for a particular set of signed or
unsigned types, you are limiting it's use. For example, if I had a
special compiler with 128 bit unsigned values, I would have to augment
my library. I ended up using std::numeric_limits<T> which is less of an
issue but definitly encroaches on the rule.

The tennets of templates is to be generic, I see the reason for
specialization is to remove special cases, not add them.

The other issue is repeated functionality. While this case may be moot,
I feel that repeating "return val^(val>>1)" N times for every unsigned
type if the kind of thing that end up hurting in the future. If I have
a chunk-o-code that I test as an implementation<unsigned short> I would
like to have high confidence that implementation<unsigned long long>
would do the right thing. I've seen too many occurrences of cut-n-paste
code causing untold problems that I fear to tread there. Call this the
"an instance of a functionality must exist in exactly one place" rule.
I break this rule more often that I should and I almost allways find
that I should have never done so. However, I often break this rule
inadvertently, after a high level design, you still have yet to discover
all the "functionality" and sometimes you find yourself implementing the
same function twice. The more experienced engineers I've met, find and
factorize this far sooner than the less experienced ones.

Yes, your code would work fine, it would just be harder to maintain and
we could argue forever on that.

G