Merge pull request #794 from rak3-sh/rp/fix-789

Fix #789: Reduce False positives on A7-1-2 (VariableMissingConstexpr.ql)
github · Nov 25, 2024 · 03be7b8 · 03be7b8
2 parents f97ec0f + 03287b3
commit 03be7b8
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 10 deletions.
diff --git a/change_notes/2024-11-11-fix-fp-789.md b/change_notes/2024-11-11-fix-fp-789.md
@@ -0,0 +1,3 @@
+- `A7-1-2` - `VariableMissingConstexpr.ql`:
+  - Do not report on member variables if the class has un-instantiated member function(s).
+  - Check a call's qualifier as well whether it can be compile time evaluated or not.
diff --git a/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql b/cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql
@@ -35,13 +35,25 @@ 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
+  (
+    not v instanceof MemberVariable
+    or
+    // 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
   // constexpr constructors, and does not take into account zero initialization
@@ -66,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
@@ -7,9 +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: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: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
@@ -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; // NON_COMPLIANT
 };
 
 class VirtualBaseClass {};
@@ -138,7 +138,7 @@ class DerivedClass : public virtual VirtualBaseClass {
   DerivedClass(int i) = delete;           // COMPLIANT
   DerivedClass(int i, LiteralClass lc) {} // COMPLIANT
 private:
-  int m1 = 0;
+  int m1 = 0; // NON_COMPLIANT
 };
 
 class NotAllMembersInitializedClass {
@@ -274,4 +274,4 @@ template <typename T> T *init() {
   return t;
 }
 
-void test_template_instantiation() { int *t = init<int>(); }
+void test_template_instantiation() { int *t = init<int>(); }
diff --git 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()))
 }
 
 /*