Skip to content
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

Fix #789: Reduce False positives on A7-1-2 (VariableMissingConstexpr.ql) #794

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
2 changes: 2 additions & 0 deletions change_notes/2024-11-11-fix-fp-789.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `A7-1-2` - `VariableMissingConstexpr.ql`:
- Fixes #789. Doesn't alert on non-static member variables and compiler generated variables of range based for-loops.
6 changes: 6 additions & 0 deletions cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ where
not v.isConstexpr() and
not v instanceof Parameter and
not v.isAffectedByMacro() and
// Don't consider non-static member variables.
rak3-sh marked this conversation as resolved.
Show resolved Hide resolved
(
not v instanceof MemberVariable
or
v.isStatic()
) and
isLiteralType(v.getType()) and
// Unfortunately, `isConstant` is not sufficient here because it doesn't include calls to
// constexpr constructors, and does not take into account zero initialization
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@
| test.cpp:41:14:41:15 | l2 | Variable 'l2' could be marked 'constexpr'. |
| test.cpp:44:16:44:17 | lc | Variable 'lc' could be marked 'constexpr'. |
| test.cpp:45:17:45:19 | lc2 | Variable 'lc2' could be marked 'constexpr'. |
| test.cpp:55:7:55:8 | m2 | Variable 'm2' could be marked 'constexpr'. |
| test.cpp:130:7:130:8 | m1 | Variable 'm1' could be marked 'constexpr'. |
| test.cpp:141:7:141:8 | m1 | Variable 'm1' could be marked 'constexpr'. |
| test.cpp:55:20:55:21 | m2 | Variable 'm2' could be marked 'constexpr'. |
| test.cpp:143:5:143:20 | m1 | Variable 'm1' could be marked 'constexpr'. |
| test.cpp:221:7:221:8 | l1 | Variable 'l1' could be marked 'constexpr'. |
| test.cpp:235:7:235:8 | l6 | Variable 'l6' could be marked 'constexpr'. |
| test.cpp:237:7:237:8 | l8 | Variable 'l8' could be marked 'constexpr'. |
Expand Down
25 changes: 18 additions & 7 deletions cpp/autosar/test/rules/A7-1-2/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ class MemberConstExpr {
MemberConstExpr(int p3) : m3(p3) {}

private:
int m1; // COMPLIANT - is not always zero initialized
int m2 = 0; // NON_COMPLIANT
int m3 = 0; // COMPLIANT - can be set by constructor
int m1; // COMPLIANT - is not always zero initialized
static const int m2 = 0; // NON_COMPLIANT
int m3 = 0; // COMPLIANT - can be set by constructor
};

int h1(int x, int y) { // NON_COMPLIANT
Expand Down Expand Up @@ -127,7 +127,7 @@ class MissingConstexprClass {
MissingConstexprClass(int i) = delete; // NON_COMPLIANT
MissingConstexprClass(int i, LiteralClass lc) {} // NON_COMPLIANT
private:
int m1 = 0;
int m1 = 0; // COMPLIANT - non-static member variable
};

class VirtualBaseClass {};
Expand All @@ -138,9 +138,9 @@ class DerivedClass : public virtual VirtualBaseClass {
DerivedClass(int i) = delete; // COMPLIANT
DerivedClass(int i, LiteralClass lc) {} // COMPLIANT
private:
int m1 = 0;
static int m1; // NON_COMPLAINT - static member variable can be constexpr
};

int DerivedClass::m1 = 0;
class NotAllMembersInitializedClass {
public:
NotAllMembersInitializedClass() = default; // COMPLIANT
Expand Down Expand Up @@ -274,4 +274,15 @@ template <typename T> T *init() {
return t;
}

void test_template_instantiation() { int *t = init<int>(); }
void test_template_instantiation() { int *t = init<int>(); }

#include <memory>
#include <vector>
void a_function() {
auto origin = std::vector<int>{1, 2, 3, 4, 5, 6, 7, 8, 9};
auto values = std::vector<std::unique_ptr<int>>{};
for (auto &value :
origin) { // Sometimes, CodeQL reports "value" should be constexpr
values.emplace_back(std::make_unique<int>(value));
}
}
4 changes: 3 additions & 1 deletion cpp/common/src/codingstandards/cpp/Expr.qll
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,9 @@ predicate isCompileTimeEvaluatedCall(Call call) {
parameterUsingDefaultValue.getAnAssignedValue() = defaultValue
|
isDirectCompileTimeEvaluatedExpression(defaultValue)
)
) and
// 4. the call's qualifier is compile time evaluated.
(not call.hasQualifier() or isCompileTimeEvaluatedExpression(call.getQualifier()))
}

/*
Expand Down