From de24d43f7c5590528e026a6ea21f76d7a971a8a3 Mon Sep 17 00:00:00 2001
From: "rakesh.pothengil" <rakesh.pothengil@woven-planet.global>
Date: Mon, 11 Nov 2024 13:17:25 +0900
Subject: [PATCH] Fix #789

---
 change_notes/2024-11-11-fix-fp-789.md         |  2 ++
 .../rules/A7-1-2/VariableMissingConstexpr.ql  |  6 +++++
 .../A7-1-2/VariableMissingConstexpr.expected  |  5 ++--
 cpp/autosar/test/rules/A7-1-2/test.cpp        | 25 +++++++++++++------
 cpp/common/src/codingstandards/cpp/Expr.qll   |  4 ++-
 5 files changed, 31 insertions(+), 11 deletions(-)
 create mode 100644 change_notes/2024-11-11-fix-fp-789.md

diff --git a/change_notes/2024-11-11-fix-fp-789.md b/change_notes/2024-11-11-fix-fp-789.md
new file mode 100644
index 000000000..6ba34dbd7
--- /dev/null
+++ b/change_notes/2024-11-11-fix-fp-789.md
@@ -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.
diff --git a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
index f0adab07d..b051965a5 100644
--- a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
+++ b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
@@ -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.
+  (
+    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
diff --git a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
index f86faf1a7..ee33044a2 100644
--- a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
+++ b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
@@ -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'. |
diff --git a/cpp/autosar/test/rules/A7-1-2/test.cpp b/cpp/autosar/test/rules/A7-1-2/test.cpp
index 8395f60ff..3b45516bc 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
-  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
@@ -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 {};
@@ -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
@@ -274,4 +274,15 @@ template <typename T> T *init() {
   return t;
 }
 
-void test_template_instantiation() { int *t = init<int>(); }
\ No newline at end of file
+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));
+  }
+}
diff --git a/cpp/common/src/codingstandards/cpp/Expr.qll b/cpp/common/src/codingstandards/cpp/Expr.qll
index fe2877f84..90730b971 100644
--- a/cpp/common/src/codingstandards/cpp/Expr.qll
+++ b/cpp/common/src/codingstandards/cpp/Expr.qll
@@ -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()))
 }
 
 /*