Merge pull request #9419 from github/revert-9373-cfg-for-key-paths

Revert "Swift: Control-flow for key paths"
2025-12-17 01:03:14 +01:00 · 2022-06-02 15:26:07 +02:00
parent 8b1605a617 c9d109ddd3
commit 2187bf5dde
7 changed files with 16 additions and 435 deletions
--- a/swift/ql/lib/codeql/swift/controlflow/BasicBlocks.qll
+++ b/swift/ql/lib/codeql/swift/controlflow/BasicBlocks.qll
@@ -207,10 +207,6 @@ private module JoinBlockPredecessors {
    isPropertySetterElement(n, _, result)
    or
    isPropertyObserverElement(n, _, result)
-    or
-    result = n.(KeyPathElement).getAst()
-    or
-    result = n.(FuncDeclElement).getAst()
  }

  int getId(JoinBlockPredecessor jbp) {
--- a/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowElements.qll
+++ b/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowElements.qll
@@ -10,8 +10,7 @@ newtype TControlFlowElement =
  } or
  TPropertyObserverElement(AccessorDecl observer, AssignExpr assign) {
    isPropertyObserverElement(observer, assign)
-  } or
-  TKeyPathElement(KeyPathExpr expr)
+  }

 predicate isLValue(Expr e) { any(AssignExpr assign).getDest() = e }

@@ -172,18 +171,4 @@ class FuncDeclElement extends ControlFlowElement, TFuncDeclElement {
  override string toString() { result = func.toString() }

  override Location getLocation() { result = func.getLocation() }
-
-  AbstractFunctionDecl getAst() { result = func }
-}
-
-class KeyPathElement extends ControlFlowElement, TKeyPathElement {
-  KeyPathExpr expr;
-
-  KeyPathElement() { this = TKeyPathElement(expr) }
-
-  override Location getLocation() { result = expr.getLocation() }
-
-  KeyPathExpr getAst() { result = expr }
-
-  override string toString() { result = expr.toString() }
 }
--- a/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll
@@ -47,23 +47,19 @@ module CfgScope {
  }

  private class BodyStmtCallableScope extends Range_ instanceof AbstractFunctionDecl {
-    Decls::FuncDeclTree tree;
+    final override predicate entry(ControlFlowElement first) {
+      exists(Decls::FuncDeclTree tree |
+        tree.getAst() = this and
+        first = tree
+      )
+    }

-    BodyStmtCallableScope() { tree.getAst() = this }
-
-    final override predicate entry(ControlFlowElement first) { first(tree, first) }
-
-    final override predicate exit(ControlFlowElement last, Completion c) { last(tree, last, c) }
-  }
-
-  private class KeyPathScope extends Range_ instanceof KeyPathExpr {
-    AstControlFlowTree tree;
-
-    KeyPathScope() { tree.getAst() = this.getParsedRoot().getFullyConverted() }
-
-    final override predicate entry(ControlFlowElement first) { first(tree, first) }
-
-    final override predicate exit(ControlFlowElement last, Completion c) { last(tree, last, c) }
+    final override predicate exit(ControlFlowElement last, Completion c) {
+      exists(Decls::FuncDeclTree tree |
+        tree.getAst() = this and
+        tree.last(last, c)
+      )
+    }
  }
 }

@@ -1116,20 +1112,6 @@ module Exprs {
    }
  }

-  class KeyPathTree extends AstLeafTree {
-    override KeyPathExpr ast;
-  }
-
-  class KeyPathApplicationTree extends AstStandardPostOrderTree {
-    override KeyPathApplicationExpr ast;
-
-    final override ControlFlowElement getChildElement(int i) {
-      i = 0 and result.asAstNode() = ast.getBase().getFullyConverted()
-      or
-      i = 1 and result.asAstNode() = ast.getKeyPath().getFullyConverted()
-    }
-  }
-
  private class InOutTree extends AstStandardPostOrderTree {
    override InOutExpr ast;

@@ -1666,9 +1648,7 @@ private module Cached {
    result = scopeOfAst(n.asAstNode()) or
    result = scopeOfAst(n.(PropertyGetterElement).getRef()) or
    result = scopeOfAst(n.(PropertySetterElement).getAssignExpr()) or
-    result = scopeOfAst(n.(PropertyObserverElement).getAssignExpr()) or
-    result = n.(FuncDeclElement).getAst() or
-    result = n.(KeyPathElement).getAst()
+    result = scopeOfAst(n.(PropertyObserverElement).getAssignExpr())
  }

  cached
--- a/swift/ql/lib/codeql/swift/controlflow/internal/Scope.qll
+++ b/swift/ql/lib/codeql/swift/controlflow/internal/Scope.qll
@@ -2,7 +2,7 @@ private import swift
 private import codeql.swift.generated.GetImmediateParent

 module CallableBase {
-  class TypeRange = @abstract_function_decl or @key_path_expr;
+  class TypeRange = @abstract_function_decl;

  class Range extends Scope::Range, TypeRange { }
 }
--- a/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowDispatch.qll
+++ b/swift/ql/lib/codeql/swift/dataflow/internal/DataFlowDispatch.qll
@@ -1,7 +1,6 @@
 private import swift
 private import DataFlowPrivate
 private import DataFlowPublic
-private import codeql.swift.controlflow.ControlFlowGraph

 newtype TReturnKind = TNormalReturnKind()

@@ -54,7 +53,7 @@ class DataFlowCall extends ExprNode {
 cached
 private module Cached {
  cached
-  newtype TDataFlowCallable = TDataFlowFunc(CfgScope scope)
+  newtype TDataFlowCallable = TDataFlowFunc(FuncDecl func)

  /** Gets a viable run-time target for the call `call`. */
  cached
--- a/swift/ql/test/library-tests/controlflow/graph/Cfg.expected
+++ b/swift/ql/test/library-tests/controlflow/graph/Cfg.expected
@@ -5157,360 +5157,3 @@ cfg.swift:

 #  405| y
 #-----|  -> (...)
-
-#  409| (unnamed function decl)
-#-----|  -> yield ...
-#-----|  -> TBD (YieldStmt)
-
-#  409| enter (unnamed function decl)
-#-----|  -> (unnamed function decl)
-
-#  409| enter get
-#-----|  -> get
-
-#  409| enter set
-#-----|  -> set
-
-#  409| exit (unnamed function decl)
-
-#  409| exit (unnamed function decl) (normal)
-#-----|  -> exit (unnamed function decl)
-
-#  409| exit get
-
-#  409| exit get (normal)
-#-----|  -> exit get
-
-#  409| exit set
-
-#  409| exit set (normal)
-#-----|  -> exit set
-
-#  409| get
-
-#  409| set
-#-----|  -> value
-
-#  409| value
-
-#  409| yield ...
-#-----|  -> exit (unnamed function decl) (normal)
-
-#  409| TBD (YieldStmt)
-#-----|  -> exit (unnamed function decl) (normal)
-
-#  413| (unnamed function decl)
-#-----|  -> yield ...
-#-----|  -> TBD (YieldStmt)
-
-#  413| enter (unnamed function decl)
-#-----|  -> (unnamed function decl)
-
-#  413| enter get
-#-----|  -> get
-
-#  413| enter set
-#-----|  -> set
-
-#  413| exit (unnamed function decl)
-
-#  413| exit (unnamed function decl) (normal)
-#-----|  -> exit (unnamed function decl)
-
-#  413| exit get
-
-#  413| exit get (normal)
-#-----|  -> exit get
-
-#  413| exit set
-
-#  413| exit set (normal)
-#-----|  -> exit set
-
-#  413| get
-
-#  413| set
-#-----|  -> value
-
-#  413| value
-
-#  413| yield ...
-#-----|  -> exit (unnamed function decl) (normal)
-
-#  413| TBD (YieldStmt)
-#-----|  -> exit (unnamed function decl) (normal)
-
-#  414| (unnamed function decl)
-#-----|  -> yield ...
-#-----|  -> TBD (YieldStmt)
-
-#  414| enter (unnamed function decl)
-#-----|  -> (unnamed function decl)
-
-#  414| enter get
-#-----|  -> get
-
-#  414| enter set
-#-----|  -> set
-
-#  414| exit (unnamed function decl)
-
-#  414| exit (unnamed function decl) (normal)
-#-----|  -> exit (unnamed function decl)
-
-#  414| exit get
-
-#  414| exit get (normal)
-#-----|  -> exit get
-
-#  414| exit set
-
-#  414| exit set (normal)
-#-----|  -> exit set
-
-#  414| get
-
-#  414| set
-#-----|  -> value
-
-#  414| value
-
-#  414| yield ...
-#-----|  -> exit (unnamed function decl) (normal)
-
-#  414| TBD (YieldStmt)
-#-----|  -> exit (unnamed function decl) (normal)
-
-#  415| (unnamed function decl)
-#-----|  -> yield ...
-#-----|  -> TBD (YieldStmt)
-
-#  415| enter (unnamed function decl)
-#-----|  -> (unnamed function decl)
-
-#  415| enter get
-#-----|  -> get
-
-#  415| enter set
-#-----|  -> set
-
-#  415| exit (unnamed function decl)
-
-#  415| exit (unnamed function decl) (normal)
-#-----|  -> exit (unnamed function decl)
-
-#  415| exit get
-
-#  415| exit get (normal)
-#-----|  -> exit get
-
-#  415| exit set
-
-#  415| exit set (normal)
-#-----|  -> exit set
-
-#  415| get
-
-#  415| set
-#-----|  -> value
-
-#  415| value
-
-#  415| yield ...
-#-----|  -> exit (unnamed function decl) (normal)
-
-#  415| TBD (YieldStmt)
-#-----|  -> exit (unnamed function decl) (normal)
-
-#  418| enter test
-#-----|  -> test
-
-#  418| exit test
-
-#  418| exit test (normal)
-#-----|  -> exit test
-
-#  418| test
-#-----|  -> a
-
-#  418| a
-#-----|  -> kpGet_b_x
-
-#  419| var ... = ...
-#-----|  -> kpGet_b_x
-
-#  419| kpGet_b_x
-#-----|  -> #keyPath(...)
-
-#  419| kpGet_b_x
-#-----|  -> kpGet_bs_0_x
-
-#  419| #keyPath(...)
-#-----|  -> var ... = ...
-
-#  419| enter #keyPath(...)
-#-----|  -> TBD (UnresolvedDotExpr)
-
-#  419| exit #keyPath(...)
-
-#  419| exit #keyPath(...) (normal)
-#-----|  -> exit #keyPath(...)
-
-#  419| TBD (UnresolvedDotExpr)
-#-----|  -> exit #keyPath(...) (normal)
-
-#  420| var ... = ...
-#-----|  -> kpGet_bs_0_x
-
-#  420| kpGet_bs_0_x
-#-----|  -> #keyPath(...)
-
-#  420| kpGet_bs_0_x
-#-----|  -> kpGet_mayB_force_x
-
-#  420| #keyPath(...)
-#-----|  -> var ... = ...
-
-#  420| enter #keyPath(...)
-#-----|  -> TBD (UnresolvedDotExpr)
-
-#  420| exit #keyPath(...)
-
-#  420| exit #keyPath(...) (normal)
-#-----|  -> exit #keyPath(...)
-
-#  420| TBD (UnresolvedDotExpr)
-#-----|  -> exit #keyPath(...) (normal)
-
-#  421| var ... = ...
-#-----|  -> kpGet_mayB_force_x
-
-#  421| kpGet_mayB_force_x
-#-----|  -> #keyPath(...)
-
-#  421| kpGet_mayB_force_x
-#-----|  -> kpGet_mayB_x
-
-#  421| #keyPath(...)
-#-----|  -> var ... = ...
-
-#  421| enter #keyPath(...)
-#-----|  -> TBD (UnresolvedDotExpr)
-
-#  421| exit #keyPath(...)
-
-#  421| exit #keyPath(...) (normal)
-#-----|  -> exit #keyPath(...)
-
-#  421| TBD (UnresolvedDotExpr)
-#-----|  -> exit #keyPath(...) (normal)
-
-#  422| var ... = ...
-#-----|  -> kpGet_mayB_x
-
-#  422| kpGet_mayB_x
-#-----|  -> #keyPath(...)
-
-#  422| kpGet_mayB_x
-#-----|  -> apply_kpGet_b_x
-
-#  422| #keyPath(...)
-#-----|  -> var ... = ...
-
-#  422| enter #keyPath(...)
-#-----|  -> TBD (UnresolvedDotExpr)
-
-#  422| exit #keyPath(...)
-
-#  422| exit #keyPath(...) (normal)
-#-----|  -> exit #keyPath(...)
-
-#  422| OptionalEvaluationExpr
-#-----|  -> exit #keyPath(...) (normal)
-
-#  422| TBD (UnresolvedDotExpr)
-#-----|  -> OptionalEvaluationExpr
-
-#  424| var ... = ...
-#-----|  -> apply_kpGet_b_x
-
-#  424| apply_kpGet_b_x
-#-----|  -> a
-
-#  424| apply_kpGet_b_x
-#-----|  -> apply_kpGet_bs_0_x
-
-#  424| a
-#-----|  -> kpGet_b_x
-
-#  424| \...[...]
-#-----|  -> var ... = ...
-
-#  424| (WritableKeyPath<A, Int>) ...
-#-----|  -> \...[...]
-
-#  424| kpGet_b_x
-#-----|  -> (WritableKeyPath<A, Int>) ...
-
-#  425| var ... = ...
-#-----|  -> apply_kpGet_bs_0_x
-
-#  425| apply_kpGet_bs_0_x
-#-----|  -> a
-
-#  425| apply_kpGet_bs_0_x
-#-----|  -> apply_kpGet_mayB_force_x
-
-#  425| a
-#-----|  -> kpGet_bs_0_x
-
-#  425| \...[...]
-#-----|  -> var ... = ...
-
-#  425| (WritableKeyPath<A, Int>) ...
-#-----|  -> \...[...]
-
-#  425| kpGet_bs_0_x
-#-----|  -> (WritableKeyPath<A, Int>) ...
-
-#  426| var ... = ...
-#-----|  -> apply_kpGet_mayB_force_x
-
-#  426| apply_kpGet_mayB_force_x
-#-----|  -> a
-
-#  426| apply_kpGet_mayB_force_x
-#-----|  -> apply_kpGet_mayB_x
-
-#  426| a
-#-----|  -> kpGet_mayB_force_x
-
-#  426| \...[...]
-#-----|  -> var ... = ...
-
-#  426| (WritableKeyPath<A, Int>) ...
-#-----|  -> \...[...]
-
-#  426| kpGet_mayB_force_x
-#-----|  -> (WritableKeyPath<A, Int>) ...
-
-#  427| var ... = ...
-#-----|  -> apply_kpGet_mayB_x
-
-#  427| apply_kpGet_mayB_x
-#-----|  -> a
-
-#  427| apply_kpGet_mayB_x
-#-----|  -> exit test (normal)
-
-#  427| a
-#-----|  -> kpGet_mayB_x
-
-#  427| \...[...]
-#-----|  -> var ... = ...
-
-#  427| (KeyPath<A, Int?>) ...
-#-----|  -> \...[...]
-
-#  427| kpGet_mayB_x
-#-----|  -> (KeyPath<A, Int?>) ...
--- a/swift/ql/test/library-tests/controlflow/graph/cfg.swift
+++ b/swift/ql/test/library-tests/controlflow/graph/cfg.swift
@@ -403,26 +403,4 @@ class Structors {

 func dictionaryLiteral(x: Int, y: Int) -> [String: Int] {
  return ["x": x, "y": y]
-}
-
-struct B {
-  var x : Int
-}
-
-struct A {
-  var b : B
-  var bs : [B]
-  var mayB : B?
-}
-
-func test(a : A) {
-  var kpGet_b_x = \A.b.x
-  var kpGet_bs_0_x = \A.bs[0].x
-  var kpGet_mayB_force_x = \A.mayB!.x
-  var kpGet_mayB_x = \A.mayB?.x
-
-  var apply_kpGet_b_x = a[keyPath: kpGet_b_x]
-  var apply_kpGet_bs_0_x = a[keyPath: kpGet_bs_0_x]
-  var apply_kpGet_mayB_force_x = a[keyPath: kpGet_mayB_force_x]
-  var apply_kpGet_mayB_x = a[keyPath: kpGet_mayB_x]
 }