From 27efc319d67e44524f3eeeae10b61b85f71671c0 Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Fri, 6 Dec 2024 22:53:18 +0000
Subject: [PATCH 01/20] Remove unused Scope import

---
 cpp/cert/src/rules/DCL53-CPP/LocalFunctionDeclaration.ql | 1 -
 1 file changed, 1 deletion(-)

diff --git a/cpp/cert/src/rules/DCL53-CPP/LocalFunctionDeclaration.ql b/cpp/cert/src/rules/DCL53-CPP/LocalFunctionDeclaration.ql
index 3f91530c84..368c0a05e4 100644
--- a/cpp/cert/src/rules/DCL53-CPP/LocalFunctionDeclaration.ql
+++ b/cpp/cert/src/rules/DCL53-CPP/LocalFunctionDeclaration.ql
@@ -13,7 +13,6 @@
 
 import cpp
 import codingstandards.cpp.cert
-import codingstandards.cpp.Scope
 
 class LocalUserFunctionDeclarationEntry extends FunctionDeclarationEntry {
   DeclStmt ds;

From 0c92f2efa9b6305fae4e3e188c5eab2939f71886 Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Fri, 6 Dec 2024 23:54:56 +0000
Subject: [PATCH 02/20] DCL53-CPP: Migrate to hidesStrict

Any conflicting variable will be, by definition, in a different
scope.
---
 .../LocalConstructorInitializedObjectHidesIdentifier.ql         | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cpp/cert/src/rules/DCL53-CPP/LocalConstructorInitializedObjectHidesIdentifier.ql b/cpp/cert/src/rules/DCL53-CPP/LocalConstructorInitializedObjectHidesIdentifier.ql
index 237ebbe985..f6fe18a3db 100644
--- a/cpp/cert/src/rules/DCL53-CPP/LocalConstructorInitializedObjectHidesIdentifier.ql
+++ b/cpp/cert/src/rules/DCL53-CPP/LocalConstructorInitializedObjectHidesIdentifier.ql
@@ -20,6 +20,6 @@ from UserVariable v, UserVariable hidden
 where
   not isExcluded(v, ScopePackage::localConstructorInitializedObjectHidesIdentifierQuery()) and
   v.getInitializer().getExpr() instanceof ConstructorCall and
-  hides(hidden, v)
+  hidesStrict(hidden, v)
 select v, "The declaration declares variable " + v.getName() + " that hides $@", hidden,
   hidden.getName()

From 90d4127a7bc8c8f3a9a09541f39aebb50f25a98d Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Sat, 7 Dec 2024 00:04:46 +0000
Subject: [PATCH 03/20] Scope: Remove hides(..) and related predicates

There are no more consumers of hides(..).

In addition, it doesn't make sense conceptually to look for
variables in the same scope with the same name, as scopes will
prohibit using the same name in the same scope. Reviewing real
world cases where this occurs, they all seem to be extractor
oddities (multiple copies of parameters for the same function etc.)
which provides further evidence that this mode is not required.
---
 cpp/common/src/codingstandards/cpp/Scope.qll | 28 --------------------
 1 file changed, 28 deletions(-)

diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll
index d9a81b98e3..b05c5dc9c4 100644
--- a/cpp/common/src/codingstandards/cpp/Scope.qll
+++ b/cpp/common/src/codingstandards/cpp/Scope.qll
@@ -160,15 +160,6 @@ predicate inSameTranslationUnit(File f1, File f2) {
   )
 }
 
-/**
- * Gets a user variable which occurs in the "potential scope" of variable `v`.
- */
-cached
-UserVariable getPotentialScopeOfVariable(UserVariable v) {
-  result = getPotentialScopeOfVariable_candidate(v) and
-  inSameTranslationUnit(v.getFile(), result.getFile())
-}
-
 /**
  * Gets a user variable which occurs in the "outer scope" of variable `v`.
  */
@@ -203,15 +194,6 @@ class TranslationUnit extends SourceFile {
   }
 }
 
-/** Holds if `v2` may hide `v1`. */
-private predicate hides_candidate(UserVariable v1, UserVariable v2) {
-  not v1 = v2 and
-  v2 = getPotentialScopeOfVariable(v1) and
-  v1.getName() = v2.getName() and
-  // Member variables cannot hide other variables nor be hidden because the can be referenced through their qualified name.
-  not (v1.isMember() or v2.isMember())
-}
-
 /** Holds if `v2` may hide `v1`. */
 private predicate hides_candidateStrict(UserVariable v1, UserVariable v2) {
   not v1 = v2 and
@@ -256,16 +238,6 @@ private Stmt getEnclosingStmt(LocalScopeVariable v) {
   )
 }
 
-/** Holds if `v2` hides `v1`. */
-predicate hides(UserVariable v1, UserVariable v2) {
-  hides_candidate(v1, v2) and
-  // Confirm that there's no closer candidate variable which `v2` hides
-  not exists(UserVariable mid |
-    hides_candidate(v1, mid) and
-    hides_candidate(mid, v2)
-  )
-}
-
 /** Holds if `v2` strictly (`v2` is in an inner scope compared to `v1`) hides `v1`. */
 predicate hidesStrict(UserVariable v1, UserVariable v2) {
   hides_candidateStrict(v1, v2) and

From 6b5021a923702d9b85bca2e923ff279b8e5fd69f Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Sun, 8 Dec 2024 17:23:28 +0000
Subject: [PATCH 04/20] Scope: Add getParentScope testing

 - Expose the internal getParentScope for testing.
 - Add test cases
---
 cpp/common/src/codingstandards/cpp/Scope.qll  | 107 ++++++++++--------
 .../cpp/scope/ParentScope.expected            |  96 ++++++++++++++++
 .../codingstandards/cpp/scope/ParentScope.ql  |   5 +
 .../codingstandards/cpp/scope/test.cpp        |  37 ++++++
 4 files changed, 195 insertions(+), 50 deletions(-)
 create mode 100644 cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected
 create mode 100644 cpp/common/test/library/codingstandards/cpp/scope/ParentScope.ql
 create mode 100644 cpp/common/test/library/codingstandards/cpp/scope/test.cpp

diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll
index b05c5dc9c4..9af5c8e21c 100644
--- a/cpp/common/src/codingstandards/cpp/Scope.qll
+++ b/cpp/common/src/codingstandards/cpp/Scope.qll
@@ -5,54 +5,59 @@
 import cpp
 
 /**
- * Gets the parent scope of this `Element`, if any.
- * A scope is a `Type` (`Class` / `Enum`), a `Namespace`, a `Block`, a `Function`,
- * or certain kinds of `Statement`.
- * Differs from `Element::getParentScope` when `e` is a `LoopControlVariable`
+ * Internal module, exposed for testing.
  */
-private Element getParentScope(Element e) {
-  /*
-   * A `Block` cannot have a `ForStmt` as its parent scope so we have to special case
-   * for loop bodies to ensure that identifiers inside the loop bodies have the for stmt as a parent scope.
-   * If this is not the case then `i2` in the following example cannot be in `i1`'s potential scope, because
-   * the parent scope of `i1` is the `ForStmt` while the transitive closure of the parent scope for `i2` doesn't include
-   * outer scope. Blocks can only have blocks as parent scopes.
-   * void f() {
-   *  for( int i1; ... ) {
-   *    for (int i2; ...) {
-   *    }
-   *  }
-   * }
+module Internal {
+  /**
+   * Gets the parent scope of this `Element`, if any.
+   * A scope is a `Type` (`Class` / `Enum`), a `Namespace`, a `Block`, a `Function`,
+   * or certain kinds of `Statement`.
+   * Differs from `Element::getParentScope` when `e` is a `LoopControlVariable`
    */
-
-  exists(Loop loop | loop.getStmt() = e and result = loop)
-  or
-  exists(IfStmt ifStmt |
-    (ifStmt.getThen() = e or ifStmt.getElse() = e) and
-    result = ifStmt
-  )
-  or
-  exists(SwitchStmt switchStmt | switchStmt.getStmt() = e and result = switchStmt)
-  or
-  not result.(Loop).getStmt() = e and
-  not result.(IfStmt).getThen() = e and
-  not result.(IfStmt).getElse() = e and
-  not result.(SwitchStmt).getStmt() = e and
-  if exists(e.getParentScope())
-  then result = e.getParentScope()
-  else (
-    // Statements do no have a parent scope, so return the enclosing block.
-    result = e.(Stmt).getEnclosingBlock()
+  Element getParentScope(Element e) {
+    /*
+     * A `Block` cannot have a `ForStmt` as its parent scope so we have to special case
+     * for loop bodies to ensure that identifiers inside the loop bodies have the for stmt as a parent scope.
+     * If this is not the case then `i2` in the following example cannot be in `i1`'s potential scope, because
+     * the parent scope of `i1` is the `ForStmt` while the transitive closure of the parent scope for `i2` doesn't include
+     * outer scope. Blocks can only have blocks as parent scopes.
+     * void f() {
+     *  for( int i1; ... ) {
+     *    for (int i2; ...) {
+     *    }
+     *  }
+     * }
+     */
+
+    exists(Loop loop | loop.getStmt() = e and result = loop)
+    or
+    exists(IfStmt ifStmt |
+      (ifStmt.getThen() = e or ifStmt.getElse() = e) and
+      result = ifStmt
+    )
     or
-    result = e.(Expr).getEnclosingBlock()
+    exists(SwitchStmt switchStmt | switchStmt.getStmt() = e and result = switchStmt)
     or
-    // Catch block parameters don't have an enclosing scope, so attach them to the
-    // the block itself
-    exists(CatchBlock cb |
-      e = cb.getParameter() and
-      result = cb
+    not result.(Loop).getStmt() = e and
+    not result.(IfStmt).getThen() = e and
+    not result.(IfStmt).getElse() = e and
+    not result.(SwitchStmt).getStmt() = e and
+    if exists(e.getParentScope())
+    then result = e.getParentScope()
+    else (
+      // Statements do no have a parent scope, so return the enclosing block.
+      result = e.(Stmt).getEnclosingBlock()
+      or
+      result = e.(Expr).getEnclosingBlock()
+      or
+      // Catch block parameters don't have an enclosing scope, so attach them to the
+      // the block itself
+      exists(CatchBlock cb |
+        e = cb.getParameter() and
+        result = cb
+      )
     )
-  )
+  }
 }
 
 /** A variable which is defined by the user, rather than being from a third party or compiler generated. */
@@ -68,19 +73,19 @@ class UserVariable extends Variable {
 
 /** An element which is the parent scope of at least one other element in the program. */
 class Scope extends Element {
-  Scope() { this = getParentScope(_) }
+  Scope() { this = Internal::getParentScope(_) }
 
-  UserVariable getAVariable() { getParentScope(result) = this }
+  UserVariable getAVariable() { Internal::getParentScope(result) = this }
 
   int getNumberOfVariables() { result = count(getAVariable()) }
 
   Scope getAnAncestor() { result = this.getStrictParent+() }
 
-  Scope getStrictParent() { result = getParentScope(this) }
+  Scope getStrictParent() { result = Internal::getParentScope(this) }
 
-  Declaration getADeclaration() { getParentScope(result) = this }
+  Declaration getADeclaration() { Internal::getParentScope(result) = this }
 
-  Expr getAnExpr() { this = getParentScope(result) }
+  Expr getAnExpr() { this = Internal::getParentScope(result) }
 
   private predicate getLocationInfo(
     PreprocessorBranchDirective pbd, string file1, string file2, int startline1, int startline2
@@ -112,9 +117,11 @@ class Scope extends Element {
   predicate isGenerated() { this instanceof GeneratedBlockStmt }
 
   int getDepth() {
-    exists(Scope parent | parent = getParentScope(this) and result = 1 + parent.getDepth())
+    exists(Scope parent |
+      parent = Internal::getParentScope(this) and result = 1 + parent.getDepth()
+    )
     or
-    not exists(getParentScope(this)) and result = 0
+    not exists(Internal::getParentScope(this)) and result = 0
   }
 }
 
diff --git a/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected b/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected
new file mode 100644
index 0000000000..0d36cc980d
--- /dev/null
+++ b/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected
@@ -0,0 +1,96 @@
+| file://:0:0:0:0 | (unnamed parameter 0) | file://:0:0:0:0 | operator= |
+| file://:0:0:0:0 | (unnamed parameter 0) | file://:0:0:0:0 | operator= |
+| file://:0:0:0:0 | (unnamed parameter 0) | test.cpp:8:7:8:7 | operator= |
+| file://:0:0:0:0 | (unnamed parameter 0) | test.cpp:8:7:8:7 | operator= |
+| file://:0:0:0:0 | (unnamed parameter 0) | test.cpp:27:28:27:28 | (unnamed constructor) |
+| file://:0:0:0:0 | (unnamed parameter 0) | test.cpp:27:28:27:28 | (unnamed constructor) |
+| file://:0:0:0:0 | (unnamed parameter 0) | test.cpp:27:28:27:28 | operator= |
+| file://:0:0:0:0 | __va_list_tag | file://:0:0:0:0 | (global namespace) |
+| file://:0:0:0:0 | fp_offset | file://:0:0:0:0 | __va_list_tag |
+| file://:0:0:0:0 | gp_offset | file://:0:0:0:0 | __va_list_tag |
+| file://:0:0:0:0 | operator= | file://:0:0:0:0 | __va_list_tag |
+| file://:0:0:0:0 | operator= | file://:0:0:0:0 | __va_list_tag |
+| file://:0:0:0:0 | overflow_arg_area | file://:0:0:0:0 | __va_list_tag |
+| file://:0:0:0:0 | reg_save_area | file://:0:0:0:0 | __va_list_tag |
+| test.cpp:1:5:1:7 | id1 | file://:0:0:0:0 | (global namespace) |
+| test.cpp:3:11:3:13 | ns1 | file://:0:0:0:0 | (global namespace) |
+| test.cpp:4:5:4:7 | id1 | test.cpp:3:11:3:13 | ns1 |
+| test.cpp:6:11:6:13 | ns1::ns2 | test.cpp:3:11:3:13 | ns1 |
+| test.cpp:7:5:7:7 | id1 | test.cpp:6:11:6:13 | ns1::ns2 |
+| test.cpp:8:7:8:7 | C1 | test.cpp:8:7:8:8 | C1 |
+| test.cpp:8:7:8:7 | operator= | test.cpp:8:7:8:8 | C1 |
+| test.cpp:8:7:8:7 | operator= | test.cpp:8:7:8:8 | C1 |
+| test.cpp:8:7:8:8 | C1 | test.cpp:6:11:6:13 | ns1::ns2 |
+| test.cpp:9:7:9:9 | id1 | test.cpp:8:7:8:8 | C1 |
+| test.cpp:10:8:10:17 | test_scope | test.cpp:8:7:8:8 | C1 |
+| test.cpp:10:23:10:25 | id1 | test.cpp:10:8:10:17 | test_scope |
+| test.cpp:10:28:34:3 | { ... } | test.cpp:10:8:10:17 | test_scope |
+| test.cpp:11:5:33:5 | for(...;...;...) ... | test.cpp:10:28:34:3 | { ... } |
+| test.cpp:11:10:11:17 | declaration | test.cpp:10:28:34:3 | { ... } |
+| test.cpp:11:14:11:16 | id1 | test.cpp:11:5:33:5 | for(...;...;...) ... |
+| test.cpp:11:19:11:21 | id1 | test.cpp:10:28:34:3 | { ... } |
+| test.cpp:11:19:11:25 | ... < ... | test.cpp:10:28:34:3 | { ... } |
+| test.cpp:11:25:11:25 | 1 | test.cpp:10:28:34:3 | { ... } |
+| test.cpp:11:28:11:30 | id1 | test.cpp:10:28:34:3 | { ... } |
+| test.cpp:11:28:11:32 | ... ++ | test.cpp:10:28:34:3 | { ... } |
+| test.cpp:11:35:33:5 | { ... } | test.cpp:10:28:34:3 | { ... } |
+| test.cpp:11:35:33:5 | { ... } | test.cpp:11:5:33:5 | for(...;...;...) ... |
+| test.cpp:12:7:32:7 | for(...;...;...) ... | test.cpp:11:35:33:5 | { ... } |
+| test.cpp:12:12:12:19 | declaration | test.cpp:11:35:33:5 | { ... } |
+| test.cpp:12:16:12:18 | id1 | test.cpp:12:7:32:7 | for(...;...;...) ... |
+| test.cpp:12:21:12:23 | id1 | test.cpp:11:35:33:5 | { ... } |
+| test.cpp:12:21:12:27 | ... < ... | test.cpp:11:35:33:5 | { ... } |
+| test.cpp:12:27:12:27 | 1 | test.cpp:11:35:33:5 | { ... } |
+| test.cpp:12:30:12:32 | id1 | test.cpp:11:35:33:5 | { ... } |
+| test.cpp:12:30:12:34 | ... ++ | test.cpp:11:35:33:5 | { ... } |
+| test.cpp:12:37:32:7 | { ... } | test.cpp:11:35:33:5 | { ... } |
+| test.cpp:12:37:32:7 | { ... } | test.cpp:12:7:32:7 | for(...;...;...) ... |
+| test.cpp:13:9:31:9 | { ... } | test.cpp:12:37:32:7 | { ... } |
+| test.cpp:14:11:14:18 | declaration | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:14:15:14:17 | id1 | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:16:11:20:11 | if (...) ...  | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:16:15:16:17 | id1 | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:16:15:16:22 | ... == ... | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:16:22:16:22 | 0 | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:16:25:18:11 | { ... } | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:16:25:18:11 | { ... } | test.cpp:16:11:20:11 | if (...) ...  |
+| test.cpp:17:13:17:20 | declaration | test.cpp:16:25:18:11 | { ... } |
+| test.cpp:17:17:17:19 | id1 | test.cpp:16:25:18:11 | { ... } |
+| test.cpp:18:18:20:11 | { ... } | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:18:18:20:11 | { ... } | test.cpp:16:11:20:11 | if (...) ...  |
+| test.cpp:19:13:19:20 | declaration | test.cpp:18:18:20:11 | { ... } |
+| test.cpp:19:17:19:19 | id1 | test.cpp:18:18:20:11 | { ... } |
+| test.cpp:21:11:25:11 | switch (...) ...  | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:21:19:21:21 | id1 | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:21:24:25:11 | { ... } | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:21:24:25:11 | { ... } | test.cpp:21:11:25:11 | switch (...) ...  |
+| test.cpp:22:11:22:17 | case ...: | test.cpp:21:24:25:11 | { ... } |
+| test.cpp:22:16:22:16 | 0 | test.cpp:21:24:25:11 | { ... } |
+| test.cpp:23:13:23:20 | declaration | test.cpp:21:24:25:11 | { ... } |
+| test.cpp:23:17:23:19 | id1 | test.cpp:21:24:25:11 | { ... } |
+| test.cpp:24:13:24:18 | break; | test.cpp:21:24:25:11 | { ... } |
+| test.cpp:25:11:25:11 | label ...: | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:26:11:28:11 | try { ... } | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:26:15:28:11 | { ... } | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:27:13:27:53 | declaration | test.cpp:26:15:28:11 | { ... } |
+| test.cpp:27:18:27:24 | lambda1 | test.cpp:26:15:28:11 | { ... } |
+| test.cpp:27:27:27:52 | [...](...){...} | test.cpp:26:15:28:11 | { ... } |
+| test.cpp:27:27:27:52 | {...} | test.cpp:26:15:28:11 | { ... } |
+| test.cpp:27:28:27:28 | (unnamed constructor) | file://:0:0:0:0 | decltype([...](...){...}) |
+| test.cpp:27:28:27:28 | (unnamed constructor) | file://:0:0:0:0 | decltype([...](...){...}) |
+| test.cpp:27:28:27:28 | (unnamed constructor) | file://:0:0:0:0 | decltype([...](...){...}) |
+| test.cpp:27:28:27:28 | operator= | file://:0:0:0:0 | decltype([...](...){...}) |
+| test.cpp:27:29:27:29 | id1 | file://:0:0:0:0 | decltype([...](...){...}) |
+| test.cpp:27:29:27:31 | id1 | test.cpp:26:15:28:11 | { ... } |
+| test.cpp:27:33:27:33 | operator() | file://:0:0:0:0 | decltype([...](...){...}) |
+| test.cpp:27:36:27:52 | { ... } | test.cpp:27:33:27:33 | operator() |
+| test.cpp:27:38:27:50 | declaration | test.cpp:27:36:27:52 | { ... } |
+| test.cpp:27:42:27:44 | id1 | test.cpp:27:36:27:52 | { ... } |
+| test.cpp:27:47:27:49 | 10 | test.cpp:27:36:27:52 | { ... } |
+| test.cpp:27:52:27:52 | return ... | test.cpp:27:36:27:52 | { ... } |
+| test.cpp:28:24:28:26 | id1 | test.cpp:28:29:30:11 | { ... } |
+| test.cpp:28:29:30:11 | <handler> | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:28:29:30:11 | { ... } | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:29:13:29:20 | declaration | test.cpp:28:29:30:11 | { ... } |
+| test.cpp:29:17:29:19 | id1 | test.cpp:28:29:30:11 | { ... } |
+| test.cpp:34:3:34:3 | return ... | test.cpp:10:28:34:3 | { ... } |
diff --git a/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.ql b/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.ql
new file mode 100644
index 0000000000..47d27fb0f0
--- /dev/null
+++ b/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.ql
@@ -0,0 +1,5 @@
+import codingstandards.cpp.Scope
+
+from Element e, Element parent
+where Internal::getParentScope(e) = parent
+select e, parent
diff --git a/cpp/common/test/library/codingstandards/cpp/scope/test.cpp b/cpp/common/test/library/codingstandards/cpp/scope/test.cpp
new file mode 100644
index 0000000000..a0b617916d
--- /dev/null
+++ b/cpp/common/test/library/codingstandards/cpp/scope/test.cpp
@@ -0,0 +1,37 @@
+int id1;
+
+namespace ns1 {
+int id1; // COMPLIANT
+
+namespace ns2 {
+int id1; // COMPLIANT
+class C1 {
+  int id1;
+  void test_scope(int id1) {
+    for (int id1; id1 < 1; id1++) {
+      for (int id1; id1 < 1; id1++) {
+        {
+          int id1;
+
+          if (id1 == 0) {
+            int id1;
+          } else {
+            int id1;
+          }
+          switch (id1) {
+          case 0:
+            int id1;
+            break;
+          }
+          try {
+            auto lambda1 = [id1]() { int id1 = 10; };
+          } catch (int id1) {
+            int id1;
+          }
+        }
+      }
+    }
+  }
+};
+} // namespace ns2
+} // namespace ns1

From b8b41312c24fa6d9ce511a5d0543ddef08c7f6ee Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Sun, 8 Dec 2024 17:26:09 +0000
Subject: [PATCH 05/20] Scope: Address logic flaw creating multiple parents

We adjust the parent scope explicitly for loops, if statements and
switch statements, but, due to a logic bug, we previously retained
the existing results provided by Element.getParentScope().
---
 cpp/common/src/codingstandards/cpp/Scope.qll               | 7 +++----
 .../library/codingstandards/cpp/scope/ParentScope.expected | 5 -----
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll
index 9af5c8e21c..ce8c5be113 100644
--- a/cpp/common/src/codingstandards/cpp/Scope.qll
+++ b/cpp/common/src/codingstandards/cpp/Scope.qll
@@ -38,10 +38,9 @@ module Internal {
     or
     exists(SwitchStmt switchStmt | switchStmt.getStmt() = e and result = switchStmt)
     or
-    not result.(Loop).getStmt() = e and
-    not result.(IfStmt).getThen() = e and
-    not result.(IfStmt).getElse() = e and
-    not result.(SwitchStmt).getStmt() = e and
+    not exists(Loop loop | loop.getStmt() = e) and
+    not exists(IfStmt ifStmt | ifStmt.getThen() = e or ifStmt.getElse() = e) and
+    not exists(SwitchStmt switchStmt | switchStmt.getStmt() = e) and
     if exists(e.getParentScope())
     then result = e.getParentScope()
     else (
diff --git a/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected b/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected
index 0d36cc980d..6335394970 100644
--- a/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected
+++ b/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected
@@ -33,7 +33,6 @@
 | test.cpp:11:25:11:25 | 1 | test.cpp:10:28:34:3 | { ... } |
 | test.cpp:11:28:11:30 | id1 | test.cpp:10:28:34:3 | { ... } |
 | test.cpp:11:28:11:32 | ... ++ | test.cpp:10:28:34:3 | { ... } |
-| test.cpp:11:35:33:5 | { ... } | test.cpp:10:28:34:3 | { ... } |
 | test.cpp:11:35:33:5 | { ... } | test.cpp:11:5:33:5 | for(...;...;...) ... |
 | test.cpp:12:7:32:7 | for(...;...;...) ... | test.cpp:11:35:33:5 | { ... } |
 | test.cpp:12:12:12:19 | declaration | test.cpp:11:35:33:5 | { ... } |
@@ -43,7 +42,6 @@
 | test.cpp:12:27:12:27 | 1 | test.cpp:11:35:33:5 | { ... } |
 | test.cpp:12:30:12:32 | id1 | test.cpp:11:35:33:5 | { ... } |
 | test.cpp:12:30:12:34 | ... ++ | test.cpp:11:35:33:5 | { ... } |
-| test.cpp:12:37:32:7 | { ... } | test.cpp:11:35:33:5 | { ... } |
 | test.cpp:12:37:32:7 | { ... } | test.cpp:12:7:32:7 | for(...;...;...) ... |
 | test.cpp:13:9:31:9 | { ... } | test.cpp:12:37:32:7 | { ... } |
 | test.cpp:14:11:14:18 | declaration | test.cpp:13:9:31:9 | { ... } |
@@ -52,17 +50,14 @@
 | test.cpp:16:15:16:17 | id1 | test.cpp:13:9:31:9 | { ... } |
 | test.cpp:16:15:16:22 | ... == ... | test.cpp:13:9:31:9 | { ... } |
 | test.cpp:16:22:16:22 | 0 | test.cpp:13:9:31:9 | { ... } |
-| test.cpp:16:25:18:11 | { ... } | test.cpp:13:9:31:9 | { ... } |
 | test.cpp:16:25:18:11 | { ... } | test.cpp:16:11:20:11 | if (...) ...  |
 | test.cpp:17:13:17:20 | declaration | test.cpp:16:25:18:11 | { ... } |
 | test.cpp:17:17:17:19 | id1 | test.cpp:16:25:18:11 | { ... } |
-| test.cpp:18:18:20:11 | { ... } | test.cpp:13:9:31:9 | { ... } |
 | test.cpp:18:18:20:11 | { ... } | test.cpp:16:11:20:11 | if (...) ...  |
 | test.cpp:19:13:19:20 | declaration | test.cpp:18:18:20:11 | { ... } |
 | test.cpp:19:17:19:19 | id1 | test.cpp:18:18:20:11 | { ... } |
 | test.cpp:21:11:25:11 | switch (...) ...  | test.cpp:13:9:31:9 | { ... } |
 | test.cpp:21:19:21:21 | id1 | test.cpp:13:9:31:9 | { ... } |
-| test.cpp:21:24:25:11 | { ... } | test.cpp:13:9:31:9 | { ... } |
 | test.cpp:21:24:25:11 | { ... } | test.cpp:21:11:25:11 | switch (...) ...  |
 | test.cpp:22:11:22:17 | case ...: | test.cpp:21:24:25:11 | { ... } |
 | test.cpp:22:16:22:16 | 0 | test.cpp:21:24:25:11 | { ... } |

From 1f8e2baedb543b5a4318fc87d63b797865902f18 Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Sun, 8 Dec 2024 17:31:41 +0000
Subject: [PATCH 06/20] Scope: Ensure loop entries have correct parent scope

All direct children of a for loop should have the for loop itself
as the scope.
---
 cpp/common/src/codingstandards/cpp/Scope.qll         |  4 ++--
 .../codingstandards/cpp/scope/ParentScope.expected   | 12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll
index ce8c5be113..ad99ab8bd4 100644
--- a/cpp/common/src/codingstandards/cpp/Scope.qll
+++ b/cpp/common/src/codingstandards/cpp/Scope.qll
@@ -29,7 +29,7 @@ module Internal {
      * }
      */
 
-    exists(Loop loop | loop.getStmt() = e and result = loop)
+    exists(Loop loop | loop.getAChild() = e and result = loop)
     or
     exists(IfStmt ifStmt |
       (ifStmt.getThen() = e or ifStmt.getElse() = e) and
@@ -38,7 +38,7 @@ module Internal {
     or
     exists(SwitchStmt switchStmt | switchStmt.getStmt() = e and result = switchStmt)
     or
-    not exists(Loop loop | loop.getStmt() = e) and
+    not exists(Loop loop | loop.getAChild() = e) and
     not exists(IfStmt ifStmt | ifStmt.getThen() = e or ifStmt.getElse() = e) and
     not exists(SwitchStmt switchStmt | switchStmt.getStmt() = e) and
     if exists(e.getParentScope())
diff --git a/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected b/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected
index 6335394970..34a48c8065 100644
--- a/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected
+++ b/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected
@@ -26,22 +26,22 @@
 | test.cpp:10:23:10:25 | id1 | test.cpp:10:8:10:17 | test_scope |
 | test.cpp:10:28:34:3 | { ... } | test.cpp:10:8:10:17 | test_scope |
 | test.cpp:11:5:33:5 | for(...;...;...) ... | test.cpp:10:28:34:3 | { ... } |
-| test.cpp:11:10:11:17 | declaration | test.cpp:10:28:34:3 | { ... } |
+| test.cpp:11:10:11:17 | declaration | test.cpp:11:5:33:5 | for(...;...;...) ... |
 | test.cpp:11:14:11:16 | id1 | test.cpp:11:5:33:5 | for(...;...;...) ... |
 | test.cpp:11:19:11:21 | id1 | test.cpp:10:28:34:3 | { ... } |
-| test.cpp:11:19:11:25 | ... < ... | test.cpp:10:28:34:3 | { ... } |
+| test.cpp:11:19:11:25 | ... < ... | test.cpp:11:5:33:5 | for(...;...;...) ... |
 | test.cpp:11:25:11:25 | 1 | test.cpp:10:28:34:3 | { ... } |
 | test.cpp:11:28:11:30 | id1 | test.cpp:10:28:34:3 | { ... } |
-| test.cpp:11:28:11:32 | ... ++ | test.cpp:10:28:34:3 | { ... } |
+| test.cpp:11:28:11:32 | ... ++ | test.cpp:11:5:33:5 | for(...;...;...) ... |
 | test.cpp:11:35:33:5 | { ... } | test.cpp:11:5:33:5 | for(...;...;...) ... |
 | test.cpp:12:7:32:7 | for(...;...;...) ... | test.cpp:11:35:33:5 | { ... } |
-| test.cpp:12:12:12:19 | declaration | test.cpp:11:35:33:5 | { ... } |
+| test.cpp:12:12:12:19 | declaration | test.cpp:12:7:32:7 | for(...;...;...) ... |
 | test.cpp:12:16:12:18 | id1 | test.cpp:12:7:32:7 | for(...;...;...) ... |
 | test.cpp:12:21:12:23 | id1 | test.cpp:11:35:33:5 | { ... } |
-| test.cpp:12:21:12:27 | ... < ... | test.cpp:11:35:33:5 | { ... } |
+| test.cpp:12:21:12:27 | ... < ... | test.cpp:12:7:32:7 | for(...;...;...) ... |
 | test.cpp:12:27:12:27 | 1 | test.cpp:11:35:33:5 | { ... } |
 | test.cpp:12:30:12:32 | id1 | test.cpp:11:35:33:5 | { ... } |
-| test.cpp:12:30:12:34 | ... ++ | test.cpp:11:35:33:5 | { ... } |
+| test.cpp:12:30:12:34 | ... ++ | test.cpp:12:7:32:7 | for(...;...;...) ... |
 | test.cpp:12:37:32:7 | { ... } | test.cpp:12:7:32:7 | for(...;...;...) ... |
 | test.cpp:13:9:31:9 | { ... } | test.cpp:12:37:32:7 | { ... } |
 | test.cpp:14:11:14:18 | declaration | test.cpp:13:9:31:9 | { ... } |

From 63a60b1855f00c96a63e16242055ab2f660782f8 Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Sun, 8 Dec 2024 18:19:26 +0000
Subject: [PATCH 07/20] Scope: Address performance issues with
 excludedViaNestedNamespace

Add pragma_inline to ensure we consider this as a post-filtering
step.
---
 cpp/common/src/codingstandards/cpp/Scope.qll | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll
index ad99ab8bd4..3e1bb6c803 100644
--- a/cpp/common/src/codingstandards/cpp/Scope.qll
+++ b/cpp/common/src/codingstandards/cpp/Scope.qll
@@ -269,6 +269,8 @@ predicate hasBlockScope(Declaration decl) { exists(BlockStmt b | b.getADeclarati
 /**
  * identifiers in nested (named/nonglobal) namespaces are exceptions to hiding due to being able access via fully qualified ids
  */
+bindingset[outerDecl, innerDecl]
+pragma[inline_late]
 predicate excludedViaNestedNamespaces(UserVariable outerDecl, UserVariable innerDecl) {
   exists(Namespace inner, Namespace outer |
     outer.getAChildNamespace+() = inner and

From 1bd839c34c39e3546484d666ebe8b15d0e9e0451 Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Sat, 7 Dec 2024 00:43:37 +0000
Subject: [PATCH 08/20] Scope: Improve performance of hidesStrict

Improves performance by:
 - Capturing for each scope the list of names defined by nested
   scopes
 - Use that to determine hidden identifiers for a scope.
 - Separately determine the hiding identifiers for a scope.

This addresses performance issues in the now deleted predicate
getOuterScopesOfVariable_candidate().
---
 cpp/common/src/codingstandards/cpp/Scope.qll | 119 +++++++++++++------
 1 file changed, 82 insertions(+), 37 deletions(-)

diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll
index 3e1bb6c803..8e618cb21e 100644
--- a/cpp/common/src/codingstandards/cpp/Scope.qll
+++ b/cpp/common/src/codingstandards/cpp/Scope.qll
@@ -122,39 +122,77 @@ class Scope extends Element {
     or
     not exists(Internal::getParentScope(this)) and result = 0
   }
-}
 
-class GeneratedBlockStmt extends BlockStmt {
-  GeneratedBlockStmt() { this.getLocation() instanceof UnknownLocation }
-}
+  /**
+   * Holds if `name` is declared in this scope, or in a nested scope.
+   */
+  private predicate isNameDeclaredInThisOrNestedScope(string name) {
+    name = getAVariable().getName()
+    or
+    isNameDeclaredInNestedScope(name)
+  }
+
+  /**
+   * Holds if `name` is declared in a nested scope.
+   */
+  private predicate isNameDeclaredInNestedScope(string name) {
+    exists(Scope child |
+      child.getStrictParent() = this and
+      child.isNameDeclaredInThisOrNestedScope(name)
+    )
+  }
+
+  /**
+   * Holds if `name` is declared in this scope and is hidden in a child scope.
+   */
+  private predicate isDeclaredNameHiddenByChild(string name) {
+    isNameDeclaredInNestedScope(name) and
+    name = getAVariable().getName()
+  }
+
+  /**
+   * Gets a variable with `name` which is hidden in at least one nested scope.
+   */
+  UserVariable getAHiddenVariable(string name) {
+    result = getAVariable() and
+    result.getName() = name and
+    isDeclaredNameHiddenByChild(name)
+  }
 
-/** Gets a variable that is in the potential scope of variable `v`. */
-private UserVariable getPotentialScopeOfVariable_candidate(UserVariable v) {
-  exists(Scope s |
-    result = s.getAVariable() and
+  /**
+   * Holds if `name` is declared above this scope and hidden by this or a nested scope.
+   */
+  private predicate isNameDeclaredAboveHiddenByThisOrNested(string name) {
     (
-      // Variable in an ancestor scope, but only if there are less than 100 variables in this scope
-      v = s.getAnAncestor().getAVariable() and
-      s.getNumberOfVariables() < 100
+      this.getStrictParent().isDeclaredNameHiddenByChild(name) or
+      this.getStrictParent().isNameDeclaredAboveHiddenByThisOrNested(name)
+    ) and
+    isNameDeclaredInThisOrNestedScope(name)
+  }
+
+  /**
+   * Gets a variable with `name` which is declared above and hidden by a variable in this or a nested scope.
+   */
+  UserVariable getAHidingVariable(string name) {
+    isNameDeclaredAboveHiddenByThisOrNested(name) and
+    (
+      // Declared in this scope
+      getAVariable().getName() = name and
+      result = getAVariable() and
+      result.getName() = name
       or
-      // In the same scope, but not the same variable, and choose just one to report
-      v = s.getAVariable() and
-      not result = v and
-      v.getName() <= result.getName()
+      // Declared in a child scope
+      exists(Scope child |
+        child.getStrictParent() = this and
+        child.isNameDeclaredInThisOrNestedScope(name) and
+        result = child.getAHidingVariable(name)
+      )
     )
-  )
+  }
 }
 
-/** Gets a variable that is in the potential scope of variable `v`. */
-private UserVariable getOuterScopesOfVariable_candidate(UserVariable v) {
-  exists(Scope s |
-    result = s.getAVariable() and
-    (
-      // Variable in an ancestor scope, but only if there are less than 100 variables in this scope
-      v = s.getAnAncestor().getAVariable() and
-      s.getNumberOfVariables() < 100
-    )
-  )
+class GeneratedBlockStmt extends BlockStmt {
+  GeneratedBlockStmt() { this.getLocation() instanceof UnknownLocation }
 }
 
 /** Holds if there exists a translation unit that includes both `f1` and `f2`. */
@@ -167,12 +205,17 @@ predicate inSameTranslationUnit(File f1, File f2) {
 }
 
 /**
- * Gets a user variable which occurs in the "outer scope" of variable `v`.
+ * Holds if there exists a translation unit that includes both `f1` and `f2`.
+ *
+ * This version is late bound.
  */
-cached
-UserVariable getPotentialScopeOfVariableStrict(UserVariable v) {
-  result = getOuterScopesOfVariable_candidate(v) and
-  inSameTranslationUnit(v.getFile(), result.getFile())
+bindingset[f1, f2]
+pragma[inline_late]
+predicate inSameTranslationUnitLate(File f1, File f2) {
+  exists(TranslationUnit c |
+    c.getAUserFile() = f1 and
+    c.getAUserFile() = f2
+  )
 }
 
 /** A file that is a C/C++ source file */
@@ -200,12 +243,14 @@ class TranslationUnit extends SourceFile {
   }
 }
 
-/** Holds if `v2` may hide `v1`. */
-private predicate hides_candidateStrict(UserVariable v1, UserVariable v2) {
-  not v1 = v2 and
-  v2 = getPotentialScopeOfVariableStrict(v1) and
-  v1.getName() = v2.getName() and
-  // Member variables cannot hide other variables nor be hidden because the can be referenced through their qualified name.
+/** Holds if `v2` strictly (`v2` is in an inner scope compared to `v1`) hides `v1`. */
+predicate hides_candidateStrict(UserVariable v1, UserVariable v2) {
+  exists(Scope s, string name |
+    v1 = s.getStrictParent().getAHiddenVariable(name) and
+    v2 = s.getAHidingVariable(name) and
+    not v1 = v2
+  ) and
+  inSameTranslationUnitLate(v1.getFile(), v2.getFile()) and
   not (v1.isMember() or v2.isMember()) and
   (
     // If v1 is a local variable, ensure that v1 is declared before v2

From f25507e821aeae14df6e2b676599b57768016387 Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Sun, 8 Dec 2024 19:20:03 +0000
Subject: [PATCH 09/20] Scope: Add a child scope accessor

---
 cpp/common/src/codingstandards/cpp/Scope.qll | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll
index 8e618cb21e..1c9a621b39 100644
--- a/cpp/common/src/codingstandards/cpp/Scope.qll
+++ b/cpp/common/src/codingstandards/cpp/Scope.qll
@@ -80,6 +80,8 @@ class Scope extends Element {
 
   Scope getAnAncestor() { result = this.getStrictParent+() }
 
+  Scope getAChildScope() { result.getStrictParent() = this }
+
   Scope getStrictParent() { result = Internal::getParentScope(this) }
 
   Declaration getADeclaration() { Internal::getParentScope(result) = this }

From b5ff407e07197d229664b462503916f1da268104 Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Sun, 8 Dec 2024 19:20:40 +0000
Subject: [PATCH 10/20] Scope: Adopt use of getAChildScope()

---
 cpp/common/src/codingstandards/cpp/Scope.qll | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll
index 1c9a621b39..d0f5995537 100644
--- a/cpp/common/src/codingstandards/cpp/Scope.qll
+++ b/cpp/common/src/codingstandards/cpp/Scope.qll
@@ -138,10 +138,7 @@ class Scope extends Element {
    * Holds if `name` is declared in a nested scope.
    */
   private predicate isNameDeclaredInNestedScope(string name) {
-    exists(Scope child |
-      child.getStrictParent() = this and
-      child.isNameDeclaredInThisOrNestedScope(name)
-    )
+    this.getAChildScope().isNameDeclaredInThisOrNestedScope(name)
   }
 
   /**
@@ -173,7 +170,8 @@ class Scope extends Element {
   }
 
   /**
-   * Gets a variable with `name` which is declared above and hidden by a variable in this or a nested scope.
+   * Gets a variable with `name` which is declared in a scope above this one, and hidden by a variable in this or a
+   * nested scope.
    */
   UserVariable getAHidingVariable(string name) {
     isNameDeclaredAboveHiddenByThisOrNested(name) and
@@ -185,7 +183,7 @@ class Scope extends Element {
       or
       // Declared in a child scope
       exists(Scope child |
-        child.getStrictParent() = this and
+        getAChildScope() = child and
         child.isNameDeclaredInThisOrNestedScope(name) and
         result = child.getAHidingVariable(name)
       )
@@ -248,8 +246,8 @@ class TranslationUnit extends SourceFile {
 /** Holds if `v2` strictly (`v2` is in an inner scope compared to `v1`) hides `v1`. */
 predicate hides_candidateStrict(UserVariable v1, UserVariable v2) {
   exists(Scope s, string name |
-    v1 = s.getStrictParent().getAHiddenVariable(name) and
-    v2 = s.getAHidingVariable(name) and
+    v1 = s.getAHiddenVariable(name) and
+    v2 = s.getAChildScope().getAHidingVariable(name) and
     not v1 = v2
   ) and
   inSameTranslationUnitLate(v1.getFile(), v2.getFile()) and

From dbd3fe6eeb10a1db03b2599765632a80539557e7 Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Sun, 8 Dec 2024 22:27:35 +0000
Subject: [PATCH 11/20] Scope: Fix the parent scope of catch blocks

We now tie the Handler into the TryStmt, and catch-block parameters
into the Handler for a consistent AST hierarchy.
---
 cpp/common/src/codingstandards/cpp/Scope.qll   | 18 ++++++++++--------
 .../cpp/scope/ParentScope.expected             |  4 ++--
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll
index d0f5995537..1027e251f2 100644
--- a/cpp/common/src/codingstandards/cpp/Scope.qll
+++ b/cpp/common/src/codingstandards/cpp/Scope.qll
@@ -38,23 +38,25 @@ module Internal {
     or
     exists(SwitchStmt switchStmt | switchStmt.getStmt() = e and result = switchStmt)
     or
+    // A catch-block parameter, whose parent is the `Handler`
+    exists(CatchBlock c | c.getParameter() = e and result = c.getParent())
+    or
+    // A catch-block `Handler`, whose parent is the `TryStmt`
+    e.(Handler).getParent() = result
+    or
     not exists(Loop loop | loop.getAChild() = e) and
     not exists(IfStmt ifStmt | ifStmt.getThen() = e or ifStmt.getElse() = e) and
     not exists(SwitchStmt switchStmt | switchStmt.getStmt() = e) and
+    not exists(CatchBlock c | c.getParameter() = e) and
+    not e instanceof Handler and
     if exists(e.getParentScope())
     then result = e.getParentScope()
     else (
-      // Statements do no have a parent scope, so return the enclosing block.
+      // Statements do not have a parent scope, so return the enclosing block.
       result = e.(Stmt).getEnclosingBlock()
       or
+      // Expressions do not have a parent scope, so return the enclosing block.
       result = e.(Expr).getEnclosingBlock()
-      or
-      // Catch block parameters don't have an enclosing scope, so attach them to the
-      // the block itself
-      exists(CatchBlock cb |
-        e = cb.getParameter() and
-        result = cb
-      )
     )
   }
 }
diff --git a/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected b/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected
index 34a48c8065..e2152773af 100644
--- a/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected
+++ b/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected
@@ -83,8 +83,8 @@
 | test.cpp:27:42:27:44 | id1 | test.cpp:27:36:27:52 | { ... } |
 | test.cpp:27:47:27:49 | 10 | test.cpp:27:36:27:52 | { ... } |
 | test.cpp:27:52:27:52 | return ... | test.cpp:27:36:27:52 | { ... } |
-| test.cpp:28:24:28:26 | id1 | test.cpp:28:29:30:11 | { ... } |
-| test.cpp:28:29:30:11 | <handler> | test.cpp:13:9:31:9 | { ... } |
+| test.cpp:28:24:28:26 | id1 | test.cpp:28:29:30:11 | <handler> |
+| test.cpp:28:29:30:11 | <handler> | test.cpp:26:11:28:11 | try { ... } |
 | test.cpp:28:29:30:11 | { ... } | test.cpp:13:9:31:9 | { ... } |
 | test.cpp:29:13:29:20 | declaration | test.cpp:28:29:30:11 | { ... } |
 | test.cpp:29:17:29:19 | id1 | test.cpp:28:29:30:11 | { ... } |

From 61521e0144707fe65c611988f2412d86af94e07c Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Sun, 8 Dec 2024 23:20:37 +0000
Subject: [PATCH 12/20] Scope: Simplify mechanism for identifying declaration
 order

---
 cpp/common/src/codingstandards/cpp/Scope.qll | 50 +++++++-------------
 1 file changed, 16 insertions(+), 34 deletions(-)

diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll
index 1027e251f2..c716896356 100644
--- a/cpp/common/src/codingstandards/cpp/Scope.qll
+++ b/cpp/common/src/codingstandards/cpp/Scope.qll
@@ -247,48 +247,30 @@ class TranslationUnit extends SourceFile {
 
 /** Holds if `v2` strictly (`v2` is in an inner scope compared to `v1`) hides `v1`. */
 predicate hides_candidateStrict(UserVariable v1, UserVariable v2) {
-  exists(Scope s, string name |
-    v1 = s.getAHiddenVariable(name) and
-    v2 = s.getAChildScope().getAHidingVariable(name) and
+  exists(Scope parentScope, Scope childScope, string name |
+    v1 = parentScope.getAHiddenVariable(name) and
+    childScope = parentScope.getAChildScope() and
+    v2 = childScope.getAHidingVariable(name) and
     not v1 = v2
-  ) and
-  inSameTranslationUnitLate(v1.getFile(), v2.getFile()) and
-  not (v1.isMember() or v2.isMember()) and
-  (
-    // If v1 is a local variable, ensure that v1 is declared before v2
+  |
+    // If v1 is a local variable defined in a `DeclStmt` ensure that it is declared before `v2`,
+    // otherwise it would not be hidden
     (
-      v1 instanceof LocalVariable and
-      // Ignore variables declared in conditional expressions, as they apply to
-      // the nested scope
-      not v1 = any(ConditionDeclExpr cde).getVariable() and
-      // Ignore variables declared in loops
-      not exists(Loop l | l.getADeclaration() = v1)
+      parentScope instanceof BlockStmt and
+      exists(DeclStmt ds | ds.getADeclaration() = v1) and
+      exists(parentScope.(BlockStmt).getIndexOfStmt(childScope))
     )
     implies
     exists(BlockStmt bs, DeclStmt v1Stmt, Stmt v2Stmt |
-      v1 = v1Stmt.getADeclaration() and
-      getEnclosingStmt(v2).getParentStmt*() = v2Stmt
+      bs = parentScope and
+      v2Stmt = childScope and
+      v1Stmt.getADeclaration() = v1
     |
       bs.getIndexOfStmt(v1Stmt) <= bs.getIndexOfStmt(v2Stmt)
     )
-  )
-}
-
-/**
- * Gets the enclosing statement of the given variable, if any.
- */
-private Stmt getEnclosingStmt(LocalScopeVariable v) {
-  result.(DeclStmt).getADeclaration() = v
-  or
-  exists(ConditionDeclExpr cde |
-    cde.getVariable() = v and
-    result = cde.getEnclosingStmt()
-  )
-  or
-  exists(CatchBlock cb |
-    cb.getParameter() = v and
-    result = cb.getEnclosingStmt()
-  )
+  ) and
+  inSameTranslationUnitLate(v1.getFile(), v2.getFile()) and
+  not (v1.isMember() or v2.isMember())
 }
 
 /** Holds if `v2` strictly (`v2` is in an inner scope compared to `v1`) hides `v1`. */

From 47b3fb272f27f2fd7f06ab09d0c1195a7d06a16d Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Mon, 9 Dec 2024 12:15:50 +0000
Subject: [PATCH 13/20] Scope: refactor hides calculation to expose pairs of
 variables

Behaviour preserving refactor to allow future filtering of invalid
pairs of variables during the traversal algorithm.

For example, whether a variable declared within a lambda variable
hides an outer scope variable depends on the type and nature of the
variable.

By exposing pairs of candidate variables, we can more easily filter
on these conditions.
---
 cpp/common/src/codingstandards/cpp/Scope.qll | 54 ++++++++++++--------
 1 file changed, 33 insertions(+), 21 deletions(-)

diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll
index c716896356..6ca3fc8657 100644
--- a/cpp/common/src/codingstandards/cpp/Scope.qll
+++ b/cpp/common/src/codingstandards/cpp/Scope.qll
@@ -78,6 +78,14 @@ class Scope extends Element {
 
   UserVariable getAVariable() { Internal::getParentScope(result) = this }
 
+  /**
+   * Gets the `Variable` with the given `name` that is declared in this scope.
+   */
+  UserVariable getVariable(string name) {
+    result = getAVariable() and
+    result.getName() = name
+  }
+
   int getNumberOfVariables() { result = count(getAVariable()) }
 
   Scope getAnAncestor() { result = this.getStrictParent+() }
@@ -152,9 +160,9 @@ class Scope extends Element {
   }
 
   /**
-   * Gets a variable with `name` which is hidden in at least one nested scope.
+   * Gets a variable with `name` which is potentially hidden in at least one nested scope.
    */
-  UserVariable getAHiddenVariable(string name) {
+  private UserVariable getAPotentiallyHiddenVariable(string name) {
     result = getAVariable() and
     result.getName() = name and
     isDeclaredNameHiddenByChild(name)
@@ -163,34 +171,43 @@ class Scope extends Element {
   /**
    * Holds if `name` is declared above this scope and hidden by this or a nested scope.
    */
-  private predicate isNameDeclaredAboveHiddenByThisOrNested(string name) {
-    (
-      this.getStrictParent().isDeclaredNameHiddenByChild(name) or
-      this.getStrictParent().isNameDeclaredAboveHiddenByThisOrNested(name)
+  UserVariable getAVariableHiddenByThisOrNestedScope(string name) {
+    exists(Scope parent | parent = this.getStrictParent() |
+      result = parent.getAPotentiallyHiddenVariable(name) or
+      result = parent.getAVariableHiddenByThisOrNestedScope(name)
     ) and
     isNameDeclaredInThisOrNestedScope(name)
   }
 
   /**
-   * Gets a variable with `name` which is declared in a scope above this one, and hidden by a variable in this or a
-   * nested scope.
+   * Holds if `hiddenVariable` and `hidingVariable` are a candidate hiding pair at this scope.
    */
-  UserVariable getAHidingVariable(string name) {
-    isNameDeclaredAboveHiddenByThisOrNested(name) and
+  private predicate hidesCandidate(
+    UserVariable hiddenVariable, UserVariable hidingVariable, string name
+  ) {
     (
       // Declared in this scope
-      getAVariable().getName() = name and
-      result = getAVariable() and
-      result.getName() = name
+      hidingVariable = getVariable(name) and
+      hiddenVariable = getAVariableHiddenByThisOrNestedScope(name)
       or
       // Declared in a child scope
       exists(Scope child |
         getAChildScope() = child and
-        child.isNameDeclaredInThisOrNestedScope(name) and
-        result = child.getAHidingVariable(name)
+        child.hidesCandidate(hiddenVariable, hidingVariable, name)
       )
     )
   }
+
+  /**
+   * Holds if `hiddenVariable` is declared in this scope and hidden by `hidingVariable`.
+   */
+  predicate hides(UserVariable hiddenVariable, UserVariable hidingVariable, Scope childScope) {
+    exists(string name |
+      hiddenVariable = getAPotentiallyHiddenVariable(name) and
+      childScope = getAChildScope() and
+      childScope.hidesCandidate(hiddenVariable, hidingVariable, name)
+    )
+  }
 }
 
 class GeneratedBlockStmt extends BlockStmt {
@@ -247,12 +264,7 @@ class TranslationUnit extends SourceFile {
 
 /** Holds if `v2` strictly (`v2` is in an inner scope compared to `v1`) hides `v1`. */
 predicate hides_candidateStrict(UserVariable v1, UserVariable v2) {
-  exists(Scope parentScope, Scope childScope, string name |
-    v1 = parentScope.getAHiddenVariable(name) and
-    childScope = parentScope.getAChildScope() and
-    v2 = childScope.getAHidingVariable(name) and
-    not v1 = v2
-  |
+  exists(Scope parentScope, Scope childScope | parentScope.hides(v1, v2, childScope) |
     // If v1 is a local variable defined in a `DeclStmt` ensure that it is declared before `v2`,
     // otherwise it would not be hidden
     (

From 9b7e1299fc91d9845f5e9250d33102b6fb627755 Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Mon, 9 Dec 2024 12:27:50 +0000
Subject: [PATCH 14/20] Scope: Special case lambda expressions

Lambda expressions have special visibility rules that affect
identifier hiding, which we incorporate into the Scope hiding
calculations.

Note: Lambda expressions are not currently tied into the parent
scope hierarchy, so this change doesn't affect calculations until
getParentScope(Element e) is extended to support them.
---
 cpp/common/src/codingstandards/cpp/Scope.qll | 59 ++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll
index 6ca3fc8657..c705137d8c 100644
--- a/cpp/common/src/codingstandards/cpp/Scope.qll
+++ b/cpp/common/src/codingstandards/cpp/Scope.qll
@@ -3,6 +3,19 @@
  */
 
 import cpp
+import codingstandards.cpp.ConstHelpers
+
+/**
+ * a `Variable` that is nonvolatile and const
+ * and of type `IntegralOrEnumType`
+ */
+class NonVolatileConstIntegralOrEnumVariable extends Variable {
+  NonVolatileConstIntegralOrEnumVariable() {
+    not this.isVolatile() and
+    this.isConst() and
+    this.getUnspecifiedType() instanceof IntegralOrEnumType
+  }
+}
 
 /**
  * Internal module, exposed for testing.
@@ -210,6 +223,52 @@ class Scope extends Element {
   }
 }
 
+/**
+ * A scope representing the generated `operator()` of a lambda function.
+ */
+class LambdaScope extends Scope {
+  Closure closure;
+
+  LambdaScope() { this = closure.getLambdaFunction() }
+
+  override UserVariable getAVariableHiddenByThisOrNestedScope(string name) {
+    // Handle special cases for lambdas
+    exists(UserVariable hiddenVariable, LambdaExpression lambdaExpr |
+      // Find the variable that is potentially hidden inside the lambda
+      hiddenVariable = super.getAVariableHiddenByThisOrNestedScope(name) and
+      result = hiddenVariable and
+      lambdaExpr = closure.getLambdaExpression()
+    |
+      // A definition can be hidden if it is in scope and it is captured by the lambda,
+      exists(LambdaCapture cap |
+        lambdaExpr.getACapture() = cap and
+        // The outer declaration is captured by the lambda
+        hiddenVariable.getAnAccess() = cap.getInitializer()
+      )
+      or
+      // it is is non-local,
+      hiddenVariable instanceof GlobalVariable
+      or
+      // it has static or thread local storage duration,
+      (hiddenVariable.isThreadLocal() or hiddenVariable.isStatic())
+      or
+      //it is a reference that has been initialized with a constant expression.
+      hiddenVariable.getType().stripTopLevelSpecifiers() instanceof ReferenceType and
+      hiddenVariable.getInitializer().getExpr() instanceof Literal
+      or
+      // //it const non-volatile integral or enumeration type and has been initialized with a constant expression
+      hiddenVariable instanceof NonVolatileConstIntegralOrEnumVariable and
+      hiddenVariable.getInitializer().getExpr() instanceof Literal
+      or
+      //it is constexpr and has no mutable members
+      hiddenVariable.isConstexpr() and
+      not exists(Class c |
+        c = hiddenVariable.getType() and not c.getAMember() instanceof MutableVariable
+      )
+    )
+  }
+}
+
 class GeneratedBlockStmt extends BlockStmt {
   GeneratedBlockStmt() { this.getLocation() instanceof UnknownLocation }
 }

From 117d0fba033e58ac03d7903ada230e2a09114972 Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Mon, 9 Dec 2024 12:30:30 +0000
Subject: [PATCH 15/20] Scope: Extend getParentScope for lambda expressions

Lambda functions are tied into the parent statement of their
declaring lambda expression, which enables Scope's hiding
predicates to calculate hiding behaviour for lambda expressions.
---
 cpp/common/src/codingstandards/cpp/Scope.qll          | 11 +++++++++++
 .../codingstandards/cpp/scope/ParentScope.expected    |  2 +-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll
index c705137d8c..5438c17133 100644
--- a/cpp/common/src/codingstandards/cpp/Scope.qll
+++ b/cpp/common/src/codingstandards/cpp/Scope.qll
@@ -57,11 +57,22 @@ module Internal {
     // A catch-block `Handler`, whose parent is the `TryStmt`
     e.(Handler).getParent() = result
     or
+    // The parent scope of a lambda is the scope in which the lambda expression is defined.
+    //
+    // Lambda functions are defined in a generated `Closure` class, as the `operator()` function. We choose the
+    // enclosing statement of the lambda expression as the parent scope of the lambda function. This is so we can
+    // determine the order of definition if a variable is defined in the same scope as the lambda expression.
+    exists(Closure lambdaClosure |
+      lambdaClosure.getLambdaFunction() = e and
+      lambdaClosure.getLambdaExpression().getEnclosingStmt() = result
+    )
+    or
     not exists(Loop loop | loop.getAChild() = e) and
     not exists(IfStmt ifStmt | ifStmt.getThen() = e or ifStmt.getElse() = e) and
     not exists(SwitchStmt switchStmt | switchStmt.getStmt() = e) and
     not exists(CatchBlock c | c.getParameter() = e) and
     not e instanceof Handler and
+    not exists(Closure lambdaClosure | lambdaClosure.getLambdaFunction() = e) and
     if exists(e.getParentScope())
     then result = e.getParentScope()
     else (
diff --git a/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected b/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected
index e2152773af..90aa3b30c8 100644
--- a/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected
+++ b/cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected
@@ -77,7 +77,7 @@
 | test.cpp:27:28:27:28 | operator= | file://:0:0:0:0 | decltype([...](...){...}) |
 | test.cpp:27:29:27:29 | id1 | file://:0:0:0:0 | decltype([...](...){...}) |
 | test.cpp:27:29:27:31 | id1 | test.cpp:26:15:28:11 | { ... } |
-| test.cpp:27:33:27:33 | operator() | file://:0:0:0:0 | decltype([...](...){...}) |
+| test.cpp:27:33:27:33 | operator() | test.cpp:27:13:27:53 | declaration |
 | test.cpp:27:36:27:52 | { ... } | test.cpp:27:33:27:33 | operator() |
 | test.cpp:27:38:27:50 | declaration | test.cpp:27:36:27:52 | { ... } |
 | test.cpp:27:42:27:44 | id1 | test.cpp:27:36:27:52 | { ... } |

From 411ecde2794bb92bb6981ee6107044d66d5de4ac Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Mon, 9 Dec 2024 12:32:08 +0000
Subject: [PATCH 16/20] IdentifierHidden: remove lambda special casing

This removes the special handling of lambda expressions, which
was causing performance issues.

Instead, we rely on the new behviour of the Scope library, which
calculates identifier hiding for lambda expressions as part of the
main calculation.

This has one semantic change - the new code applies
`isInSameTranslationUnit`, which reduces false positives where the
identifier "hiding" in a lambda occurred with an outer variable in
a different translation unit.
---
 .../identifierhidden/IdentifierHidden.qll     | 66 +------------------
 1 file changed, 1 insertion(+), 65 deletions(-)

diff --git a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll
index 9534c2f78a..39d24299b8 100644
--- a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll
+++ b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll
@@ -9,75 +9,11 @@ import cpp
 import codingstandards.cpp.Customizations
 import codingstandards.cpp.Exclusions
 import codingstandards.cpp.Scope
-import codingstandards.cpp.ConstHelpers
 
 abstract class IdentifierHiddenSharedQuery extends Query { }
 
 Query getQuery() { result instanceof IdentifierHiddenSharedQuery }
 
-/**
- * a `Variable` that is nonvolatile and const
- * and of type `IntegralOrEnumType`
- */
-class NonVolatileConstIntegralOrEnumVariable extends Variable {
-  NonVolatileConstIntegralOrEnumVariable() {
-    not this.isVolatile() and
-    this.isConst() and
-    this.getUnspecifiedType() instanceof IntegralOrEnumType
-  }
-}
-
-/**
- * Holds if declaration `innerDecl`, declared in a lambda, hides a declaration `outerDecl` by the lambda.
- */
-predicate hiddenInLambda(UserVariable outerDecl, UserVariable innerDecl) {
-  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 |
-        lambdaExpr.getACapture() = cap and
-        // The outer declaration is captured by the lambda
-        outerDecl.getAnAccess() = cap.getInitializer()
-      )
-      or
-      // it is is non-local,
-      outerDecl instanceof GlobalVariable
-      or
-      // it has static or thread local storage duration,
-      (outerDecl.isThreadLocal() or outerDecl.isStatic())
-      or
-      //it is a reference that has been initialized with a constant expression.
-      outerDecl.getType().stripTopLevelSpecifiers() instanceof ReferenceType and
-      outerDecl.getInitializer().getExpr() instanceof Literal
-      or
-      // //it const non-volatile integral or enumeration type and has been initialized with a constant expression
-      outerDecl instanceof NonVolatileConstIntegralOrEnumVariable and
-      outerDecl.getInitializer().getExpr() instanceof Literal
-      or
-      //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()
-  )
-}
-
 query predicate problems(
   UserVariable innerDecl, string message, UserVariable outerDecl, string varName
 ) {
@@ -86,7 +22,7 @@ query predicate problems(
   //ignore template variables for this rule
   not outerDecl instanceof TemplateVariable and
   not innerDecl instanceof TemplateVariable and
-  (hidesStrict(outerDecl, innerDecl) or hiddenInLambda(outerDecl, innerDecl)) and
+  hidesStrict(outerDecl, innerDecl) and
   not excludedViaNestedNamespaces(outerDecl, innerDecl) and
   varName = outerDecl.getName() and
   message = "Variable is hiding variable $@."

From 52e1bc142013d35d1ebb6050111b7939dc1f25cb Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Mon, 9 Dec 2024 12:54:54 +0000
Subject: [PATCH 17/20] IdentifierHiding - Add change note

---
 change_notes/2024-12-08-identifier-hiding.md | 4 ++++
 1 file changed, 4 insertions(+)
 create mode 100644 change_notes/2024-12-08-identifier-hiding.md

diff --git a/change_notes/2024-12-08-identifier-hiding.md b/change_notes/2024-12-08-identifier-hiding.md
new file mode 100644
index 0000000000..0600c9e6ee
--- /dev/null
+++ b/change_notes/2024-12-08-identifier-hiding.md
@@ -0,0 +1,4 @@
+ - `A2-10-1` - `IdentifierHiding.ql`:
+   - Improved evaluation performance.
+   - Addressed false negatives where nested loops used the same variable name.
+   - Exclude cases where a variable declared in a lambda expression shadowed a globa or namespace variable that did not appear in the same translation unit.

From cf315bad52bb20208ab84809f90310e3875bb7b3 Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Mon, 9 Dec 2024 12:57:12 +0000
Subject: [PATCH 18/20] Add extra change note entry

---
 change_notes/2024-12-08-identifier-hiding.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/change_notes/2024-12-08-identifier-hiding.md b/change_notes/2024-12-08-identifier-hiding.md
index 0600c9e6ee..c983f5390a 100644
--- a/change_notes/2024-12-08-identifier-hiding.md
+++ b/change_notes/2024-12-08-identifier-hiding.md
@@ -2,3 +2,6 @@
    - Improved evaluation performance.
    - Addressed false negatives where nested loops used the same variable name.
    - Exclude cases where a variable declared in a lambda expression shadowed a globa or namespace variable that did not appear in the same translation unit.
+ - `RULE-5-3` - `IdentifierHidingC.ql`:
+   - Improved evaluation performance.
+   - Addressed false negatives where nested loops used the same variable name.
\ No newline at end of file

From c56e1ce54401c05d5f27c9c86a843625f6f6b37c Mon Sep 17 00:00:00 2001
From: Luke Cartey <lcartey@github.com>
Date: Mon, 9 Dec 2024 23:34:03 +0000
Subject: [PATCH 19/20] TypographicallyDifferent: Update after changes to Scope

Scope no longer provides a suitable predicate for determining
variables in nested scopes.

Instead, first determine the set of conflicting names, then
identify a set of variables which are conflicting, and are hidden
within a nested scope.
---
 ...entifiersNotTypographicallyUnambiguous.qll | 47 ++++++++++++++++---
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/cpp/common/src/codingstandards/cpp/rules/differentidentifiersnottypographicallyunambiguous/DifferentIdentifiersNotTypographicallyUnambiguous.qll b/cpp/common/src/codingstandards/cpp/rules/differentidentifiersnottypographicallyunambiguous/DifferentIdentifiersNotTypographicallyUnambiguous.qll
index 87a4580ab3..5c7475883e 100644
--- a/cpp/common/src/codingstandards/cpp/rules/differentidentifiersnottypographicallyunambiguous/DifferentIdentifiersNotTypographicallyUnambiguous.qll
+++ b/cpp/common/src/codingstandards/cpp/rules/differentidentifiersnottypographicallyunambiguous/DifferentIdentifiersNotTypographicallyUnambiguous.qll
@@ -46,16 +46,32 @@ string step1(string s) {
 
 string step2(string s) { s = "m_" and result = "rn" }
 
-predicate violation(UserVariable v1, UserVariable v2) {
-  v2 = getPotentialScopeOfVariable(v1) and
+class VariableName extends string {
+  VariableName() { exists(UserVariable uv | uv.getName() = this) }
+
+  string getCanon() {
+    result =
+      this.toLowerCase()
+          .replaceAll("_", "")
+          .regexpReplaceAll("[il]", "1")
+          .replaceAll("s", "5")
+          .replaceAll("z", "2")
+          .replaceAll("b", "8")
+          .replaceAll("h", "n")
+          .replaceAll("m", "rn")
+          .replaceAll("o", "0")
+  }
+}
+
+predicate isConflictingName(VariableName name1, VariableName name2) {
   exists(string s1, string s2 |
     // over-approximate a match, because it is cheaper to compute
-    getCanon(v1) = getCanon(v2) and
-    v1 != v2 and
-    not v1.getName() = v2.getName() and
+    name1.getCanon() = name2.getCanon() and
+    // Exclude identical names
+    not name1 = name2 and
     // expand 'm' to 'm_' to match either 'm_' or 'rn'
-    s1 = v1.getName().replaceAll("_", "").replaceAll("m", "m_") and
-    s2 = v2.getName().replaceAll("_", "").replaceAll("m", "m_") and
+    s1 = name1.replaceAll("_", "").replaceAll("m", "m_") and
+    s2 = name2.replaceAll("_", "").replaceAll("m", "m_") and
     // at this point the strings must have equal length, the substitutions do not expand nor contract the string
     s1.length() = s2.length() and
     forall(int i | i in [0 .. s1.length() - 1] |
@@ -87,6 +103,23 @@ predicate violation(UserVariable v1, UserVariable v2) {
   )
 }
 
+predicate violation(UserVariable v1, UserVariable v2) {
+  exists(string name1, string name2 |
+    isConflictingName(name1, name2) and
+    exists(Scope parentScope, Scope childScope |
+      parentScope.getVariable(name1) = v1 and
+      childScope.getVariable(name2) = v2
+    |
+      childScope.getStrictParent+() = parentScope
+      or
+      // Disambiguate names in the same scope by name order
+      childScope = parentScope and
+      name1 < name2
+    ) and
+    inSameTranslationUnitLate(v1.getFile(), v2.getFile())
+  )
+}
+
 query predicate problems(
   UserVariable v, string message, UserVariable v1, string v1Description, UserVariable v2,
   string v2Description

From 4d31f5f11cd33031a87d1c1ff070c791f0049529 Mon Sep 17 00:00:00 2001
From: Kristen Newbury <knewbury01@github.com>
Date: Tue, 10 Dec 2024 01:16:51 -0500
Subject: [PATCH 20/20] Update change_notes/2024-12-08-identifier-hiding.md

Co-authored-by: Fernando Jose <fernando.jose@woven.toyota>
---
 change_notes/2024-12-08-identifier-hiding.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/change_notes/2024-12-08-identifier-hiding.md b/change_notes/2024-12-08-identifier-hiding.md
index c983f5390a..b769b16e57 100644
--- a/change_notes/2024-12-08-identifier-hiding.md
+++ b/change_notes/2024-12-08-identifier-hiding.md
@@ -1,7 +1,7 @@
  - `A2-10-1` - `IdentifierHiding.ql`:
    - Improved evaluation performance.
    - Addressed false negatives where nested loops used the same variable name.
-   - Exclude cases where a variable declared in a lambda expression shadowed a globa or namespace variable that did not appear in the same translation unit.
+   - Exclude cases where a variable declared in a lambda expression shadowed a global or namespace variable that did not appear in the same translation unit.
  - `RULE-5-3` - `IdentifierHidingC.ql`:
    - Improved evaluation performance.
    - Addressed false negatives where nested loops used the same variable name.
\ No newline at end of file