Consider feedback on #789

Author: rak3-sh

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

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'. |

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 <typename T> T *init() {
 }
 
 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));
-  }
-}