forcing value to bool 'true' or 'false' (performance warning)

[mop65715]

I'm perusing source code (code I didn't write but I'm now trying to maintain) that has a litany of get and set methods within various message classes. The get methods within these class return - for the most part - enumerated types (and there's lot of flavors) or unsigned short.

The values of the get methods is then stored into a 'bridge'. Trouble is, the bridge class has a mix of unsigned shorts and bool as types. This was done to keep things 'generic'. IOW: The bridge shouldn't care about enumerated types.

For simplicity here's a contrived example:

# include <iostream>
enum Overall_Status
 {
   Overall_Status_UNUSED = 0,
   Overall_Status_PASSED = 1
 } ;
class test {
  unsigned short x ;
public : 
  test( unsigned short x_ ) 
  : x ( x_ ) {}

  Overall_Status GetMethod () {
   return ( static_cast < Overall_Status > ( x )  ) ;
  }
};
int main() {
  test f ( 1 ) ; 
  bool  xx                 = f.GetMethod ( ) ;      //[1]
  unsigned short yy =   f.GetMethod ( ) ;    //[2]
  Overall_Status zz =   f.GetMethod ( ) ;    //[3]
  std::cin.get(); 
}

Alas, I'm now faced with 384 warnings I have to fix. Warnings akin to the one shown above via item [1]. i.e:
warning C4800: 'Overall_Status' : forcing value to bool 'true' or 'false' (performance warning)


My solutions thus far (can't muck with the return types on the messages):
a) use an unsigned short (2 bytes on the implementation - so I use an extra byte - bool is 1 byte on the implementation) -- see [2] above
b) Modify the bridge types to match the types in the message classes -- see [3] above
c) ??????? <not sure>
Thoughts!

[Paul McKenzie]

 bool  xx = (f.GetMethod ( ) == whatever_you_say_means_true)?true:false;

Regards,

Paul McKenzie

[mop65715]

 bool  xx = (f.GetMethod ( ) == whatever_you_say_means_true)?true:false;

Regards,

Paul McKenzie

Good point!..

[Plasmator]

 bool  xx = (f.GetMethod ( ) == whatever_you_say_means_true)?true:false;

That's pretty redundant, no?

[laserlight]

I suppose the ternary operator part would be redundant, but other than that:

bool xx = f.GetMethod() != Overall_Status_UNUSED;

[GCDEF]

Don't use an enum.

Just define them as

const bool Overall_Status_UNUSED = false;
const bool Overall_Status_PASSED = true;

[TheCPUWizard]

As a side note, most of the suggestions (except GCDEF's) are eplicit implementations of the performance degradition that the compilerabout when implicitly implementing identical logic). All they have done is to remove the warning and not address the issue. GCDEF is the only one who has proposed a solution that actually avoids the conditional branching (which does nasty things like flushing pipelines, etc)

[laserlight]

As a side note, most of the suggestions (except GCDEF's) are eplicit implementations of the performance degradition that the compilerabout when implicitly implementing identical logic). All they have done is to remove the warning and not address the issue. GCDEF is the only one who has proposed a solution that actually avoids the conditional branching (which does nasty things like flushing pipelines, etc)

That is true, but I think that GCDEF's suggestion does not address the issue at all, since the enum appears to be required (see the original post on "enumerated types (and there's lot of flavors)") even though it is unnecessary in the example.

[TheCPUWizard]

That is true, but I think that GCDEF's suggestion does not address the issue at all, since the enum appears to be required (see the original post on "enumerated types (and there's lot of flavors)") even though it is unnecessary in the example.

Agreed. I was just was pointing out that the conversion to a boolean will (almost) always involve a comapre and BRANCH operation. In many ways the compiler warning and manual implementation (simply to avoid the warning) provides NO performance advantages, and in some cases may just clutter up code.

The underlying functional desire should be considered and the appropriate action (possibly a design change) taken on a case-by-case basis.

[mop65715]

I suppose the ternary operator part would be redundant, but other than that:

bool xx = f.GetMethod() != Overall_Status_UNUSED;

True but what about the reverse direction. Again contrived example:

class test {
  Overall_Status x ;
public : 
  test( Overall_Status x_ ) 
  : x ( x_ ) {}
  void SetMethod ( Overall_Status x_ )  {
    x = x_ ;
  }
  Overall_Status GetMethod () {
   return ( x ) ;
  }
};

int main( void )
{   
  test f ( Overall_Status_UNUSED ) ; 
  bool xx =  ( f.GetMethod ( ) == Overall_Status_PASSED ) ? true : false;

  f.SetMethod ( ( xx == true ) ? Overall_Status_PASSED : Overall_Status_UNUSED  ) ;   
  
  std::cin.get(); 
}

The call to SetMethod now requires conditional logic.

The underlying functional desire should be considered and the appropriate action (possibly a design change) taken on a case-by-case basis.

I'm starting to think a possible redesign, though my schedule doesn't reflect time for that. The code uses what I'll call - a push/pull model. A message is first pushed into the bridge. A notification is sent via the use of the observer pattern where observers observe/attach to message types (IOW: Observers extract only what of interest to them from the message that was placed into the bridge). Observers in this case pull messages from the bridge. Now I'm thinking out loud here so it's quiet possible this doesn't make sense, in any event, the question: Instead of having matching message** and bridge types, is it possible to make the types in the bridge 'adaptable'? Simply put during updates to the bridge the bridge will take on the message types?

** matching message and bridge types is not feasible because the enumerated types on both sides (see below) aren't the same.
[msg type side a] <===> [bridge] <====> [msg type side b]

[laserlight]

True but what about the reverse direction.

The "reverse direction" may be impossible, since the bool might not provide enough information to decide which of the enumerators to choose. If you really did want to do that without changing the enum to bool, I suppose I might write:

bool xx = f.GetMethod( ) == Overall_Status_PASSED;

f.SetMethod( xx ? Overall_Status_PASSED : Overall_Status_UNUSED );

[rohshall]

 bool  xx = (f.GetMethod ( ) == whatever_you_say_means_true)?true:false;

Regards,

Paul McKenzie

I think this suggestion is spot-on. But assuming the implementors of this bridge object knew what they were doing, I think they were wanting to use bool to represent non-zero enums. So, instead if finding out which enum to match to for all the test classes, I think only this will suffice:

bool xx = f.GetMethod() != 0;

[ranjithan]

For removing these kind of warnings

Replace : 'bool' with 'BOOL' in declaration of the bool variables.

Replace : 'true' with 'TRUE' and 'false' with 'FALSE' values if anywhere in the code, for these variables.

If you declared 'BOOL', then values should be 'TRUE' or 'FALSE' - In These kind of declaration, it convert 'int' to 'bool' without typecasting.

If you declared 'bool' then values should be 'true' or 'false' - In These kind of declaration, if 'int' value is passed to 'bool' variable then you will get warning "forcing value to bool 'true' or 'false' (performance warning)" .


( I tried in VC++ .Net 2003) May be it will work for VC++ Version 6 also.

[Paul McKenzie]

For removing these kind of warnings

Replace : 'bool' with 'BOOL' in declaration of the bool variables.

Replace : 'true' with 'TRUE' and 'false' with 'FALSE' values if anywhere in the code, for these variables.

This is not a portable solution.

My advice, only use BOOL if you're calling a Windows API function, or if you're creating a Windows DLL and you're returning a value or one of your parameter values is BOOL (since "bool" is only a C++ invention).

If you're using BOOL in your own, non-Windows API/DLL, C++ code, rethink what you're doing and use bool instead.

Regards,

Paul McKenzie

[ranjithan]

This is not a portable solution.

My advice, only use BOOL if you're calling a Windows API function, or if you're creating a Windows DLL and you're returning a value or one of your parameter values is BOOL (since "bool" is only a C++ invention).

If you're using BOOL in your own, non-Windows API/DLL, C++ code, rethink what you're doing and use bool instead.

Regards,

Paul McKenzie

you can make use of the operator ! which will return bool. See the code snippet below.

BOOL bResult = FALSE;
...

bool bReturnValue = !!bResult;


Another interesting one is it can be used to reduce code unwanted code blots.
See the following code block.

bool Function()
{
// Member which holds return in bool.
bool bReturn = false;

// Assume FnWhichReturnsBOOL is a function
// which returns BOOL.
if( FALSE != FnWhichReturnsBOOL())
{
// Since its success, set return as true.
bReturn = true;
}

return bReturn;
}

This can be easly re-written as follows

bool Function()
{
BOOL bReturn = FnWhichReturnsBOOL();
return bReturn != FALSE;
}