From dd01da493873144e20b28e3050401020b0a1a649 Mon Sep 17 00:00:00 2001
From: Robert Marsh <rdmarsh2@github.com>
Date: Thu, 14 Sep 2023 13:26:41 +0000
Subject: [PATCH 1/8] Swift: AST and CFG tests for nil coalescing

---
 .../test/library-tests/ast/PrintAst.expected  | 15 +++++++++
 swift/ql/test/library-tests/ast/cfg.swift     |  4 +++
 .../controlflow/graph/Cfg.expected            | 33 +++++++++++++++++++
 .../library-tests/controlflow/graph/cfg.swift |  4 +++
 4 files changed, 56 insertions(+)

diff --git a/swift/ql/test/library-tests/ast/PrintAst.expected b/swift/ql/test/library-tests/ast/PrintAst.expected
index c6dd456422d0..ac270c25b8ee 100644
--- a/swift/ql/test/library-tests/ast/PrintAst.expected
+++ b/swift/ql/test/library-tests/ast/PrintAst.expected
@@ -3300,6 +3300,21 @@ cfg.swift:
 #  526|     Type = Int
 #  533| [ConcreteVarDecl] i
 #  533|     Type = Int
+#  538| [NamedFunction] testNilCoalescing(x:)
+#  538|     InterfaceType = (Int?) -> Int
+#  538|   getParam(0): [ParamDecl] x
+#  538|       Type = Int?
+#  538|   getBody(): [BraceStmt] { ... }
+#  539|     getElement(0): [ReturnStmt] return ...
+#  539|       getResult(): [BinaryExpr] ... ??(_:_:) ...
+#  539|         getFunction(): [DeclRefExpr] ??(_:_:)
+#  539|         getArgument(0): [Argument] : x
+#  539|           getExpr(): [DeclRefExpr] x
+#  539|         getArgument(1): [Argument] : { ... }
+#  539|           getExpr(): [AutoClosureExpr] { ... }
+#  539|             getBody(): [BraceStmt] { ... }
+#  539|               getElement(0): [ReturnStmt] return ...
+#  539|                 getResult(): [IntegerLiteralExpr] 0
 declarations.swift:
 #    1| [StructDecl] Foo
 #    2|   getMember(0): [PatternBindingDecl] var ... = ...
diff --git a/swift/ql/test/library-tests/ast/cfg.swift b/swift/ql/test/library-tests/ast/cfg.swift
index 4d9169943a32..6d878bff48b4 100644
--- a/swift/ql/test/library-tests/ast/cfg.swift
+++ b/swift/ql/test/library-tests/ast/cfg.swift
@@ -534,3 +534,7 @@ func testAsyncFor () async {
         print(i)
     }
 }
+
+func testNilCoalescing(x: Int?) -> Int {
+  return x ?? 0
+}
diff --git a/swift/ql/test/library-tests/controlflow/graph/Cfg.expected b/swift/ql/test/library-tests/controlflow/graph/Cfg.expected
index d3f07e39c15f..5877c3af2d85 100644
--- a/swift/ql/test/library-tests/controlflow/graph/Cfg.expected
+++ b/swift/ql/test/library-tests/controlflow/graph/Cfg.expected
@@ -6180,3 +6180,36 @@ cfg.swift:
 
 #  534| i
 #-----|  -> (Any) ...
+
+#  538| enter testNilCoalescing(x:)
+#-----|  -> testNilCoalescing(x:)
+
+#  538| exit testNilCoalescing(x:)
+
+#  538| exit testNilCoalescing(x:) (abnormal)
+#-----|  -> exit testNilCoalescing(x:)
+
+#  538| exit testNilCoalescing(x:) (normal)
+#-----|  -> exit testNilCoalescing(x:)
+
+#  538| testNilCoalescing(x:)
+#-----|  -> x
+
+#  538| x
+#-----|  -> ??(_:_:)
+
+#  539| return ...
+#-----| return -> exit testNilCoalescing(x:) (normal)
+
+#  539| x
+#-----|  -> { ... }
+
+#  539| ... ??(_:_:) ...
+#-----| exception -> exit testNilCoalescing(x:) (abnormal)
+#-----|  -> return ...
+
+#  539| ??(_:_:)
+#-----|  -> x
+
+#  539| { ... }
+#-----|  -> ... ??(_:_:) ...
diff --git a/swift/ql/test/library-tests/controlflow/graph/cfg.swift b/swift/ql/test/library-tests/controlflow/graph/cfg.swift
index 4d9169943a32..6d878bff48b4 100644
--- a/swift/ql/test/library-tests/controlflow/graph/cfg.swift
+++ b/swift/ql/test/library-tests/controlflow/graph/cfg.swift
@@ -534,3 +534,7 @@ func testAsyncFor () async {
         print(i)
     }
 }
+
+func testNilCoalescing(x: Int?) -> Int {
+  return x ?? 0
+}

From 9a5fa42dbe0a2a581972a18c186f4ac48ee7ffd2 Mon Sep 17 00:00:00 2001
From: Robert Marsh <rdmarsh2@github.com>
Date: Thu, 14 Sep 2023 16:16:30 +0000
Subject: [PATCH 2/8] Swift: CFG for nil coalescing operator

---
 .../internal/ControlFlowGraphImpl.qll         | 45 ++++++++++++++++---
 swift/ql/lib/codeql/swift/elements.qll        |  1 +
 .../swift/elements/expr/NilCoalescingExpr.qll |  8 ++++
 3 files changed, 49 insertions(+), 5 deletions(-)
 create mode 100644 swift/ql/lib/codeql/swift/elements/expr/NilCoalescingExpr.qll

diff --git a/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll b/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll
index 570e18822615..af3aaa4ea951 100644
--- a/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll
@@ -1179,7 +1179,7 @@ module Exprs {
     }
 
     /**
-     * An autoclosure expression that is generated as part of a logical operation.
+     * An autoclosure expression that is generated as part of a logical operation or nil coalescing expression.
      *
      * This is needed because the Swift AST for `b1 && b2` is really syntactic sugar a function call:
      * ```swift
@@ -1188,10 +1188,13 @@ module Exprs {
      * So the `true` edge from `b1` cannot just go to `b2` since this is an implicit autoclosure.
      * To handle this dig into the autoclosure when it's an operand of a logical operator.
      */
-    private class LogicalAutoClosureTree extends AstPreOrderTree {
+    private class ShortCircuitingAutoClosureTree extends AstPreOrderTree {
       override AutoClosureExpr ast;
 
-      LogicalAutoClosureTree() { ast = any(LogicalOperation op).getAnOperand() }
+      ShortCircuitingAutoClosureTree() {
+        ast = any(LogicalOperation op).getAnOperand() or
+        ast = any(NilCoalescingExpr expr).getAnOperand()
+      }
 
       override predicate last(ControlFlowElement last, Completion c) {
         exists(Completion completion | astLast(ast.getReturn(), last, completion) |
@@ -1217,7 +1220,7 @@ module Exprs {
     private class AutoClosureTree extends AstLeafTree {
       override AutoClosureExpr ast;
 
-      AutoClosureTree() { not this instanceof LogicalAutoClosureTree }
+      AutoClosureTree() { not this instanceof ShortCircuitingAutoClosureTree }
     }
   }
 
@@ -1557,7 +1560,9 @@ module Exprs {
       // This one is handled in `LogicalNotTree`.
       not ast instanceof UnaryLogicalOperation and
       // These are handled in `LogicalOrTree` and `LogicalAndTree`.
-      not ast instanceof BinaryLogicalOperation
+      not ast instanceof BinaryLogicalOperation and
+      // This one is handled in `NilCoalescingTree`
+      not ast instanceof NilCoalescingExpr
     }
 
     final override ControlFlowElement getChildElement(int i) {
@@ -1581,6 +1586,36 @@ module Exprs {
     }
   }
 
+  private class NilCoalescingTree extends AstControlFlowTree {
+    override NilCoalescingExpr ast;
+
+    final override predicate propagatesAbnormal(ControlFlowElement child) {
+      child.asAstNode() = ast.getAnOperand().getFullyConverted()
+    }
+
+    final override predicate first(ControlFlowElement first) {
+      astFirst(ast.getLeftOperand().getFullyConverted(), first)
+    }
+
+    final override predicate last(ControlFlowElement last, Completion c) {
+      last.asAstNode() = ast and
+      exists(EmptinessCompletion ec | ec = c | not ec.isEmpty())
+      or
+      astLast(ast.getRightOperand().getFullyConverted(), last, c) and
+      c instanceof NormalCompletion
+    }
+
+    final override predicate succ(ControlFlowElement pred, ControlFlowElement succ, Completion c) {
+      astLast(ast.getLeftOperand().getFullyConverted(), pred, c) and
+      c instanceof NormalCompletion and
+      succ.asAstNode() = ast
+      or
+      pred.asAstNode() = ast and
+      c.(EmptinessCompletion).isEmpty() and
+      astFirst(ast.getRightOperand().getFullyConverted(), succ)
+    }
+  }
+
   private class LogicalAndTree extends AstPostOrderTree {
     override LogicalAndExpr ast;
 
diff --git a/swift/ql/lib/codeql/swift/elements.qll b/swift/ql/lib/codeql/swift/elements.qll
index 7c75c11c9762..b4f9ac5b2b8f 100644
--- a/swift/ql/lib/codeql/swift/elements.qll
+++ b/swift/ql/lib/codeql/swift/elements.qll
@@ -140,6 +140,7 @@ import codeql.swift.elements.expr.MakeTemporarilyEscapableExpr
 import codeql.swift.elements.expr.MemberRefExpr
 import codeql.swift.elements.expr.MetatypeConversionExpr
 import codeql.swift.elements.expr.MethodLookupExpr
+import codeql.swift.elements.expr.NilCoalescingExpr
 import codeql.swift.elements.expr.NilLiteralExpr
 import codeql.swift.elements.expr.NumberLiteralExpr
 import codeql.swift.elements.expr.ObjCSelectorExpr
diff --git a/swift/ql/lib/codeql/swift/elements/expr/NilCoalescingExpr.qll b/swift/ql/lib/codeql/swift/elements/expr/NilCoalescingExpr.qll
new file mode 100644
index 000000000000..fd0abc145200
--- /dev/null
+++ b/swift/ql/lib/codeql/swift/elements/expr/NilCoalescingExpr.qll
@@ -0,0 +1,8 @@
+private import codeql.swift.elements.expr.Expr
+private import codeql.swift.elements.expr.BinaryExpr
+
+class NilCoalescingExpr extends BinaryExpr {
+    NilCoalescingExpr() {
+        this.getStaticTarget().getName() = "??(_:_:)"
+    }
+}
\ No newline at end of file

From ab7cd5254a0c75e1db8755fb5f92673e227cfae2 Mon Sep 17 00:00:00 2001
From: Robert Marsh <rdmarsh2@github.com>
Date: Thu, 14 Sep 2023 16:28:00 +0000
Subject: [PATCH 3/8] Swift: update dataflow test for nil coalescing

---
 .../test/library-tests/dataflow/dataflow/DataFlow.expected | 7 +++++++
 .../library-tests/dataflow/dataflow/LocalFlow.expected     | 4 ++++
 swift/ql/test/library-tests/dataflow/dataflow/test.swift   | 4 ++--
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected b/swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected
index d25f5ee08496..c40139560f12 100644
--- a/swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected
+++ b/swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected
@@ -139,6 +139,8 @@ edges
 | test.swift:270:15:270:22 | call to source() | test.swift:270:15:270:31 | call to signum() |
 | test.swift:271:15:271:16 | ...? | test.swift:271:15:271:25 | call to signum() |
 | test.swift:271:15:271:25 | call to signum() | test.swift:271:15:271:25 | OptionalEvaluationExpr |
+| test.swift:275:20:275:27 | call to source() | test.swift:275:15:275:27 | ... ??(_:_:) ... |
+| test.swift:277:20:277:27 | call to source() | test.swift:277:15:277:27 | ... ??(_:_:) ... |
 | test.swift:279:26:279:26 | x [some:0] | test.swift:279:26:279:27 | ...! |
 | test.swift:279:26:279:27 | ...! | test.swift:279:15:279:31 | ... ? ... : ... |
 | test.swift:280:26:280:26 | x [some:0] | test.swift:280:26:280:27 | ...! |
@@ -673,6 +675,9 @@ nodes
 | test.swift:271:15:271:25 | call to signum() | semmle.label | call to signum() |
 | test.swift:274:15:274:20 | ... ??(_:_:) ... | semmle.label | ... ??(_:_:) ... |
 | test.swift:275:15:275:27 | ... ??(_:_:) ... | semmle.label | ... ??(_:_:) ... |
+| test.swift:275:20:275:27 | call to source() | semmle.label | call to source() |
+| test.swift:277:15:277:27 | ... ??(_:_:) ... | semmle.label | ... ??(_:_:) ... |
+| test.swift:277:20:277:27 | call to source() | semmle.label | call to source() |
 | test.swift:279:15:279:31 | ... ? ... : ... | semmle.label | ... ? ... : ... |
 | test.swift:279:26:279:26 | x [some:0] | semmle.label | x [some:0] |
 | test.swift:279:26:279:27 | ...! | semmle.label | ...! |
@@ -1187,6 +1192,8 @@ subpaths
 | test.swift:271:15:271:25 | OptionalEvaluationExpr | test.swift:259:12:259:19 | call to source() | test.swift:271:15:271:25 | OptionalEvaluationExpr | result |
 | test.swift:274:15:274:20 | ... ??(_:_:) ... | test.swift:259:12:259:19 | call to source() | test.swift:274:15:274:20 | ... ??(_:_:) ... | result |
 | test.swift:275:15:275:27 | ... ??(_:_:) ... | test.swift:259:12:259:19 | call to source() | test.swift:275:15:275:27 | ... ??(_:_:) ... | result |
+| test.swift:275:15:275:27 | ... ??(_:_:) ... | test.swift:275:20:275:27 | call to source() | test.swift:275:15:275:27 | ... ??(_:_:) ... | result |
+| test.swift:277:15:277:27 | ... ??(_:_:) ... | test.swift:277:20:277:27 | call to source() | test.swift:277:15:277:27 | ... ??(_:_:) ... | result |
 | test.swift:279:15:279:31 | ... ? ... : ... | test.swift:259:12:259:19 | call to source() | test.swift:279:15:279:31 | ... ? ... : ... | result |
 | test.swift:280:15:280:38 | ... ? ... : ... | test.swift:259:12:259:19 | call to source() | test.swift:280:15:280:38 | ... ? ... : ... | result |
 | test.swift:280:15:280:38 | ... ? ... : ... | test.swift:280:31:280:38 | call to source() | test.swift:280:15:280:38 | ... ? ... : ... | result |
diff --git a/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected b/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected
index 0233e777fcd8..2756be187f89 100644
--- a/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected
+++ b/swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected
@@ -247,15 +247,19 @@
 | test.swift:272:15:272:25 | call to signum() | test.swift:272:15:272:25 | OptionalEvaluationExpr |
 | test.swift:274:15:274:15 | x | test.swift:274:15:274:20 | ... ??(_:_:) ... |
 | test.swift:274:15:274:15 | x | test.swift:275:15:275:15 | x |
+| test.swift:274:20:274:20 | 0 | test.swift:274:15:274:20 | ... ??(_:_:) ... |
 | test.swift:274:20:274:20 | { ... } | test.swift:274:15:274:20 | ... ??(_:_:) ... |
 | test.swift:275:15:275:15 | x | test.swift:275:15:275:27 | ... ??(_:_:) ... |
 | test.swift:275:15:275:15 | x | test.swift:279:15:279:15 | x |
+| test.swift:275:20:275:27 | call to source() | test.swift:275:15:275:27 | ... ??(_:_:) ... |
 | test.swift:275:20:275:27 | { ... } | test.swift:275:15:275:27 | ... ??(_:_:) ... |
 | test.swift:276:15:276:15 | y | test.swift:276:15:276:20 | ... ??(_:_:) ... |
 | test.swift:276:15:276:15 | y | test.swift:277:15:277:15 | y |
+| test.swift:276:20:276:20 | 0 | test.swift:276:15:276:20 | ... ??(_:_:) ... |
 | test.swift:276:20:276:20 | { ... } | test.swift:276:15:276:20 | ... ??(_:_:) ... |
 | test.swift:277:15:277:15 | y | test.swift:277:15:277:27 | ... ??(_:_:) ... |
 | test.swift:277:15:277:15 | y | test.swift:281:15:281:15 | y |
+| test.swift:277:20:277:27 | call to source() | test.swift:277:15:277:27 | ... ??(_:_:) ... |
 | test.swift:277:20:277:27 | { ... } | test.swift:277:15:277:27 | ... ??(_:_:) ... |
 | test.swift:279:15:279:15 | x | test.swift:279:26:279:26 | x |
 | test.swift:279:15:279:15 | x | test.swift:280:15:280:15 | x |
diff --git a/swift/ql/test/library-tests/dataflow/dataflow/test.swift b/swift/ql/test/library-tests/dataflow/dataflow/test.swift
index b4f7a5dadec9..b2d8756cdacc 100644
--- a/swift/ql/test/library-tests/dataflow/dataflow/test.swift
+++ b/swift/ql/test/library-tests/dataflow/dataflow/test.swift
@@ -272,9 +272,9 @@ func test_optionals(y: Int?) {
     sink(opt: y?.signum())
 
     sink(arg: x ?? 0) // $ flow=259
-    sink(arg: x ?? source()) // $ flow=259 MISSING: flow=276
+    sink(arg: x ?? source()) // $ flow=259 flow=275
     sink(arg: y ?? 0)
-    sink(arg: y ?? source()) // $ MISSING: flow=278
+    sink(arg: y ?? source()) // $ flow=277
 
     sink(arg: x != nil ? x! : 0) // $ flow=259
     sink(arg: x != nil ? x! : source()) // $ flow=259 flow=280

From 2b54ad58b0d95d8987852f5097b3eb0dcd0efe6e Mon Sep 17 00:00:00 2001
From: Robert Marsh <rdmarsh2@github.com>
Date: Fri, 15 Sep 2023 20:44:05 +0000
Subject: [PATCH 4/8] Swift: change note for nil-coalesing operator

---
 swift/ql/lib/change-notes/2023-09-15-nil-coalescing.md | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 swift/ql/lib/change-notes/2023-09-15-nil-coalescing.md

diff --git a/swift/ql/lib/change-notes/2023-09-15-nil-coalescing.md b/swift/ql/lib/change-notes/2023-09-15-nil-coalescing.md
new file mode 100644
index 000000000000..96f315c3a467
--- /dev/null
+++ b/swift/ql/lib/change-notes/2023-09-15-nil-coalescing.md
@@ -0,0 +1,5 @@
+---
+category: minorAnalysis
+---
+
+* The nil-coalescing operator `??` is now supported by the CFG construction and dataflow libraries.

From ca722dc74ccd5569175209b402691196841f78a4 Mon Sep 17 00:00:00 2001
From: Robert Marsh <rdmarsh2@github.com>
Date: Mon, 2 Oct 2023 18:07:11 +0000
Subject: [PATCH 5/8] Swift: add NilCoalescingTest node to CFG

Fixes an issue where a nil-coalescing operation used in a boolean
context would result in no control flow out of the default operand of
the nil-coalescing operator.
---
 .../internal/ControlFlowElements.qll          | 15 +++-
 .../internal/ControlFlowGraphImpl.qll         | 26 +++----
 .../controlflow/graph/Cfg.expected            | 71 +++++++++++++++++--
 .../library-tests/controlflow/graph/cfg.swift |  8 +++
 4 files changed, 103 insertions(+), 17 deletions(-)

diff --git a/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowElements.qll b/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowElements.qll
index b98adf4dde82..d026f5112557 100644
--- a/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowElements.qll
+++ b/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowElements.qll
@@ -12,7 +12,8 @@ newtype TControlFlowElement =
   TPropertyObserverElement(Accessor observer, AssignExpr assign) {
     isPropertyObserverElement(observer, assign)
   } or
-  TKeyPathElement(KeyPathExpr expr)
+  TKeyPathElement(KeyPathExpr expr) or
+  TNilCoalescingTestElement(NilCoalescingExpr expr)
 
 predicate isLValue(Expr e) { any(AssignExpr assign).getDest() = e }
 
@@ -204,3 +205,15 @@ class ClosureElement extends ControlFlowElement, TClosureElement {
 
   override string toString() { result = expr.toString() }
 }
+
+class NilCoalescingElement extends ControlFlowElement, TNilCoalescingTestElement {
+  NilCoalescingExpr expr;
+
+  NilCoalescingElement() { this = TNilCoalescingTestElement(expr) }
+
+  override Location getLocation() { result = expr.getLocation() }
+
+  NilCoalescingExpr getAst() { result = expr }
+
+  override string toString() { result = "emptiness test for " + expr.toString() }
+}
diff --git a/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll b/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll
index af3aaa4ea951..89a8d57655bb 100644
--- a/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll
@@ -1586,7 +1586,9 @@ module Exprs {
     }
   }
 
-  private class NilCoalescingTree extends AstControlFlowTree {
+  private class NilCoalescingTestTree extends LeafTree, NilCoalescingElement { }
+
+  private class NilCoalescingTree extends AstPostOrderTree {
     override NilCoalescingExpr ast;
 
     final override predicate propagatesAbnormal(ControlFlowElement child) {
@@ -1597,22 +1599,22 @@ module Exprs {
       astFirst(ast.getLeftOperand().getFullyConverted(), first)
     }
 
-    final override predicate last(ControlFlowElement last, Completion c) {
-      last.asAstNode() = ast and
-      exists(EmptinessCompletion ec | ec = c | not ec.isEmpty())
-      or
-      astLast(ast.getRightOperand().getFullyConverted(), last, c) and
-      c instanceof NormalCompletion
-    }
-
     final override predicate succ(ControlFlowElement pred, ControlFlowElement succ, Completion c) {
       astLast(ast.getLeftOperand().getFullyConverted(), pred, c) and
       c instanceof NormalCompletion and
-      succ.asAstNode() = ast
+      succ.(NilCoalescingTestTree).getAst() = ast
       or
-      pred.asAstNode() = ast and
-      c.(EmptinessCompletion).isEmpty() and
+      pred.(NilCoalescingTestTree).getAst() = ast and
+      exists(EmptinessCompletion ec | c = ec | ec.isEmpty()) and
       astFirst(ast.getRightOperand().getFullyConverted(), succ)
+      or
+      pred.(NilCoalescingTestTree).getAst() = ast and
+      exists(EmptinessCompletion ec | c = ec | not ec.isEmpty()) and
+      succ.asAstNode() = ast
+      or
+      astLast(ast.getRightOperand().getFullyConverted(), pred, c) and
+      c instanceof NormalCompletion and
+      succ.asAstNode() = ast
     }
   }
 
diff --git a/swift/ql/test/library-tests/controlflow/graph/Cfg.expected b/swift/ql/test/library-tests/controlflow/graph/Cfg.expected
index 5877c3af2d85..26878fa9135b 100644
--- a/swift/ql/test/library-tests/controlflow/graph/Cfg.expected
+++ b/swift/ql/test/library-tests/controlflow/graph/Cfg.expected
@@ -6196,20 +6196,83 @@ cfg.swift:
 #-----|  -> x
 
 #  538| x
-#-----|  -> ??(_:_:)
+#-----|  -> x
 
 #  539| return ...
 #-----| return -> exit testNilCoalescing(x:) (normal)
 
 #  539| x
-#-----|  -> { ... }
+#-----|  -> emptiness test for ... ??(_:_:) ...
 
 #  539| ... ??(_:_:) ...
 #-----| exception -> exit testNilCoalescing(x:) (abnormal)
 #-----|  -> return ...
 
-#  539| ??(_:_:)
-#-----|  -> x
+#  539| emptiness test for ... ??(_:_:) ...
+#-----| non-empty -> ... ??(_:_:) ...
+#-----| empty -> { ... }
+
+#  539| 0
+#-----|  -> return ...
+
+#  539| return ...
+#-----|  -> ... ??(_:_:) ...
 
 #  539| { ... }
+#-----|  -> 0
+
+#  542| enter testNilCoalescing2(x:)
+#-----|  -> testNilCoalescing2(x:)
+
+#  542| exit testNilCoalescing2(x:)
+
+#  542| exit testNilCoalescing2(x:) (abnormal)
+#-----|  -> exit testNilCoalescing2(x:)
+
+#  542| exit testNilCoalescing2(x:) (normal)
+#-----|  -> exit testNilCoalescing2(x:)
+
+#  542| testNilCoalescing2(x:)
+#-----|  -> x
+
+#  542| x
+#-----|  -> if ... then { ... } else { ... }
+
+#  543| if ... then { ... } else { ... }
+#-----|  -> StmtCondition
+
+#  543| x
+#-----|  -> emptiness test for ... ??(_:_:) ...
+
+#  543| ... ??(_:_:) ...
+#-----| exception -> exit testNilCoalescing2(x:) (abnormal)
+#-----| true -> 1
+#-----| false -> 0
+
+#  543| StmtCondition
+#-----|  -> x
+
+#  543| emptiness test for ... ??(_:_:) ...
+#-----| non-empty -> ... ??(_:_:) ...
+#-----| empty -> { ... }
+
+#  543| false
+#-----|  -> return ...
+
+#  543| return ...
 #-----|  -> ... ??(_:_:) ...
+
+#  543| { ... }
+#-----|  -> false
+
+#  544| return ...
+#-----| return -> exit testNilCoalescing2(x:) (normal)
+
+#  544| 1
+#-----|  -> return ...
+
+#  546| return ...
+#-----| return -> exit testNilCoalescing2(x:) (normal)
+
+#  546| 0
+#-----|  -> return ...
diff --git a/swift/ql/test/library-tests/controlflow/graph/cfg.swift b/swift/ql/test/library-tests/controlflow/graph/cfg.swift
index 6d878bff48b4..9763c530082d 100644
--- a/swift/ql/test/library-tests/controlflow/graph/cfg.swift
+++ b/swift/ql/test/library-tests/controlflow/graph/cfg.swift
@@ -538,3 +538,11 @@ func testAsyncFor () async {
 func testNilCoalescing(x: Int?) -> Int {
   return x ?? 0
 }
+
+func testNilCoalescing2(x: Bool?) -> Int {
+  if x ?? false {
+    return 1
+  } else {
+    return 0
+  }
+}

From 497f0aa8ab572adf45f2b97e5130e4d342ce918c Mon Sep 17 00:00:00 2001
From: Robert Marsh <rdmarsh2@github.com>
Date: Tue, 3 Oct 2023 14:57:04 +0000
Subject: [PATCH 6/8] Swift: sync test files and update expectation

---
 .../test/library-tests/ast/PrintAst.expected  | 23 +++++++++++++++++++
 swift/ql/test/library-tests/ast/cfg.swift     |  8 +++++++
 2 files changed, 31 insertions(+)

diff --git a/swift/ql/test/library-tests/ast/PrintAst.expected b/swift/ql/test/library-tests/ast/PrintAst.expected
index ac270c25b8ee..2d1f36134bb7 100644
--- a/swift/ql/test/library-tests/ast/PrintAst.expected
+++ b/swift/ql/test/library-tests/ast/PrintAst.expected
@@ -3315,6 +3315,29 @@ cfg.swift:
 #  539|             getBody(): [BraceStmt] { ... }
 #  539|               getElement(0): [ReturnStmt] return ...
 #  539|                 getResult(): [IntegerLiteralExpr] 0
+#  542| [NamedFunction] testNilCoalescing2(x:)
+#  542|     InterfaceType = (Bool?) -> Int
+#  542|   getParam(0): [ParamDecl] x
+#  542|       Type = Bool?
+#  542|   getBody(): [BraceStmt] { ... }
+#  543|     getElement(0): [IfStmt] if ... then { ... } else { ... }
+#  543|       getCondition(): [StmtCondition] StmtCondition
+#  543|         getElement(0): [ConditionElement] ... ??(_:_:) ...
+#  543|           getBoolean(): [BinaryExpr] ... ??(_:_:) ...
+#  543|             getFunction(): [DeclRefExpr] ??(_:_:)
+#  543|             getArgument(0): [Argument] : x
+#  543|               getExpr(): [DeclRefExpr] x
+#  543|             getArgument(1): [Argument] : { ... }
+#  543|               getExpr(): [AutoClosureExpr] { ... }
+#  543|                 getBody(): [BraceStmt] { ... }
+#  543|                   getElement(0): [ReturnStmt] return ...
+#  543|                     getResult(): [BooleanLiteralExpr] false
+#  543|       getThen(): [BraceStmt] { ... }
+#  544|         getElement(0): [ReturnStmt] return ...
+#  544|           getResult(): [IntegerLiteralExpr] 1
+#  545|       getElse(): [BraceStmt] { ... }
+#  546|         getElement(0): [ReturnStmt] return ...
+#  546|           getResult(): [IntegerLiteralExpr] 0
 declarations.swift:
 #    1| [StructDecl] Foo
 #    2|   getMember(0): [PatternBindingDecl] var ... = ...
diff --git a/swift/ql/test/library-tests/ast/cfg.swift b/swift/ql/test/library-tests/ast/cfg.swift
index 6d878bff48b4..9763c530082d 100644
--- a/swift/ql/test/library-tests/ast/cfg.swift
+++ b/swift/ql/test/library-tests/ast/cfg.swift
@@ -538,3 +538,11 @@ func testAsyncFor () async {
 func testNilCoalescing(x: Int?) -> Int {
   return x ?? 0
 }
+
+func testNilCoalescing2(x: Bool?) -> Int {
+  if x ?? false {
+    return 1
+  } else {
+    return 0
+  }
+}

From cdef0796e361b81370f110be9fdd5cd9a298a489 Mon Sep 17 00:00:00 2001
From: Robert Marsh <rdmarsh2@github.com>
Date: Tue, 3 Oct 2023 15:00:03 +0000
Subject: [PATCH 7/8] Swift: QLDoc for NilCoalescingExpr.qll

---
 .../swift/elements/expr/NilCoalescingExpr.qll       | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/swift/ql/lib/codeql/swift/elements/expr/NilCoalescingExpr.qll b/swift/ql/lib/codeql/swift/elements/expr/NilCoalescingExpr.qll
index fd0abc145200..23fe5fc09c51 100644
--- a/swift/ql/lib/codeql/swift/elements/expr/NilCoalescingExpr.qll
+++ b/swift/ql/lib/codeql/swift/elements/expr/NilCoalescingExpr.qll
@@ -1,8 +1,13 @@
+/**
+ * Provides a class for the Swift nil-coalesing expr (`??`)
+ */
+
 private import codeql.swift.elements.expr.Expr
 private import codeql.swift.elements.expr.BinaryExpr
 
+/**
+ * A Swift nil-coalesing expr (`??`).
+ */
 class NilCoalescingExpr extends BinaryExpr {
-    NilCoalescingExpr() {
-        this.getStaticTarget().getName() = "??(_:_:)"
-    }
-}
\ No newline at end of file
+  NilCoalescingExpr() { this.getStaticTarget().getName() = "??(_:_:)" }
+}

From 06da5fd05c1918246680029b1296f5e42283a3c4 Mon Sep 17 00:00:00 2001
From: Robert Marsh <rdmarsh2@github.com>
Date: Tue, 3 Oct 2023 17:23:00 +0000
Subject: [PATCH 8/8] Swift: move import to make codegen happy

---
 swift/ql/lib/codeql/swift/elements.qll | 1 -
 swift/ql/lib/swift.qll                 | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/swift/ql/lib/codeql/swift/elements.qll b/swift/ql/lib/codeql/swift/elements.qll
index b4f9ac5b2b8f..7c75c11c9762 100644
--- a/swift/ql/lib/codeql/swift/elements.qll
+++ b/swift/ql/lib/codeql/swift/elements.qll
@@ -140,7 +140,6 @@ import codeql.swift.elements.expr.MakeTemporarilyEscapableExpr
 import codeql.swift.elements.expr.MemberRefExpr
 import codeql.swift.elements.expr.MetatypeConversionExpr
 import codeql.swift.elements.expr.MethodLookupExpr
-import codeql.swift.elements.expr.NilCoalescingExpr
 import codeql.swift.elements.expr.NilLiteralExpr
 import codeql.swift.elements.expr.NumberLiteralExpr
 import codeql.swift.elements.expr.ObjCSelectorExpr
diff --git a/swift/ql/lib/swift.qll b/swift/ql/lib/swift.qll
index 608795f8886b..2ea5d3774f3f 100644
--- a/swift/ql/lib/swift.qll
+++ b/swift/ql/lib/swift.qll
@@ -4,6 +4,7 @@ import codeql.swift.elements
 import codeql.swift.elements.expr.ArithmeticOperation
 import codeql.swift.elements.expr.BitwiseOperation
 import codeql.swift.elements.expr.LogicalOperation
+import codeql.swift.elements.expr.NilCoalescingExpr
 import codeql.swift.elements.expr.InitializerLookupExpr
 import codeql.swift.elements.expr.MethodCallExpr
 import codeql.swift.elements.expr.InitializerCallExpr