5. Expressions
5.1 Primary Expressions
5.1.1 Use symbolic names instead of literal values in code
Use of “magic” numbers and strings in expressions should be avoided in preference to constant variables with meaningful names. The use of named constants improves both the readability and maintainability of the code.
#include <iostream> #include <cstdint> namespace { const int32_t MAX_ITERATIONS (10); const char * const LOOP_ITER_S ("iter "); const char SEP_C (':'); } void foo () { for (int32_t i = 0 ; i < 10; ++i) // @@- Non-Compliant -@@ { std::cout << "iter " << i << ':' << std::endl; // @@- Non-Compliant -@@ // ... } for (int32_t i = 0 ; i < MAX_ITERATIONS; ++i) // @@+ Compliant +@@ { std::cout << LOOP_ITER_S << i << SEP_C << std::endl; // @@+ Compliant +@@ // ... } }
See 7.4: "Linkage specifications" for guidance on where to declare such symbolic names.
References
- HIC++ v3.3 - 10.1
5.1.2 Do not rely on the sequence of evaluation within an expression
To enable optimizations and parallelization, the C++ standard uses a notion of sequenced before, e.g.:
- Evaluation of a full expression is sequenced before the next full-expression to be evaluated.
- Evaluation of operands of an operator are sequenced before the evaluation of the operator.
- Evaluation of arguments in a function call are sequenced before the execution of the called function.
- For built-in operators &&, ||, , and operator ? evaluation of the first operand is sequenced before evaluation of the other operand(s).
This defines a partial order on evaluations, and where two evaluations are unsequenced with respect to one another, their execution can overlap. Additionally, two evaluations may be indeterminately sequenced, which is similar, except that the execution cannot overlap. This definition leaves great latitude to a compiler to re-order evaluation of sub-expressions, which can lead to unexpected, and even undefined behavior. For this reason, and to improve code readability an expression should not:
- Have more than one side effect
- Result in the modification and access of the same scalar object
- Include a sub-expression that is an assignment operation
- Include a sub-expression that is a pre- or post-increment/decrement operation
- Include a built-in comma operator (for overloaded comma operator see rule <hicpp ref=”over.oper.no-special-semantics”/>)
#include <cstdint> int32_t foo (int32_t i, int32_t j) { int32_t k = ++i + ++j; // @@- Non-Compliant: two side effects in full expression -@@ k = ++i; // @@- Non-Compliant: pre-increment as a sub-expression -@@ ++i; // @@+ Compliant: pre-increment as an expression statement +@@ ++j; // @@+ Compliant +@@ k = i + j; // @@+ Compliant +@@ k = i + ++i; // @@- Non-Compliant: undefined behavior -@@ if (k = 0) // @@- Non-Compliant: assignment as a condition expression -@@ { } return i, j; // @@- Non-Compliant: built-in comma operator used -@@ }
References
- HIC++ v3.3 – 10.3
- HIC++ v3.3 – 10.5
- HIC++ v3.3 – 10.19
- JSF AV C++ Rev C – 204
- MISRA C++:2008 – 5-2-10
5.1.3 Use parentheses in expressions to specify the intent of the expression
The effects of precedence and associativity of operators in a complicated expression can be far from obvious. To enhance readability and maintainability of code, no reliance on precedence or associativity in an expression should be made, by using explicit parentheses, except for:
- Operands of assignment
- Any combination of + and – operations only
- Any combination of * and / operations only
- Sequence of && operations only
- Sequence of || operations only
#include <cstdint> int32_t foo (int32_t i, int32_t j) { int32_t k; k = i + j; // @@+ Compliant +@@ int32_t r = i + j * k; // @@- Non-Compliant -@@ r = i + j + k; // @@+ Compliant +@@ // @@+ Compliant +@@ if ((i != 0) && (j != 0) && (k != 0)) { } // @@- Non-Compliant -@@ if ((i != 0) && (j != 0) || (k != 0)) { } // @@+ Compliant +@@ if (((i != 0) && (j != 0)) || (k != 0)) { } return i + j + k; // @@+ Compliant +@@ }
References
- HIC++ v3.3 – 10.4
5.1.4 Do not capture variables implicitly in a lambda
Capturing variables helps document the intention of the author. It also allows for different variables to be captured by copy and captured by reference within the same lambda.
#include <cstddef> #include <vector> #include <algorithm> void foo (std::vector<size_t> const & v) { size_t sum = 0; std::for_each(v.cbegin () , v.cend () , [&](size_t s) { sum += s; } ); // @@- Non-Compliant -@@ sum = 0; std::for_each(v.cbegin () , v.cend () , [&sum](size_t s) { sum += s; } ); // @@+ Compliant +@@ }
Exception
It is not necessary to capture objects with static storage duration or constants that are not ODR used.
However, the use of objects with static storage duration should be avoided. See Rule 3.3.1: ”Do not use variables with static storage duration”.
For example:
# include <cstddef> # include <cstdint> void foo () { const size_t N = 10; static int32_t j = 0; // Non-Compliant: object with static storage duration []( size_t s) { int32_t array [N]; // Compliant: Not ODR used ++j; // Compliant }; }
5.1.5 Include a (possibly empty) parameter list in every lambda expression
The lambda-declarator is optional in a lambda expression and results in a closure that can be called without any parameters. To avoid any visual ambiguity with other C++ constructs, it is recommended to explicitly include ( ), even though it is not strictly required.
#include <cstdint> int32_t i; int32_t j; void foo () { int32_t a[] { ++i, ++j } ; // Not a lambda [] { ++i, ++j ;} ; // @@- Non-Compliant -@@ [] () { ++i, ++j ;} ; // @@+ Compliant +@@ }
5.1.6 Do not code side effects into the right-hand operands of: &&, ||, sizeof, typeid or a function passed to condition_variable::wait
For some expressions, the side effect of a sub-expression may not be evaluated at all or can be conditionally evaluated, that is, evaluated only when certain conditions are met. For Example:
- The right-hand operands of the && and || operators are only evaluated if the left hand operand is true and false respectively.
- The operand of sizeof is never evaluated.
- The operand of typeid is evaluated only if it is a function call that returns reference to a polymorphic type.
Having visible side effects that do not take place, or only take place under special circumstances makes the code harder to maintain and can also make it harder to achieve a high level of test coverage.
#include <typeinfo> bool doSideAffect(); class C { public: virtual ~C(); // polymorphic class }; C& foo(); void foo( bool condition ) { {} if (true || doSideAffect ()) // @@- Non-Compliant: doSideAffect not called -@@ {} sizeof (doSideAffect()); // @@- Non-Compliant: doSideAffect not called -@@ typeid (doSideAffect()); // @@- Non-Compliant: doSideAffect not called -@@ typeid (foo()); // @@- Non-Compliant: foo called to determine -@@ // @@- the polymorphic type -@@ }
Conditional Variable: wait member
Every time a waiting thread wakes up, it checks the condition. The wake-up may not necessarily happen in direct response to a notification from another thread. This is called a spurious wake. It is indeterminate how many times and when such spurious wakes happen. Therefore it is advisable to avoid using a function with side effects to perform the condition check.
#include <mutex> #include <condition_variable> #include <cstdint> std::mutex mut; std::condition_variable cv; int32_t i; bool sideEffects() { ++i; return (i > 10); } void threadX() { i = 0; std::unique_lock<std::mutex> guard(mut); cv.wait(guard, sideEffects); // @@- Non-Compliant -@@ // @@- value of i depends on the number of wakes -@@ }
References
- HIC++ v3.3 – 10.9
5.2 Postfix Expressions
5.2.1 Ensure that pointer or array access is demonstrably within bounds of a valid object
Unlike standard library containers, arrays do not benefit from bounds checking. Array access can take one of the equivalent forms: *(p + i) or p[i], and will result in undefined behavior, unless p and p + i point to elements of the same array object. Calculating (but not dereferencing) an address one past the last element of the array is well defined also. Note that a scalar object can be considered as equivalent to an array dimensioned to 1. To avoid undefined behavior, appropriate safeguards should be coded explicitly (or instrumented by a tool), to ensure that array access is within bounds, and that indirection operations (*) will not result in a null pointer dereference.
#include <cassert> #include <cstdint> void foo (int32_t* i) { int32_t k = *i; // @@- Non-Compliant: foo could be called with a null pointer -@@ assert (i != nullptr); k = *i; // @@+ Compliant +@@ int32_t a [10]; for (int32_t i (0); i < 10; ++i) { a [i] = i; // @@+ Compliant, array index is 0..9 +@@ } int32_t * p = & (a [10]); // @@+ Compliant: calculating one past the end of array +@@ k = *p; // @@- Non-Compliant: out of bounds access -@@ }
References
- HIC++ v3.3 – 10.2
5.2.2 Ensure that functions do not call themselves, either directly or indirectly
As the program stack tends to be one of the more limited resources, excessive use of recursion may limit the scalability and portability of the program. Tail recursion can readily be replaced with a loop. Other forms of recursion can be replaced with an iterative algorithm and worklists.
#include <cstdint> int32_t a (int32_t); int32_t b (int32_t); int32_t c (int32_t); int32_t d (int32_t); int32_t e (int32_t); int32_t f (int32_t); int32_t g (int32_t); int32_t h (int32_t); int32_t foo (int32_t v) { if (a (v)) { return e (v); } else if (b (v)) { return foo (f (v)); // @@- Non-Compliant: tail recursion -@@ } else if (c (v)) { return g (v); } else if (d (v)) { return foo (h (v)); // @@- Non-Compliant: tail recursion -@@ } } // @@+ Compliant: equivalent algorithm +@@ int32_t bar (int32_t v) { for (;;) { if (a (v)) { v = e (v); break; } else if (b (v)) { v = f (v); } else if (c (v)) { v = g (v); break; } else if (d (v)) { v = h (v); } } return v; }
References
- JSF AV C++ Rev C – 119
5.3 Unary Expressions
5.3.1 Do not apply unary minus to operands of unsigned type
The result of applying a unary minus operator (-) to an operand of unsigned type (after integral promotion) is a value that is unsigned and typically very large. Prefer to use the bitwise complement (~) operator instead.
#include <cstdint> void foo () { uint32_t v; v = -1u; // @@- Non-Compliant -@@ v = ~0u; // @@+ Compliant +@@ }
References
- HIC++ v3.3 – 10.21
5.3.2 Allocate memory using new and release it using delete
C style allocation is not type safe, and does not invoke constructors or destructors. For this reason only operators new and delete should be used to manage objects with dynamic storage duration. Note: Invoking delete on a pointer allocated with malloc or invoking free on a pointer allocated with new will result in undefined behavior.
#include <cstdlib> #include <cstdint> void foo () { // @@- Non-Compliant -@@ int32_t * i = static_cast <int32_t *> (std::malloc (sizeof (int32_t))); std::free (i); } void bar () { // @@+ Compliant +@@ int32_t * i = new int32_t; delete i; }
References
- HIC++ v3.3 – 1.3.1
- Use RAII for resources – 3.4.3
5.3.3 Ensure that the form of delete matches the form of new used to allocate the memory
The C++ standard requires that the operand to the delete operator is either:
- A null pointer.
- Pointer to a non array object allocated with new.
- Pointer to a base class3 subobject of a non array object allocated with new.
Similarly, the operand to the delete[] operator is either:
- A null pointer.
- Pointer to an array object allocated with new[].
- In order to avoid undefined behavior, plain and array forms of delete and new should not be mixed.
#include <cstdint> void foo () { int32_t * i = new int32_t [10]; // ... delete i; // @@- Non-Compliant -@@ } typedef int32_t ARRAY [10]; void bar () { int32_t * i = new ARRAY; delete i; // @@- Non-Compliant -@@ }
References
- HIC++ v3.3 – 12.3
3 containing a virtual destructor
5.4 Explicit Type Conversion
5.4.1 Only use casting forms: static_cast (excl. void*), dynamic_cast or explicit constructor call
All casts result in some degree of type punning, however, some casts may be considered more error-prone than others:
- It is undefined behavior for the result of a static_cast to void* to be cast to any type other than the original from type.
- Depending on the type of an object, casting away const or volatile and attempting to write to the result is undefined behavior.
- Casts using reinterpret_cast are generally unspecified and/or implementation defined. Use of this cast increases the effort required to reason about the code and reduces its portability.
- Simplistically, a C-style cast and a non class function style cast can be considered as a sequence of the other cast kinds. Therefore, these casts suffer from the same set of problems. In addition, without a unique syntax, searching for such casts in code is extremely difficult.
#include <cstdint>
void bar (int8_t);
void foo (const int32_t * p)
{
int32_t * r = const_cast <int32_t *> (p); // @@- Non-Compliant: casting away const -@@
int32_t i = reinterpret_cast <int32_t> (r); // @@- Non-Compliant -@@
i = (int32_t) r; // @@- Non-Compliant: C-style cast -@@
bar (int8_t (i)); // @@- Non-Compliant: function style cast -@@
bar (static_cast <int8_t> (i)); // @@+ Compliant +@@
}
class Base
{
public:
virtual ~Base ();
};
class Derived : virtual public Base
{
public:
Derived (int32_t);
};
void foo (Base * base)
{
Derived * d;
d = reinterpret_cast<Derived *> (base); // @@- Non-Compliant -@@
d = dynamic_cast<Derived *> (base); // @@+ Compliant +@@
auto d2 = Derived (0); // @@+ Compliant: Explicit constructor call +@@
}
References
- Do not convert from a base class to a derived class – 5.4.3
- HIC++ v3.3 – 7.1
- HIC++ v3.3 – 7.3
- HIC++ v3.3 – 7.4
- HIC++ v3.3 – 7.5
- HIC++ v3.3 – 7.7
- HIC++ v3.3 – 13.7
- MISRA C++:2008 – 5-2-2
- MISRA C++:2008 – 5-2-7
- MISRA C++:2008 – 5-2-8
- MISRA C++:2008 – 5-2-9
5.4.2 Do not cast an expression to an enumeration type
The result of casting an integer to an enumeration type is unspecified if the value is not within the range of the enumeration. This also applies when casting between different enumeration types. For this reason, conversions to an enumeration type should be avoided.
enum Colors { RED, GREEN = 2, BLUE }; void bar() { Colors color = static_cast (1000); // @@- Non-Compliant: -@@ if (1000 == color) // @@- may be false -@@ { } } void foo() { Colors color = static_cast (1); // @@- Non-Compliant -@@ switch (color) // @@- the value is unspecified -@@ { case RED: case GREEN: case BLUE: break; default: break; } }
References
- HIC++ v3.3 – 15.4
5.4.3 Do not convert from a base class to a derived class
The most common reason for casting down an inheritance hierarchy is to call derived class methods on an object that is a reference or pointer to the base class. Using a virtual function removes the need for the cast completely and improves the maintainability of the code.
class A { public: virtual void bar(); }; class B : public A { public: void bar() override; void foo(); }; void foo (A* a) { (dynamic_cast<B*> (a))->foo(); // @@- Non-Compliant -@@ a->bar(); // @@+ Compliant +@@ }
Where the cast is unavoidable, dynamic_cast should be used in preference to static_cast as the compiler will check the validity of the cast at runtime.
References
- Only use casting forms: static cast (excl. void*), dynamic cast or explicit constructor call – 5.4.1
- HIC++ v3.3 – 3.3.3
5.5 Multiplicative Operators
5.5.1 Ensure that the right hand operand of the division or remainder operators is demonstrably non-zero
The result of integer division or remainder operation is undefined if the right hand operand is zero. Therefore, appropriate safeguards should be coded explicitly (or instrumented by a tool) to ensure that division by zero does not occur.
#include <cstdint> #include <cassert> int32_t doDivide(int32_t number, int32_t divisor) { assert (0 != divisor); return number / divisor; }
References
- HIC++ v3.3 – 10.17
5.6 Shift Operators
5.6.1 Do not use bitwise operators with signed operands
Use of signed operands with bitwise operators is in some cases subject to undefined or implementation defined behavior. Therefore, bitwise operators should only be used with operands of unsigned integral types.
#include <cstdint> void foo (int32_t i) { int32_t r = i << -1; // @@- Non-Compliant: undefined behavior -@@ r = -1 >> 1; // @@- Non-Compliant: implementation defined -@@ r = ~0; // @@- Non-Compliant: implementation defined -@@ uint32_t u = (-1) & 2u; // @@- Non-Compliant: implementation defined -@@ u = (-1) | 1u; // @@- Non-Compliant: implementation defined -@@ u = (-1) ^ 1u; // @@- Non-Compliant: implementation defined -@@ }
References
- HIC++ v3.3 – 10.11
5.7 Equality Operators
5.7.1 Do not write code that expects floating point calculations to yield exact results
Floating point calculations suffer from machine precision (epsilon), such that the exact result may not be representable. Epsilon is defined as the difference between 1 and the smallest value greater than 1 that can be represented in a given floating point type. Therefore, comparisons of floating point values need to take epsilon into account.
#include <cmath> #include <limits> bool isEqual( const double a, const double b ) { const double scale = ( std::fabs( a ) + std::fabs( b ) ) / 2.0; return std::fabs( a - b ) <= ( std::numeric_limits<double>::epsilon() * scale ); } void foo( double f ) { if (3.142 == f) // @@- Non-Compliant -@@ {} if (isEqual (f, 3.142)) // @@+ Compliant +@@ {} }
References
- HIC++ v3.3 - 10.15
5.7.2 Ensure that a pointer to member that is a virtual function is only compared (==) with nullptr
The result of comparing a pointer to member to a virtual function to anything other than nullptr is unspecified.
class A { public: void f1(); void f2(); virtual void f3(); }; void foo( ) { if (&A::f1 == &A::f2); // @@+ Compliant +@@ if (&A::f1 == nullptr); // @@+ Compliant +@@ if (&A::f3 == &A::f2); // @@- Not Compliant -@@ if (&A::f3 == nullptr); // @@+ Compliant +@@ }
References
- JSF AV C++ Rev C – 97.1
5.8 Conditional Operator
5.8.1 Do not use the conditional operator (?:) as a sub-expression
Evaluation of a complex condition is best achieved through explicit conditional statements (if/else). Using the result of the conditional operator as an operand reduces the maintainability of the code. The only permissible uses of a conditional expression are:
- Argument expression in a function call.
- Return expression.
- Initializer in a member initialization list.
- Object initializer.
- The right hand side operand of assignment (excluding compound assignment).
The last use is allowed on the basis of initialization of an object with automatic storage duration being equivalent to its declaration, followed by assignment.
#include <cstdint> void foo (int32_t i, int32_t j) { int32_t k; k = (j != 0) ? 1 : 0; // @@+ Compliant: equivalent to initialization +@@ // @@- Non-Compliant: nested conditional operations -@@ k = (i != 0) ? ((j != 0) ? 1 : 0) : 0; k = i + ((j != 0) ? 1 : 0); // @@- Non-Compliant -@@ }
References
- HIC++ v3.3 – 10.20