From 035fcbdea5fb9a8980f042160eb0780a2b9e3544 Mon Sep 17 00:00:00 2001
From: Remco Vermeulen <rvermeulen@users.noreply.github.com>
Date: Wed, 31 Jan 2024 16:55:28 -0800
Subject: [PATCH 01/10] Address incomplete `constexpr` function behavior

A call to a `constexpr` function is insufficient to determine that the
return value is compile time computed. We need to also validate that
its arguments are compile time computed.
---
 .../src/rules/A7-1-2/VariableMissingConstexpr.ql     | 11 ++++++++++-
 .../rules/A7-1-2/VariableMissingConstexpr.expected   |  1 +
 cpp/autosar/test/rules/A7-1-2/test.cpp               | 12 +++++++++++-
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
index e3981b3836..5cf458d4cc 100644
--- a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
+++ b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
@@ -33,6 +33,15 @@ predicate isTypeZeroInitializable(Type t) {
   t.getUnderlyingType() instanceof ArrayType
 }
 
+predicate isCompileTimeEvaluated(Call call) {
+  call.getTarget().isConstexpr() and
+  forall(Expr arg | arg = call.getAnArgument() |
+    DataFlow::localExprFlow(any(Literal l), arg)
+    or
+    DataFlow::localExprFlow(any(Call c | isCompileTimeEvaluated(call)), arg)
+  )
+}
+
 from Variable v
 where
   not isExcluded(v, ConstPackage::variableMissingConstexprQuery()) and
@@ -46,7 +55,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
diff --git a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
index 6b6ed61dc8..1b4a2e3945 100644
--- a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
+++ b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
@@ -10,3 +10,4 @@
 | 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:215:7:215:7 | x | Variable x 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 020ba09a2b..616cee2ac3 100644
--- a/cpp/autosar/test/rules/A7-1-2/test.cpp
+++ b/cpp/autosar/test/rules/A7-1-2/test.cpp
@@ -204,4 +204,14 @@ class ExcludedCases {
 
   void operator=(ExcludedCases &) {}  // COMPLIANT
   void operator=(ExcludedCases &&) {} // COMPLIANT
-};
\ No newline at end of file
+};
+
+
+constexpr int add(int x, int y) {
+  return x + y;
+}
+
+void fp_reported_in_466(int p) {
+  int x = add(1,2); // NON_COMPLIANT
+  int y = add(1,p); // COMPLIANT
+}
\ No newline at end of file

From 9d25d34a748dd7d7bf71e069d4cf5409eece12cd Mon Sep 17 00:00:00 2001
From: Remco Vermeulen <rvermeulen@users.noreply.github.com>
Date: Wed, 31 Jan 2024 17:05:03 -0800
Subject: [PATCH 02/10] Add changenote

---
 change_notes/2024-01-31-fix-fp-a7-1-2.md | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 change_notes/2024-01-31-fix-fp-a7-1-2.md

diff --git a/change_notes/2024-01-31-fix-fp-a7-1-2.md b/change_notes/2024-01-31-fix-fp-a7-1-2.md
new file mode 100644
index 0000000000..94a74d463f
--- /dev/null
+++ b/change_notes/2024-01-31-fix-fp-a7-1-2.md
@@ -0,0 +1,2 @@
+`A7-1-2` - `VariableMissingConstexpr.ql`:
+    - Fix FP reported in #466. Addresses incorrect assumption that calls to `constexpr` functions are always compile-time evaluated.
\ No newline at end of file

From 95f8fe2a8875b059c129f3d736edce6258bc5df4 Mon Sep 17 00:00:00 2001
From: Remco Vermeulen <rvermeulen@users.noreply.github.com>
Date: Wed, 31 Jan 2024 17:28:21 -0800
Subject: [PATCH 03/10] Address incorrect test case formatting

---
 .../test/rules/A7-1-2/VariableMissingConstexpr.expected  | 2 +-
 cpp/autosar/test/rules/A7-1-2/test.cpp                   | 9 +++------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
index 1b4a2e3945..5d014cb33d 100644
--- a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
+++ b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
@@ -10,4 +10,4 @@
 | 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:215:7:215:7 | x | Variable x could be marked 'constexpr'. |
+| test.cpp:212:7:212:7 | x | Variable x 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 616cee2ac3..bfea13ea9a 100644
--- a/cpp/autosar/test/rules/A7-1-2/test.cpp
+++ b/cpp/autosar/test/rules/A7-1-2/test.cpp
@@ -206,12 +206,9 @@ class ExcludedCases {
   void operator=(ExcludedCases &&) {} // COMPLIANT
 };
 
-
-constexpr int add(int x, int y) {
-  return x + y;
-}
+constexpr int add(int x, int y) { return x + y; }
 
 void fp_reported_in_466(int p) {
-  int x = add(1,2); // NON_COMPLIANT
-  int y = add(1,p); // COMPLIANT
+  int x = add(1, 2); // NON_COMPLIANT
+  int y = add(1, p); // COMPLIANT
 }
\ No newline at end of file

From 4bed9b6dc3232c1548aa9539f5ead62fe93f5870 Mon Sep 17 00:00:00 2001
From: Remco Vermeulen <rvermeulen@users.noreply.github.com>
Date: Thu, 1 Feb 2024 10:58:04 -0800
Subject: [PATCH 04/10] Mark new test case `constexpr`

This is required to exclude it from the FunctionMissingConstexpr.ql query
because it is not marked `constexpr`
---
 cpp/autosar/test/rules/A7-1-2/test.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cpp/autosar/test/rules/A7-1-2/test.cpp b/cpp/autosar/test/rules/A7-1-2/test.cpp
index bfea13ea9a..ca368c6069 100644
--- a/cpp/autosar/test/rules/A7-1-2/test.cpp
+++ b/cpp/autosar/test/rules/A7-1-2/test.cpp
@@ -208,7 +208,7 @@ class ExcludedCases {
 
 constexpr int add(int x, int y) { return x + y; }
 
-void fp_reported_in_466(int p) {
+constexpr void fp_reported_in_466(int p) {
   int x = add(1, 2); // NON_COMPLIANT
   int y = add(1, p); // COMPLIANT
 }
\ No newline at end of file

From 9b4593b5e71515f573f0a17a583b80f23209cff7 Mon Sep 17 00:00:00 2001
From: Remco Vermeulen <rvermeulen@users.noreply.github.com>
Date: Fri, 2 Feb 2024 16:49:07 -0800
Subject: [PATCH 05/10] Address incorrect is compiled evaluated logic

Need to consider all possible values passed as arguments and
default values.
---
 .../rules/A7-1-2/VariableMissingConstexpr.ql  | 18 ++++++++--
 .../A7-1-2/VariableMissingConstexpr.expected  | 13 +++++++-
 cpp/autosar/test/rules/A7-1-2/test.cpp        | 33 +++++++++++++++++++
 3 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
index 5cf458d4cc..efe458b624 100644
--- a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
+++ b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
@@ -33,12 +33,24 @@ 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
-  forall(Expr arg | arg = call.getAnArgument() |
-    DataFlow::localExprFlow(any(Literal l), arg)
+  // 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
-    DataFlow::localExprFlow(any(Call c | isCompileTimeEvaluated(call)), arg)
+    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
   )
 }
 
diff --git a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
index 5d014cb33d..34c76f2de3 100644
--- a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
+++ b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
@@ -10,4 +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:212:7:212:7 | x | Variable x 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'. |
diff --git a/cpp/autosar/test/rules/A7-1-2/test.cpp b/cpp/autosar/test/rules/A7-1-2/test.cpp
index ca368c6069..234cff423a 100644
--- a/cpp/autosar/test/rules/A7-1-2/test.cpp
+++ b/cpp/autosar/test/rules/A7-1-2/test.cpp
@@ -206,9 +206,42 @@ class ExcludedCases {
   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
 }
\ No newline at end of file

From 7361106cb1d91de8e7c864ebdd646841751779df Mon Sep 17 00:00:00 2001
From: Remco Vermeulen <rvermeulen@users.noreply.github.com>
Date: Fri, 2 Feb 2024 17:00:12 -0800
Subject: [PATCH 06/10] Exclude non-static members

Exclude non-static members from being marked as `constexpr`.
---
 change_notes/2024-01-31-fix-fp-a7-1-2.md                 | 3 ++-
 cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/change_notes/2024-01-31-fix-fp-a7-1-2.md b/change_notes/2024-01-31-fix-fp-a7-1-2.md
index 94a74d463f..a8a23b193c 100644
--- a/change_notes/2024-01-31-fix-fp-a7-1-2.md
+++ b/change_notes/2024-01-31-fix-fp-a7-1-2.md
@@ -1,2 +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.
\ No newline at end of file
+    - 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`.
\ No newline at end of file
diff --git a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
index efe458b624..86d278f94c 100644
--- a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
+++ b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
@@ -81,5 +81,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'."

From b7df30e8e863120e1dbfbbd510ce34826896229f Mon Sep 17 00:00:00 2001
From: Remco Vermeulen <rvermeulen@users.noreply.github.com>
Date: Fri, 2 Feb 2024 17:04:33 -0800
Subject: [PATCH 07/10] Apply correct query format

---
 .../rules/A7-1-2/VariableMissingConstexpr.ql   | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
index 86d278f94c..930b8c47c6 100644
--- a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
+++ b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
@@ -34,20 +34,28 @@ predicate isTypeZeroInitializable(Type t) {
 }
 
 /*
-  * 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.
-  */
+ * 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) |
+  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 |
+  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 28f54a799b1f6ba1ea9b9ab43ccd6f9c40e7334c Mon Sep 17 00:00:00 2001
From: Remco Vermeulen <rvermeulen@users.noreply.github.com>
Date: Mon, 5 Feb 2024 14:40:57 -0800
Subject: [PATCH 08/10] Revert "Exclude non-static members"

This reverts commit 7361106cb1d91de8e7c864ebdd646841751779df.
The change was incorrect and caused the rule to miss some cases.
---
 change_notes/2024-01-31-fix-fp-a7-1-2.md                 | 3 +--
 cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql | 4 +---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/change_notes/2024-01-31-fix-fp-a7-1-2.md b/change_notes/2024-01-31-fix-fp-a7-1-2.md
index a8a23b193c..94a74d463f 100644
--- a/change_notes/2024-01-31-fix-fp-a7-1-2.md
+++ b/change_notes/2024-01-31-fix-fp-a7-1-2.md
@@ -1,3 +1,2 @@
 `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`.
\ No newline at end of file
+    - Fix FP reported in #466. Addresses incorrect assumption that calls to `constexpr` functions are always compile-time evaluated.
\ No newline at end of file
diff --git a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
index 930b8c47c6..93391cdcb9 100644
--- a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
+++ b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
@@ -89,7 +89,5 @@ 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 and
-  // If it is a member, it must be static to be constexpr
-  (v instanceof MemberVariable implies v.isStatic())
+  not v.getDeclaringType() instanceof Union
 select v, "Variable " + v.getName() + " could be marked 'constexpr'."

From fe630e9a1c3581bf1c25bdd8622d30b92f72c902 Mon Sep 17 00:00:00 2001
From: Remco Vermeulen <rvermeulen@users.noreply.github.com>
Date: Mon, 5 Feb 2024 15:07:33 -0800
Subject: [PATCH 09/10] Annotate test cases to explain their intent

---
 .../A7-1-2/VariableMissingConstexpr.expected  | 23 +++++----
 cpp/autosar/test/rules/A7-1-2/test.cpp        | 49 +++++++++++--------
 2 files changed, 40 insertions(+), 32 deletions(-)

diff --git a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
index 34c76f2de3..7404c5193e 100644
--- a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
+++ b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
@@ -10,15 +10,14 @@
 | 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'. |
+| test.cpp:221:7:221:7 | x | Variable x could be marked 'constexpr'. |
+| test.cpp:234:7:234:7 | v | Variable v could be marked 'constexpr'. |
+| test.cpp:235:7:235:7 | w | Variable w could be marked 'constexpr'. |
+| test.cpp:237:7:237:7 | a | Variable a could be marked 'constexpr'. |
+| test.cpp:239:7:239:7 | b | Variable b could be marked 'constexpr'. |
+| test.cpp:242:7:242:7 | e | Variable e could be marked 'constexpr'. |
+| test.cpp:244:7:244:7 | f | Variable f could be marked 'constexpr'. |
+| test.cpp:245:7:245:7 | g | Variable g could be marked 'constexpr'. |
+| test.cpp:248:7:248:7 | j | Variable j could be marked 'constexpr'. |
+| test.cpp:252:7:252:7 | m | Variable m could be marked 'constexpr'. |
+| test.cpp:253:7:253:7 | n | Variable n 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 234cff423a..b60c0f9979 100644
--- a/cpp/autosar/test/rules/A7-1-2/test.cpp
+++ b/cpp/autosar/test/rules/A7-1-2/test.cpp
@@ -208,9 +208,13 @@ class ExcludedCases {
 
 extern int random();
 constexpr int add(int x, int y) { return x + y; }
+// Example with compile time constant literal value as default argument
 constexpr int add1(int x, int y = 1) { return x + y; }
+// Example with compile time constant function call as default argument
 constexpr int add2(int x, int y = add(add1(1), 2)) { return x + y; }
+// Example with non compile time constant function call as default argument
 constexpr int add3(int x, int y = random()) { return x + y; }
+// Example with compile time constant literal value as default arguments
 constexpr int add4(int x = 1, int y = 2) { return x + y; }
 
 constexpr void fp_reported_in_466(int p) {
@@ -224,24 +228,29 @@ constexpr void fp_reported_in_466(int p) {
     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
+  constexpr int t = add(1, 2); //  COMPLIANT
+
+  int u = add(z, 2); // COMPLIANT - z is not compile time constant on all paths
+  int v = add(t, 2); // NON_COMPLIANT
+  int w =
+      add1(t, 2);  // NON_COMPLIANT - all arguments are compile time constants
+  int a = add1(t); // NON_COMPLIANT - s and the default value of the second
+                   // argument are compile time constants
+  int b = add1(1); // NON_COMPLIANT
+  int c = add1(1, z); // COMPLIANT - z is not compile time constant on all paths
+  int d = add1(z);    // COMPLIANT - z is not compile time constant on all paths
+  int e = add2(1);    // NON_COMPLIANT - provided argument and default value are
+                      // compile time constants
+  int f = add2(1, 2); // NON_COMPLIANT
+  int g = add2(t, 2); // NON_COMPLIANT
+  int h = add2(z);    // COMPLIANT - z is not compile time constant on all paths
+  int i = add2(z, 1); // COMPLIANT - z is not compile time constant on all paths
+  int j = add3(1, 1); // NON_COMPLIANT
+  int k = add3(1);    // COMPLIANT - default value for second argument is not a
+                      // compile time constant
+  int l = add3(1, z); // COMPLIANT - z is not compile time constant on all paths
+  int m = add4();  // NON_COMPLIANT - default values are compile time constants
+  int n = add4(1); // NON_COMPLIANT - default value for second argument is a
+                   // compile time constant
+  int o = add4(1, z); // COMPLIANT - z is not compile time constant on all paths
 }
\ No newline at end of file

From 591c755765d1f29c83c3eeabaa65901959ff6600 Mon Sep 17 00:00:00 2001
From: Remco Vermeulen <rvermeulen@users.noreply.github.com>
Date: Mon, 5 Feb 2024 16:32:05 -0800
Subject: [PATCH 10/10] Exclude flows through non constexpr variables

Before the analysis only considered whether the source of an argument
passed to a function was computed at compile time. Now we consider
whether intermediate variables are also constexpr even though their
values are compile time constants, because otherwise the compiler
will accept the variable receiving the compiled time constant to be a
constexpr variable.
---
 .../rules/A7-1-2/VariableMissingConstexpr.ql  | 54 ++++++++++++--
 .../A7-1-2/VariableMissingConstexpr.expected  | 22 +++---
 cpp/autosar/test/rules/A7-1-2/test.cpp        | 71 +++++++++++--------
 3 files changed, 101 insertions(+), 46 deletions(-)

diff --git a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
index 93391cdcb9..3c2ae9a592 100644
--- a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
+++ b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
@@ -17,6 +17,7 @@ import cpp
 import codingstandards.cpp.autosar
 import codingstandards.cpp.TrivialType
 import codingstandards.cpp.SideEffect
+import semmle.code.cpp.controlflow.SSA
 
 predicate isZeroInitializable(Variable v) {
   not exists(v.getInitializer().getExpr()) and
@@ -33,6 +34,39 @@ predicate isTypeZeroInitializable(Type t) {
   t.getUnderlyingType() instanceof ArrayType
 }
 
+/**
+ * An optimized set of expressions used to determine the flow through constexpr variables.
+ */
+class VariableAccessOrCallOrLiteral extends Expr {
+  VariableAccessOrCallOrLiteral() {
+    this instanceof VariableAccess or
+    this instanceof Call or
+    this instanceof Literal
+  }
+}
+
+/**
+ * Holds if the value of source flows through compile time evaluated variables to target.
+ */
+predicate flowsThroughConstExprVariables(
+  VariableAccessOrCallOrLiteral source, VariableAccessOrCallOrLiteral target
+) {
+  (
+    source = target
+    or
+    source != target and
+    exists(SsaDefinition intermediateDef, StackVariable intermediate |
+      intermediateDef.getAVariable().getFunction() = source.getEnclosingFunction() and
+      intermediateDef.getAVariable().getFunction() = target.getEnclosingFunction() and
+      intermediateDef.getAVariable() = intermediate and
+      intermediate.isConstexpr()
+    |
+      DataFlow::localExprFlow(source, intermediateDef.getDefiningValue(intermediate)) and
+      flowsThroughConstExprVariables(intermediateDef.getAUse(intermediate), target)
+    )
+  )
+}
+
 /*
  * 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.
@@ -42,13 +76,23 @@ 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
+  forall(DataFlow::Node ultimateArgSource, DataFlow::Node argSource |
+    argSource = DataFlow::exprNode(call.getAnArgument()) and
+    DataFlow::localFlow(ultimateArgSource, argSource) and
     not DataFlow::localFlowStep(_, ultimateArgSource)
   |
-    ultimateArgSource.asExpr() instanceof Literal
-    or
-    any(Call c | isCompileTimeEvaluated(c)) = ultimateArgSource.asExpr()
+    (
+      ultimateArgSource.asExpr() instanceof Literal
+      or
+      any(Call c | isCompileTimeEvaluated(c)) = ultimateArgSource.asExpr()
+    ) and
+    // If the ultimate argument source is not the same as the argument source, then it must flow through
+    // constexpr variables.
+    (
+      ultimateArgSource != argSource
+      implies
+      flowsThroughConstExprVariables(ultimateArgSource.asExpr(), argSource.asExpr())
+    )
   ) and
   // 3. all the default values used are compile time evaluated.
   forall(Expr defaultValue, Parameter parameterUsingDefaultValue, int idx |
diff --git a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
index 7404c5193e..dbf223e0cf 100644
--- a/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
+++ b/cpp/autosar/test/rules/A7-1-2/VariableMissingConstexpr.expected
@@ -10,14 +10,14 @@
 | 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:221:7:221:7 | x | Variable x could be marked 'constexpr'. |
-| test.cpp:234:7:234:7 | v | Variable v could be marked 'constexpr'. |
-| test.cpp:235:7:235:7 | w | Variable w could be marked 'constexpr'. |
-| test.cpp:237:7:237:7 | a | Variable a could be marked 'constexpr'. |
-| test.cpp:239:7:239:7 | b | Variable b could be marked 'constexpr'. |
-| test.cpp:242:7:242:7 | e | Variable e could be marked 'constexpr'. |
-| test.cpp:244:7:244:7 | f | Variable f could be marked 'constexpr'. |
-| test.cpp:245:7:245:7 | g | Variable g could be marked 'constexpr'. |
-| test.cpp:248:7:248:7 | j | Variable j could be marked 'constexpr'. |
-| test.cpp:252:7:252:7 | m | Variable m could be marked 'constexpr'. |
-| test.cpp:253:7:253:7 | n | Variable n 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'. |
+| test.cpp:240:7:240:9 | l10 | Variable l10 could be marked 'constexpr'. |
+| test.cpp:243:7:243:9 | l12 | Variable l12 could be marked 'constexpr'. |
+| test.cpp:248:7:248:9 | l15 | Variable l15 could be marked 'constexpr'. |
+| test.cpp:250:7:250:9 | l16 | Variable l16 could be marked 'constexpr'. |
+| test.cpp:251:7:251:9 | l17 | Variable l17 could be marked 'constexpr'. |
+| test.cpp:257:7:257:9 | l21 | Variable l21 could be marked 'constexpr'. |
+| test.cpp:262:7:262:9 | l24 | Variable l24 could be marked 'constexpr'. |
+| test.cpp:263:7:263:9 | l25 | Variable l25 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 b60c0f9979..a3b7baea83 100644
--- a/cpp/autosar/test/rules/A7-1-2/test.cpp
+++ b/cpp/autosar/test/rules/A7-1-2/test.cpp
@@ -218,39 +218,50 @@ 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 l1 = add(1, 2); // NON_COMPLIANT
+  int l2 = add(1, p); // COMPLIANT
 
-  int z = 0;
+  int l3 = 0;
   if (p > 0) {
-    z = 1;
+    l3 = 1;
   } else {
-    z = p;
+    l3 = p;
   }
 
-  constexpr int t = add(1, 2); //  COMPLIANT
-
-  int u = add(z, 2); // COMPLIANT - z is not compile time constant on all paths
-  int v = add(t, 2); // NON_COMPLIANT
-  int w =
-      add1(t, 2);  // NON_COMPLIANT - all arguments are compile time constants
-  int a = add1(t); // NON_COMPLIANT - s and the default value of the second
-                   // argument are compile time constants
-  int b = add1(1); // NON_COMPLIANT
-  int c = add1(1, z); // COMPLIANT - z is not compile time constant on all paths
-  int d = add1(z);    // COMPLIANT - z is not compile time constant on all paths
-  int e = add2(1);    // NON_COMPLIANT - provided argument and default value are
-                      // compile time constants
-  int f = add2(1, 2); // NON_COMPLIANT
-  int g = add2(t, 2); // NON_COMPLIANT
-  int h = add2(z);    // COMPLIANT - z is not compile time constant on all paths
-  int i = add2(z, 1); // COMPLIANT - z is not compile time constant on all paths
-  int j = add3(1, 1); // NON_COMPLIANT
-  int k = add3(1);    // COMPLIANT - default value for second argument is not a
-                      // compile time constant
-  int l = add3(1, z); // COMPLIANT - z is not compile time constant on all paths
-  int m = add4();  // NON_COMPLIANT - default values are compile time constants
-  int n = add4(1); // NON_COMPLIANT - default value for second argument is a
-                   // compile time constant
-  int o = add4(1, z); // COMPLIANT - z is not compile time constant on all paths
+  constexpr int l4 = add(1, 2); //  COMPLIANT
+
+  int l5 =
+      add(l3, 2); // COMPLIANT - l3 is not compile time constant on all paths
+  int l6 = add(l4, 2); // NON_COMPLIANT
+  int l7 = add(l1, 2); // COMPLIANT - l1 is not constexpr
+  int l8 =
+      add1(l4, 2); // NON_COMPLIANT - all arguments are compile time constants
+  int l9 = add1(l1, 2); // COMPLIANT - l1 is not constexpr
+  int l10 = add1(l4);   // NON_COMPLIANT - argument and the default value of the
+                        // second argument are compile time constants
+  int l11 = add1(l1);   // COMPLIANT - l1 is not constexpr
+  int l12 = add1(1);    // NON_COMPLIANT
+  int l13 =
+      add1(1, l3); // COMPLIANT - l3 is not compile time constant on all paths
+  int l14 =
+      add1(l3);      // COMPLIANT - l3 is not compile time constant on all paths
+  int l15 = add2(1); // NON_COMPLIANT - provided argument and default value are
+                     // compile time constants
+  int l16 = add2(1, 2);  // NON_COMPLIANT
+  int l17 = add2(l4, 2); // NON_COMPLIANT
+  int l18 = add2(l1, 2); // COMPLIANT - l1 is not constexpr
+  int l19 =
+      add2(l3); // COMPLIANT - l3 is not compile time constant on all paths
+  int l20 =
+      add2(l3, 1); // COMPLIANT - l3 is not compile time constant on all paths
+  int l21 = add3(1, 1); // NON_COMPLIANT
+  int l22 = add3(1); // COMPLIANT - default value for second argument is not a
+                     // compile time constant
+  int l23 =
+      add3(1, l3);  // COMPLIANT - l3 is not compile time constant on all paths
+  int l24 = add4(); // NON_COMPLIANT - default values are compile time constants
+  int l25 = add4(1); // NON_COMPLIANT - default value for second argument is a
+                     // compile time constant
+  int l26 =
+      add4(1, l3); // COMPLIANT - l3 is not compile time constant on all paths
 }
\ No newline at end of file