Skip to content

Fixes false positives for M0-1-3. #350

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions change_notes/2023-07-26-unused-local-variable.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- `M0-1-3` - Consider constexpr variables used in template instantiations as "used".
28 changes: 28 additions & 0 deletions cpp/autosar/src/rules/M0-1-3/UnusedLocalVariable.ql
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,37 @@ import cpp
import codingstandards.cpp.autosar
import codingstandards.cpp.deadcode.UnusedVariables

/** Gets the constant value of a constexpr variable. */
private string getConstExprValue(Variable v) {
result = v.getInitializer().getExpr().getValue() and
v.isConstexpr()
}

// This predicate is similar to getUseCount for M0-1-4 except that it also
// considers static_asserts. This was created to cater for M0-1-3 specifically
// and hence, doesn't attempt to reuse the M0-1-4 specific predicate
// - getUseCount()
int getUseCountConservatively(Variable v) {
result =
count(VariableAccess access | access = v.getAnAccess())
+ count(UserProvidedConstructorFieldInit cfi | cfi.getTarget() = v) +
// For constexpr variables used as template arguments, we don't see accesses (just the
// appropriate literals). We therefore take a conservative approach and count the number of
// template instantiations that use the given constant, and consider each one to be a use
// of the variable
count(ClassTemplateInstantiation cti |
cti.getTemplateArgument(_).(Expr).getValue() = getConstExprValue(v)
)
// For static asserts too, check if there is a child which has the same value
// as the constexpr variable.
+ count(StaticAssert s |
s.getCondition().getAChild*().getValue() = getConstExprValue(v))
}

from PotentiallyUnusedLocalVariable v
where
not isExcluded(v, DeadCodePackage::unusedLocalVariableQuery()) and
// Local variable is never accessed
not exists(v.getAnAccess())
and getUseCountConservatively(v) = 0
select v, "Local variable " + v.getName() + " in " + v.getFunction().getName() + " is not used."
35 changes: 34 additions & 1 deletion cpp/autosar/test/rules/M0-1-3/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,37 @@ void test_side_effect_init() {
LA a; // NON_COMPLIANT - no constructor called
LC c; // COMPLIANT - constructor called which is considered to potentially
// have side effects
}
}

#include <array>
#include <cstdio>
template <int t> class CharBuffer {
public:
int member[t];
CharBuffer() : member{0} {}
};

int test_constexpr_in_template_inst() {
constexpr int line_length = 1024U; // COMPLIANT - used in template inst.
// of buffer.
CharBuffer<line_length> buffer{};
return buffer.member[0];
}

enum DataType : unsigned char {
int8,
int16,
};

template <typename... Types> int test_constexpr_in_static_assert() {
const std::array<DataType, sizeof...(Types)> lldts{int8};
const std::array<DataType, sizeof...(Types)> llams{int16};
constexpr std::size_t mssu = 64 * 1024; // COMPLIANT - used in static assert.
static_assert((sizeof(lldts) + sizeof(llams)) <= mssu, "assert");
return 0;
}

int baz() {
test_constexpr_in_static_assert<int>();
return 0;
}
7 changes: 5 additions & 2 deletions rule_packages/cpp/DeadCode.json
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,10 @@
"tags": [
"readability",
"maintainability"
]
],
"implementation_scope": {
"description": "In limited cases, this query can raise false-positives for variables that are defined as constexpr and used in an expression to instantiate a template."
}
},
{
"description": "Unused variables complicate the program and can indicate a possible mistake on the part of the programmer.",
Expand Down Expand Up @@ -344,4 +347,4 @@
"title": "There shall be no dead code."
}
}
}
}
2 changes: 1 addition & 1 deletion rule_packages/cpp/Templates.json
Original file line number Diff line number Diff line change
Expand Up @@ -192,4 +192,4 @@
"title": "In a class template with a dependent base, any name that may be found in that dependent base shall be referred to using a qualified-id or this->."
}
}
}
}