Main Content

Expensive logical operation

A logical operation requires the evaluation of both operands because of their order, resulting in inefficient code

Since R2021a

Description

This defect occurs when all of these conditions are true:

  • Left and right operands have no side effects.

  • The right operand does not contain any calls to const member functions.

  • The left operand contains one or more calls to const member functions.

When assessing possible side effects of an operand:

  • Polyspace® assumes that const member functions of a class do not have side effects. Nonmember functions are assumed to have side effects.

  • Polyspace treats floating-point operations in accordance to the C++ standard. In C++03 or earlier, floating-point operations have no side effects. In C++11 or later, floating-point operations might have side effects, such as modifying the floating-point status flags to indicate abnormal results or auxiliary information. See Floating-point environment.

  • Polyspace treats the bool conversion operator and logical NOT operators of a struct or a class as built-in operators. These operations are not treated as member function calls. The standard template library contains many classes that define such a bool conversion operator or a logical NOT operator.

Risk

When evaluating logical operation, the compiler evaluates the left argument first, and then evaluates the right argument only when necessary. In a logical operation, it is inefficient to put function calls as the left argument while putting constant and variables as the right argument. Consider this code:

if(Object.attribute()|| var1){
//...
}
In the logical expression inside the if statement, the compiler always evaluates the function call Object.attribute(). Evaluating the function is not always necessary. For instance, if var1 evaluates to true, then the logical expression always evaluates to true. Because var1 is the right operand, not the left operand, the compiler unnecessarily evaluates a function call, which is inefficient. Because the inefficient code compiles and behaves correctly, this defect might go unnoticed.

Fix

To fix this defect, flip the order of the operands in a logical expression if the left operand does not perform an operation that must be performed before the right operand in order to evaluate the right operand safely and correctly.

If this condition is not true, then the code relies on the exact order in which the compiler evaluates the flagged logical expression. The best practice is not to rely on the evaluation order of an expression. Consider refactoring your code so that the order of evaluation has no impact. If refactoring the code is not possible, justify the defect by using annotation or review information. See:

Performance improvements might vary based on the compiler, library implementation, and environment that you are using.

Examples

expand all

#include <string>
bool updateNewProperty( const std::string& name );
void updateNewMetaProperty( const std::string& name );
volatile char externalFlag;

void updateProperty( const std::string& name )
{
	bool is_new_property = updateNewProperty( name );

	if( name.compare( "meta" ) == 0 && is_new_property ) 
	{
		updateNewMetaProperty( name );
	}
	if( name.compare( "meta" ) == 0 && externalFlag ) 
	{
		updateNewMetaProperty( name );
	}
	
}

In the first if statement, the variable is_new_property is the right operand of a logical operand. The member function std::string::compare is called as the left argument. The compiler evaluates the function call regardless of the value of is_new_property, which is inefficient. Polyspace flags the logical operation.

In the second if statement, the volatile variable externalFlag is the right operand. Because the variable is volatile, Polyspace assumes it might cause a side effect. Because the volatile variable might have a side effect, Polyspace does not flag the logical operation.

Correction

Determine if the order of the operands needs to be maintained to evaluate the expression safely and correctly. In this case, the two operands is_new_property and name.compare( "meta" ) == 0 are independent and changing their order does not change the value of the logical expression. To fix this defect, use is_new_property as the left operand.

#include <string>
bool updateNewProperty( const std::string& name );
void updateNewMetaProperty( const std::string& name );
volatile char externalFlag;

void updateProperty( const std::string& name )
{
	bool is_new_property = updateNewProperty( name );
	if(is_new_property && name.compare( "meta" ) == 0 ) 
	{
		updateNewMetaProperty( name );
	}
	if( name.compare( "meta" ) == 0 && externalFlag ) 
	{
		updateNewMetaProperty( name );
	}
}

The compiler evaluates the call to std::string::compare in the first if statement only when is_new_property is true.

When you use floating-point operations in a logical expression, Polyspace estimates the side effects of the operands differently based on the version of C++ that you use. In C++03 or earlier versions, floating-point operations do not have any side effects by themselves. In C++11 or later, floating-point operations themselves might have side effects.

class A{
	//...
	float makeFloat() const{
		//..
	}
	void testfloat(){
		if( makeFloat() == 0.1f && fp==0.2f) 
		{
			//...
		}
	}
	
private:
	float fp;		
};
In this code, if you use C++03, neither of the operands has side effects. Because the left operand invokes a member function call, Polyspace flags the expression.

If you use C++11 or later, the floating-point operations might have side effects. In this case, Polyspace does not flag the logical expression.

Correction

Determine if the order of the operands needs to be maintained to evaluate the expression safely and correctly. In this case, the two operands fp==0.2f and makeFloat() == 0.1f are independent and changing their order does not change the value of the logical expression. To fix this defect, use fp==0.2f as the left operand.

class A{
	//...
	float makeFloat() const{
		//..
	}
	void testfloat(){
		if( fp==0.2f && makeFloat() == 0.1f) 
		{
			//...
		}
	}
	
private:
	float fp;		
};
The compiler evaluates the call to makeFloat() only when fp==0.2f evaluates to true.

#include<cstdlib>
class A{
	//...
	bool isLoaded() const { return p != NULL; }
	int get() {
		if(isLoaded() && *p > 0) { 
			return *p;
		}
	}

	
private:
	int* p;		
};

In the expression (isLoaded() && *p > 0), the dereferencing of *p in the right argument is safe only when the left argument is true. Polyspace does not check when logical expression require such specific evaluation order. Because neither operands have side effects and the member function call is the left operand, Polyspace flags the operation.

Correction

In this case, the order if the operands in the logical expression needs to be maintained to evaluate the expression safely and correctly. To fix this defect, refactor your code. The best practice is not to rely on the order of evaluation of an expression.

#include<cstdlib>
class A{
	//...
	bool isLoaded() const { return p != NULL; }
	int get() {
		if(isLoaded()== true) { 
			if(*p > 0){
				return *p;	
			}
			
		}
	}

	
private:
	int* p;		
};
This code checks the two conditions separately and does not rely on the order of evaluation. If such refactoring is not feasible, justify the defect by using annotations or review information.

Result Information

Group: Performance
Language: C++
Default: Off
Command-Line Syntax: EXPENSIVE_LOGICAL_OPERATION
Impact: Low

Version History

Introduced in R2021a