Unused assignments should be removed.
Why is this an issue?
Computing or retrieving a value only to then immediately overwrite it or throw it away indicates a serious logic error in the code.
Assigning a value to a local variable that is not read by any subsequent instruction is called a dead store. The following code snippet
depicts a few dead stores.
int foo() {
int x = 0; // Noncompliant: dead store, next line overwrites x
x = 100; // Noncompliant: dead store, next line overwrites x
x = 200;
int y = 0;
y += 9001; // Noncompliant: dead store, y is never used
int z = 300; // Noncompliant: dead store, next line overwrites z
z = 400;
return x + z * 2;
}
Even if the unnecessary operations do not do any harm in terms of the program’s correctness, they are—at best—a waste of computing resources. In
most cases, these operations have their intended use but it is not expressed correctly in the code. Therefore, unused values and superfluous code
should be removed to prevent logic errors.
What is the potential impact?
Not only do unused values and superfluous code make the program unnecessary complex, but also indicate significant logic errors. And even in the
absence of logic errors, they waste computing resources in case the compiler is not able to optimize them away.
Unused values typically showcase a discrepancy between what a developer intended and what is specified in the code and should be removed to uncover
and eventually prevent logic errors.
How to fix it
Remove unused values and superfluous code.
Code examples
Noncompliant code example
int foo(int y) {
int x = 0;
x = 100; // Noncompliant: dead store
x = 200;
return x + y;
}
Compliant solution
int foo(int y) {
int x = 200; // Compliant: no unnecessary assignment
return x + y;
}
Noncompliant code example
int bar();
int buz();
int foo(bool b) {
int x = 0;
if (b) {
x = bar();
return x;
}
if (x != 0) {
int y = buz();
y += 9001; // Noncompliant: dead store
}
return x;
}
Compliant solution
int bar();
int buz();
int foo(bool b) {
int x = 0;
if (b) {
x = bar();
return x;
}
// Compliant: no more dead stores and superfluous code
// Assuming call to buz() had no important side effects
return x;
}
Pitfalls
When removing unused values and superfluous code, make sure that the right-hand side of a given assignment has no side effects.
While it is safe to remove the call to square in the following code since it has no side effects, removing the call to
fwrite changes the program’s behavior. Still, values that are never read such as n indicate code smells that should be
mitigated. In this code example, the return value of fwrite should be checked and any potential error should be handled
appropriately.
#include <stdio.h>
#include <string.h>
int square(int n) {
return n * n;
}
int foo(int i) {
int sq = square(i); // Noncompliant: dead store, assignment can be removed
const char* const str = "Hello, World!\n";
// Although `n` is never read, the call to `fwrite` cannot be removed due to side effects
size_t n = fwrite(str, sizeof(char), strlen(str), stdout); // Noncompliant: `n` is never read
return i + 9001;
}
Going the extra mile
In C++17, the nodiscard attribute has been introduced which can be used to annotate functions, enumerations and classes.
The attribute serves as a hint to the compiler and to other developers that a function’s return value should not be ignored. A function that is
marked nodiscard whose return value is ignored encourages the compiler to issue a warning. Example usages of the nodiscard
attribute are shown in the following:
[[nodiscard]] int foo() { return 100; }
int bar() { return 200; }
[[nodiscard("An explanation on why not to discard the return value")]] int buz() { return 300; }
enum class [[nodiscard]] important_error_info { OK, WARN, CRITICAL };
important_error_info compute() {
// More code ...
// In case of a critical error, return corresponding error info:
return important_error_info::CRITICAL;
}
void caller() {
foo(); // compiler warns on discarding a nodiscard value
bar(); // compiler will issue no warning
buz(); // compiler warns on discarding a nodiscard value
compute(); // compiler warns on discarding a nodiscard value
}
In case, the return value of a function marked as nodiscard should be (exceptionally) ignored, a cast to void can be used
to silence the compiler warning as shown in the following:
[[nodiscard]] int foo() { return 100; }
void caller() {
foo(); // compiler warns on discarding a nodiscard value
(void)foo(); // compiler will issue no warning
}
Resources
Standards
Related rules
- S1763 - All code should be reachable
- S2583 - Conditionally executed code should be reachable
- S2589 - Boolean expressions should not be gratuitous
- S3516 - Methods returns should not be invariant
- S3626 - Jump statements should not be redundant
