Merge pull request #2 from rvermeulen/rvermeulen/lambda-scope

Handle lambda scope using `Scope` class

cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll

@@ -23,43 +23,52 @@ class NonVolatileConstIntegralOrEnumType extends IntegralOrEnumType {
 }
 
 /**
- * Holds if declaration `innerDecl`, declared in a lambda, hides a declaration `outerDecl` captured by the lambda.
+ * Holds if declaration `innerDecl`, declared in a lambda, hides a declaration `outerDecl` by the lambda.
  */
 predicate hiddenInLambda(UserVariable outerDecl, UserVariable innerDecl) {
-  exists(Scope s, Closure le |
-    //innerDecl declared inside of the lambda
-    s.getADeclaration() = innerDecl and
-    s.getAnAncestor() = le and
-    //a variable can be accessed (therefore hide) another when:
-    //it is explicitly captured
+  exists(
+    Scope innerScope, LambdaExpression lambdaExpr, Scope lambdaExprScope, Scope outerScope,
+    Closure lambdaClosure
+  |
+    // The variable `innerDecl` is declared inside of the lambda.
+    innerScope.getADeclaration() = innerDecl and
+    // Because a lambda is compiled down to a closure, we need to use the closure to determine if the declaration
+    // is part of the lambda.
+    innerScope.getAnAncestor() = lambdaClosure and
+    // Next we determine the scope of the lambda expression to determine if `outerDecl` is visible in the scope of the lambda.
+    lambdaClosure.getLambdaExpression() = lambdaExpr and
+    lambdaExprScope.getAnExpr() = lambdaExpr and
+    outerScope.getADeclaration() = outerDecl and
+    lambdaExprScope.getStrictParent*() = outerScope and
     (
+      // A definition can be hidden if it is in scope and it is captured by the lambda,
       exists(LambdaCapture cap |
-        outerDecl.getAnAccess() = cap.getInitializer().(VariableAccess) and
-        le.getLambdaExpression().getACapture() = cap and
-        //captured variable (outerDecl) is in the same (function) scope as the lambda itself
-        outerDecl.getParentScope() = le.getEnclosingFunction().getBasicBlock().(Scope)
+        lambdaExpr.getACapture() = cap and
+        // The outer declaration is captured by the lambda
+        outerDecl.getAnAccess() = cap.getInitializer()
       )
       or
-      //is non-local
+      // it is is non-local,
       outerDecl instanceof GlobalVariable
       or
-      //has static or thread local storage duration
+      // it has static or thread local storage duration,
       (outerDecl.isThreadLocal() or outerDecl.isStatic())
       or
-      //is a reference that has been initialized with a constant expression.
+      //it is a reference that has been initialized with a constant expression.
       outerDecl.getType().stripTopLevelSpecifiers() instanceof ReferenceType and
       exists(outerDecl.getInitializer().getExpr().getValue())
       or
-      //const non-volatile integral or enumeration type and has been initialized with a constant expression
+      //it const non-volatile integral or enumeration type and has been initialized with a constant expression
       outerDecl.getType() instanceof NonVolatileConstIntegralOrEnumType and
       exists(outerDecl.getInitializer().getExpr().getValue())
       or
-      //is constexpr and has no mutable members
+      //it is constexpr and has no mutable members
       outerDecl.isConstexpr() and
       not exists(Class c |
         c = outerDecl.getType() and not c.getAMember() instanceof MutableVariable
       )
     ) and
+    // Finally, the variables must have the same names.
     innerDecl.getName() = outerDecl.getName()
   )
 }

cpp/common/test/rules/identifierhidden/IdentifierHidden.expected

@@ -11,3 +11,7 @@
 | test.cpp:81:5:81:5 | a | Declaration is hiding declaration $@. | test.cpp:79:5:79:5 | a | a |
 | test.cpp:102:9:102:9 | b | Declaration is hiding declaration $@. | test.cpp:96:11:96:11 | b | b |
 | test.cpp:114:9:114:17 | globalvar | Declaration is hiding declaration $@. | test.cpp:110:5:110:13 | globalvar | globalvar |
+| test.cpp:133:11:133:11 | b | Declaration is hiding declaration $@. | test.cpp:127:13:127:13 | b | b |
+| test.cpp:142:9:142:10 | a1 | Declaration is hiding declaration $@. | test.cpp:140:14:140:15 | a1 | a1 |
+| test.cpp:147:9:147:10 | a2 | Declaration is hiding declaration $@. | test.cpp:145:20:145:21 | a2 | a2 |
+| test.cpp:152:9:152:10 | a3 | Declaration is hiding declaration $@. | test.cpp:150:17:150:18 | a3 | a3 |

cpp/common/test/rules/identifierhidden/test.cpp

@@ -119,4 +119,65 @@ int f6() {
   auto lambda_without_shadowing = []() { return globalvar + globalvar; };
 
   return lambda_with_shadowing();
+}
+
+void f7(int p) {
+  // Introduce a nested scope to test scope comparison.
+  if (p != 0) {
+    int a1, b;
+    auto lambda1 = [a1]() {
+      int b = 10; // COMPLIANT - exception - non captured variable b
+    };
+
+    auto lambda2 = [b]() {
+      int b = 10; // NON_COMPLIANT - not an exception - captured
+                  // variable b
+    };
+  }
+}
+
+void f8() {
+  static int a1;
+  auto lambda1 = []() {
+    int a1 = 10; // NON_COMPLIANT - Lambda can access static variable.
+  };
+
+  thread_local int a2;
+  auto lambda2 = []() {
+    int a2 = 10; // NON_COMPLIANT - Lambda can access thread local variable.
+  };
+
+  constexpr int a3 = 10;
+  auto lambda3 = []() {
+    int a3 = a3 + 1; // NON_COMPLIANT - Lambda can access const
+                     // expression without mutable members.
+  };
+
+  const int &a4 = a3;
+  auto lambda4 = []() {
+    int a4 = a4 + 1; // NON_COMPLIANT[FALSE_NEGATIVE] - Lambda can access
+                     // reference initialized with constant expression.
+  };
+
+  const int a5 = 10;
+  auto lambda5 = []() {
+    int a5 = a5 + 1; // NON_COMPLIANT[FALSE_NEGATIVE] - Lambda can access const
+                     // non-volatile integral or enumeration type initialized
+                     // with constant expression.
+  };
+
+  volatile const int a6 = 10;
+  auto lambda6 = []() {
+    int a6 =
+        a6 + 1; // COMPLIANT - Lambda cannot access const volatile integral or
+                // enumeration type initialized with constant expression.
+  };
+}
+
+void f9() {
+  auto lambda1 = []() {
+    int a1 = 10; // COMPLIANT
+  };
+
+  int a1 = 10;
 }