Fix FP reported in #466

change_notes/2024-01-31-fix-fp-a7-1-2.md

@@ -0,0 +1,3 @@
+`A7-1-2` - `VariableMissingConstexpr.ql`:
+    - Fix FP reported in #466. Addresses incorrect assumption that calls to `constexpr` functions are always compile-time evaluated.
+    - Exclude member that aren't `static`, because they cannot be `constexpr`.

cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql

@@ -33,6 +33,35 @@ predicate isTypeZeroInitializable(Type t) {
   t.getUnderlyingType() instanceof ArrayType
 }
 
+/*
+ * Returns true if the given call may be evaluated at compile time and is compile time evaluated because
+ * all its arguments are compile time evaluated and its default values are compile time evaluated.
+ */
+
+predicate isCompileTimeEvaluated(Call call) {
+  // 1. The call may be evaluated at compile time, because it is constexpr, and
+  call.getTarget().isConstexpr() and
+  // 2. all its arguments are compile time evaluated, and
+  forall(DataFlow::Node ultimateArgSource |
+    DataFlow::localFlow(ultimateArgSource, DataFlow::exprNode(call.getAnArgument())) and
+    not DataFlow::localFlowStep(_, ultimateArgSource)
+  |
+    ultimateArgSource.asExpr() instanceof Literal
+    or
+    any(Call c | isCompileTimeEvaluated(c)) = ultimateArgSource.asExpr()
+  ) and
+  // 3. all the default values used are compile time evaluated.
+  forall(Expr defaultValue, Parameter parameterUsingDefaultValue, int idx |
+    parameterUsingDefaultValue = call.getTarget().getParameter(idx) and
+    not exists(call.getArgument(idx)) and
+    parameterUsingDefaultValue.getAnAssignedValue() = defaultValue
+  |
+    defaultValue instanceof Literal
+    or
+    any(Call c | isCompileTimeEvaluated(c)) = defaultValue
+  )
+}
+
 from Variable v
 where
   not isExcluded(v, ConstPackage::variableMissingConstexprQuery()) and
@@ -46,7 +75,7 @@ where
   (
     v.getInitializer().getExpr().isConstant()
     or
-    v.getInitializer().getExpr().(Call).getTarget().isConstexpr()
+    any(Call call | isCompileTimeEvaluated(call)) = v.getInitializer().getExpr()
     or
     isZeroInitializable(v)
     or
@@ -60,5 +89,7 @@ where
   // Not assigned by a user in a constructor
   not exists(ConstructorFieldInit cfi | cfi.getTarget() = v and not cfi.isCompilerGenerated()) and
   // Ignore union members
-  not v.getDeclaringType() instanceof Union
+  not v.getDeclaringType() instanceof Union and
+  // If it is a member, it must be static to be constexpr
+  (v instanceof MemberVariable implies v.isStatic())
 select v, "Variable " + v.getName() + " could be marked 'constexpr'."

cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected

@@ -10,3 +10,15 @@
 | 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:217:7:217:7 | x | Variable x could be marked 'constexpr'. |
+| test.cpp:228:7:228:7 | v | Variable v could be marked 'constexpr'. |
+| test.cpp:229:7:229:7 | w | Variable w could be marked 'constexpr'. |
+| test.cpp:230:7:230:7 | a | Variable a could be marked 'constexpr'. |
+| test.cpp:231:7:231:7 | b | Variable b could be marked 'constexpr'. |
+| test.cpp:235:7:235:7 | f | Variable f could be marked 'constexpr'. |
+| test.cpp:236:7:236:7 | g | Variable g could be marked 'constexpr'. |
+| test.cpp:237:7:237:7 | h | Variable h could be marked 'constexpr'. |
+| test.cpp:238:7:238:7 | i | Variable i could be marked 'constexpr'. |
+| test.cpp:241:7:241:7 | l | Variable l could be marked 'constexpr'. |
+| test.cpp:244:7:244:7 | o | Variable o could be marked 'constexpr'. |
+| test.cpp:245:7:245:7 | q | Variable q could be marked 'constexpr'. |

cpp/autosar/test/rules/A7-1-2/test.cpp

@@ -204,4 +204,44 @@ class ExcludedCases {
 
   void operator=(ExcludedCases &) {}  // COMPLIANT
   void operator=(ExcludedCases &&) {} // COMPLIANT
-};
+};
+
+extern int random();
+constexpr int add(int x, int y) { return x + y; }
+constexpr int add1(int x, int y = 1) { return x + y; }
+constexpr int add2(int x, int y = add(add1(1), 2)) { return x + y; }
+constexpr int add3(int x, int y = random()) { return x + y; }
+constexpr int add4(int x = 1, int y = 2) { return x + y; }
+
+constexpr void fp_reported_in_466(int p) {
+  int x = add(1, 2); // NON_COMPLIANT
+  int y = add(1, p); // COMPLIANT
+
+  int z = 0;
+  if (p > 0) {
+    z = 1;
+  } else {
+    z = p;
+  }
+
+  int u = add(z, 2);  // COMPLIANT
+  int v = add(x, 2);  // NON_COMPLIANT
+  int w = add1(x, 2); // NON_COMPLIANT
+  int a = add1(x);    // NON_COMPLIANT
+  int b = add1(1);    // NON_COMPLIANT
+  int c = add1(1, z); // COMPLIANT
+  int d = add1(1, z); // COMPLIANT
+  int e = add1(z);    // COMPLIANT
+  int f = add2(1);    // NON_COMPLIANT
+  int g = add2(1, 2); // NON_COMPLIANT
+  int h = add2(x, 2); // NON_COMPLIANT
+  int i = add2(x, 2); // NON_COMPLIANT
+  int j = add2(z);    // COMPLIANT
+  int k = add2(z, 1); // COMPLIANT
+  int l = add3(1, 1); // NON_COMPLIANT
+  int m = add3(1);    // COMPLIANT
+  int n = add3(1, z); // COMPLIANT
+  int o = add4();     // NON_COMPLIANT
+  int q = add4(1);    // NON_COMPLIANT
+  int r = add4(1, z); // COMPLIANT
+}