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 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
3 changes: 3 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,3 @@
- `A7-1-2` - `VariableMissingConstexpr.ql`:
- Do not report on member variables if the class has un-instantiated member function(s).
- Check a call's qualifier as well whether it can be compile time evaluated or not.
19 changes: 16 additions & 3 deletions cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,25 @@ predicate isTypeZeroInitializable(Type t) {
t.getUnderlyingType() instanceof ArrayType
}

from Variable v
from Variable v, string msg
where
not isExcluded(v, ConstPackage::variableMissingConstexprQuery()) and
v.hasDefinition() and
not v.isConstexpr() and
not v instanceof Parameter and
not v.isAffectedByMacro() and
(
not v instanceof MemberVariable
or
// In case member functions are left un-instantiated, it is possible
// the member variable could be modified in them.
// Hence, don't raise an alert in case this member variable's class
// has a member function that doesn't have a definition.
not exists(MemberFunction mf |
mf.getDeclaringType() = v.getDeclaringType() and
mf.isFromUninstantiatedTemplate(_)
)
) 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 All @@ -66,5 +78,6 @@ where
// Exclude variables in uninstantiated templates, as they may be incomplete
not v.isFromUninstantiatedTemplate(_) and
// Exclude compiler generated variables, which are not user controllable
not v.isCompilerGenerated()
select v, "Variable '" + v.getName() + "' could be marked 'constexpr'."
not v.isCompilerGenerated() and
if v instanceof MemberVariable and not v.isStatic() then msg = " and static." else msg = "."
select v, "Variable '" + v.getName() + "' could be marked 'constexpr'" + msg
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
| 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:7:55:8 | m2 | Variable 'm2' could be marked 'constexpr' and static. |
| test.cpp:130:7:130:8 | m1 | Variable 'm1' could be marked 'constexpr' and static. |
| test.cpp:141:7:141:8 | m1 | Variable 'm1' could be marked 'constexpr' and static. |
| 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
2 changes: 1 addition & 1 deletion cpp/autosar/test/rules/A7-1-2/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,4 +274,4 @@ template <typename T> T *init() {
return t;
}

void test_template_instantiation() { int *t = init<int>(); }
void test_template_instantiation() { int *t = init<int>(); }
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