From d584e309b212ad29e05e74a4a0760fb567ed92e7 Mon Sep 17 00:00:00 2001 From: "rakesh.pothengil" Date: Mon, 18 Nov 2024 13:12:44 +0900 Subject: [PATCH] Consider feedback on #789 --- .../rules/A7-1-2/VariableMissingConstexpr.ql | 17 ++++++++++---- .../A7-1-2/VariableMissingConstexpr.expected | 5 ++-- cpp/autosar/test/rules/A7-1-2/test.cpp | 23 +++++-------------- 3 files changed, 21 insertions(+), 24 deletions(-) diff --git a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql index b051965a5..a07dbd43f 100644 --- a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql +++ b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql @@ -35,18 +35,24 @@ 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 - // Don't consider non-static member variables. ( not v instanceof MemberVariable or - v.isStatic() + // 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 @@ -72,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 diff --git a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected index ee33044a2..31c26a11f 100644 --- a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected +++ b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected @@ -7,8 +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: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: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'. | diff --git a/cpp/autosar/test/rules/A7-1-2/test.cpp b/cpp/autosar/test/rules/A7-1-2/test.cpp index 3b45516bc..1bbe32a93 100644 --- a/cpp/autosar/test/rules/A7-1-2/test.cpp +++ b/cpp/autosar/test/rules/A7-1-2/test.cpp @@ -51,9 +51,9 @@ class MemberConstExpr { MemberConstExpr(int p3) : m3(p3) {} private: - 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 m1; // COMPLIANT - is not always zero initialized + int m2 = 0; // NON_COMPLIANT + int m3 = 0; // COMPLIANT - can be set by constructor }; int h1(int x, int y) { // NON_COMPLIANT @@ -127,7 +127,7 @@ class MissingConstexprClass { MissingConstexprClass(int i) = delete; // NON_COMPLIANT MissingConstexprClass(int i, LiteralClass lc) {} // NON_COMPLIANT private: - int m1 = 0; // COMPLIANT - non-static member variable + int m1 = 0; }; class VirtualBaseClass {}; @@ -138,9 +138,9 @@ class DerivedClass : public virtual VirtualBaseClass { DerivedClass(int i) = delete; // COMPLIANT DerivedClass(int i, LiteralClass lc) {} // COMPLIANT private: - static int m1; // NON_COMPLAINT - static member variable can be constexpr + int m1 = 0; }; -int DerivedClass::m1 = 0; + class NotAllMembersInitializedClass { public: NotAllMembersInitializedClass() = default; // COMPLIANT @@ -275,14 +275,3 @@ template T *init() { } void test_template_instantiation() { int *t = init(); } - -#include -#include -void a_function() { - auto origin = std::vector{1, 2, 3, 4, 5, 6, 7, 8, 9}; - auto values = std::vector>{}; - for (auto &value : - origin) { // Sometimes, CodeQL reports "value" should be constexpr - values.emplace_back(std::make_unique(value)); - } -}