Fix #665: M3-4-1 incorrectly computes scope for template variables and constexpr variables

change_notes/2024-09-19-fix-fp-665-M3-4-1.md

@@ -0,0 +1,2 @@
+- `M3-4-1` - `UnnecessaryExposedIdentifierDeclarationShared.qll`:
+  - Fixes #665. Exclude variables that are constexpr and coming from template instantiations.

cpp/common/src/codingstandards/cpp/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.qll

@@ -120,7 +120,10 @@ private predicate isTypeUse(Type t1, Type t2) {
 }
 
 newtype TDeclarationAccess =
-  ObjectAccess(Variable v, VariableAccess va) { va = v.getAnAccess() } or
+  ObjectAccess(Variable v, VariableAccess va) {
+    va = v.getAnAccess() or
+    v.(TemplateVariable).getAnInstantiation().getAnAccess() = va
+  } or
   /* Type access can be done in a declaration or an expression (e.g., static member function call) */
   TypeAccess(Type t, Element access) {
     isTypeUse(access.(Variable).getUnspecifiedType(), t)
@@ -205,9 +208,13 @@ class DeclarationAccess extends TDeclarationAccess {
 
 class CandidateDeclaration extends Declaration {
   CandidateDeclaration() {
-    this instanceof LocalVariable
+    this instanceof LocalVariable and
+    not this.(LocalVariable).isConstexpr() and
+    not this.isFromTemplateInstantiation(_)
     or
-    this instanceof GlobalOrNamespaceVariable
+    this instanceof GlobalOrNamespaceVariable and
+    not this.isFromTemplateInstantiation(_) and
+    not this.(GlobalOrNamespaceVariable).isConstexpr()
     or
     this instanceof Type and
     not this instanceof ClassTemplateInstantiation and
@@ -229,7 +236,13 @@ Scope possibleScopesForDeclaration(CandidateDeclaration d) {
     result = scope.getStrictParent*()
   ) and
   // Limit the best scope to block statements and namespaces or control structures
-  (result instanceof BlockStmt or result instanceof Namespace)
+  (
+    result instanceof BlockStmt and
+    // Template variables cannot be in block scope
+    not d instanceof TemplateVariable
+    or
+    result instanceof Namespace
+  )
 }
 
 /* Gets the smallest scope that includes all the declaration accesses of declaration `d`. */

cpp/common/test/rules/unnecessaryexposedidentifierdeclarationshared/test.cpp

@@ -136,4 +136,96 @@ void f17() {
     ptr = &i;
   }
   *ptr = 1;
-}
+}
+
+namespace a_namespace {
+
+constexpr static unsigned int a_constexpr_var{
+    10U}; // COMPLIANT; used in
+          // a_namespace and
+          // another_namespace_function
+static unsigned int
+    a_namespace_var[a_constexpr_var]{}; // COMPLIANT; used in
+                                        // a_namespace_function and
+                                        // another_namespace_function
+
+constexpr static unsigned int a_namespace_function(void) noexcept {
+  unsigned int a_return_value{0U};
+
+  for (auto loop_var : a_namespace_var) { // usage of a_namespace_var
+    a_return_value += loop_var;
+  }
+  return a_return_value;
+}
+
+constexpr static unsigned int another_namespace_function(void) noexcept {
+  unsigned int a_return_value{0U};
+
+  for (unsigned int i{0U}; i < a_constexpr_var;
+       i++) {                             // usage of a_constexpr_var
+    a_return_value += a_namespace_var[i]; // usage of a_namespace_var
+  }
+  return a_return_value;
+}
+} // namespace a_namespace
+
+namespace parent_namespace {
+namespace child_namespace {
+template <typename From> class a_class_in_child_namespace {
+public:
+  template <typename To> constexpr auto &&operator()(To &&val) const noexcept {
+    return static_cast<To>(val);
+  }
+}; // a_class_in_child_namespace end
+
+template <typename From>
+extern constexpr a_class_in_child_namespace<From>
+    a_class_in_child_namespace_impl{};
+
+} // namespace child_namespace
+
+template <typename From>
+static constexpr auto const &a_parent_namespace_variable =
+    child_namespace::a_class_in_child_namespace_impl<
+        From>; // COMPLIANT; used in child_namespace2::a_class::bar() and
+               // parent_namespace::another_class::foo()
+
+namespace child_namespace2 {
+class a_class {
+public:
+  int func(...) { return 0; }
+  void foo(int x) { x++; }
+  template <typename F> constexpr auto bar(F(*func), int b) {
+    foo(func(a_parent_namespace_variable<F>(
+        b))); // usage of a_parent_namespace_variable
+  }
+}; // a_class
+} // namespace child_namespace2
+
+class another_class {
+  int a;
+  int b;
+  void bar(int param) { param++; }
+
+  bool has_value() { return a == b; }
+
+public:
+  template <typename F> int foo(F(*func), int b) {
+    if (has_value()) {
+      bar(func(a_parent_namespace_variable<F>(
+          b))); // usage of a_parent_namespace_variable
+    }
+    return 0;
+  }
+}; // another_class
+} // namespace parent_namespace
+
+template <typename T> T a_func(T v) { return v++; }
+
+int main() {
+  parent_namespace::child_namespace2::a_class a_class_obj;
+  a_class_obj.bar(a_func<int>, 10);
+  parent_namespace::another_class another_class_obj;
+  another_class_obj.foo(a_func<int>, 10);
+  return 0;
+}