C++: Handle C++17 if initializers

Author: jketema

cpp/ql/lib/semmle/code/cpp/PrintAST.qll

@@ -663,12 +663,16 @@ private predicate namedStmtChildPredicates(Locatable s, Element e, string pred)
     or
     s.(ComputedGotoStmt).getExpr() = e and pred = "getExpr()"
     or
+    s.(ConstexprIfStmt).getInitialization() = e and pred = "getInitialization()"
+    or
     s.(ConstexprIfStmt).getCondition() = e and pred = "getCondition()"
     or
     s.(ConstexprIfStmt).getThen() = e and pred = "getThen()"
     or
     s.(ConstexprIfStmt).getElse() = e and pred = "getElse()"
     or
+    s.(IfStmt).getInitialization() = e and pred = "getInitialization()"
+    or
     s.(IfStmt).getCondition() = e and pred = "getCondition()"
     or
     s.(IfStmt).getThen() = e and pred = "getThen()"

cpp/ql/lib/semmle/code/cpp/controlflow/internal/CFG.qll

@@ -836,8 +836,15 @@ private predicate subEdge(Pos p1, Node n1, Node n2, Pos p2) {
     p2.nodeAt(n2, f)
   )
   or
-  // IfStmt -> condition ; { then, else } ->
+  // IfStmt -> [ init -> ] condition ; { then, else } ->
   exists(IfStmt s |
+    p1.nodeAt(n1, s) and
+    p2.nodeBefore(n2, s.getInitialization())
+    or
+    p1.nodeAfter(n1, s.getInitialization()) and
+    p2.nodeBefore(n2, s.getCondition())
+    or
+    not exists(s.getInitialization()) and
     p1.nodeAt(n1, s) and
     p2.nodeBefore(n2, s.getCondition())
     or
@@ -851,8 +858,15 @@ private predicate subEdge(Pos p1, Node n1, Node n2, Pos p2) {
     p2.nodeAfter(n2, s)
   )
   or
-  // ConstexprIfStmt -> condition ; { then, else } -> // same as IfStmt
+  // ConstexprIfStmt -> [ init -> ] condition ; { then, else } -> // same as IfStmt
   exists(ConstexprIfStmt s |
+    p1.nodeAt(n1, s) and
+    p2.nodeBefore(n2, s.getInitialization())
+    or
+    p1.nodeAfter(n1, s.getInitialization()) and
+    p2.nodeBefore(n2, s.getCondition())
+    or
+    not exists(s.getInitialization()) and
     p1.nodeAt(n1, s) and
     p2.nodeBefore(n2, s.getCondition())
     or

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedStmt.qll

@@ -421,20 +421,36 @@ class TranslatedCatchAnyHandler extends TranslatedHandler {
 class TranslatedIfStmt extends TranslatedStmt, ConditionContext {
   override IfStmt stmt;
 
-  override Instruction getFirstInstruction() { result = getCondition().getFirstInstruction() }
+  override Instruction getFirstInstruction() {
+    if hasInitialization()
+    then result = getInitialization().getFirstInstruction()
+    else result = getFirstConditionInstruction()
+  }
 
   override TranslatedElement getChild(int id) {
-    id = 0 and result = getCondition()
+    id = 0 and result = getInitialization()
+    or
+    id = 1 and result = getCondition()
     or
-    id = 1 and result = getThen()
+    id = 2 and result = getThen()
     or
-    id = 2 and result = getElse()
+    id = 3 and result = getElse()
+  }
+
+  private predicate hasInitialization() { exists(stmt.getInitialization()) }
+
+  private TranslatedStmt getInitialization() {
+    result = getTranslatedStmt(stmt.getInitialization())
   }
 
   private TranslatedCondition getCondition() {
     result = getTranslatedCondition(stmt.getCondition().getFullyConverted())
   }
 
+  private Instruction getFirstConditionInstruction() {
+    result = getCondition().getFirstInstruction()
+  }
+
   private TranslatedStmt getThen() { result = getTranslatedStmt(stmt.getThen()) }
 
   private TranslatedStmt getElse() { result = getTranslatedStmt(stmt.getElse()) }
@@ -456,6 +472,9 @@ class TranslatedIfStmt extends TranslatedStmt, ConditionContext {
   }
 
   override Instruction getChildSuccessor(TranslatedElement child) {
+    child = getInitialization() and
+    result = getFirstConditionInstruction()
+    or
     (child = getThen() or child = getElse()) and
     result = getParent().getChildSuccessor(this)
   }

cpp/ql/lib/semmle/code/cpp/stmts/Stmt.qll

@@ -213,6 +213,26 @@ class ConditionalStmt extends ControlStructure, TConditionalStmt { }
 class IfStmt extends ConditionalStmt, @stmt_if {
   override string getAPrimaryQlClass() { result = "IfStmt" }
 
+  /**
+   * Gets the initialization statement of this 'if' statement.
+   *
+   * For example, for
+   * ```
+   * if (x = y; b) { f(); }
+   * ```
+   * the result is `x = y;`.
+   *
+   * Does not hold if the initialization statement is missing or an empty statement, as in
+   * ```
+   * if (b) { f(); }
+   * ```
+   * or
+   * ```
+   * if (; b) { f(); }
+   * ```
+   */
+  Stmt getInitialization() { if_initialization(underlyingElement(this), unresolveElement(result)) }
+
   /**
    * Gets the condition expression of this 'if' statement.
    *
@@ -222,7 +242,7 @@ class IfStmt extends ConditionalStmt, @stmt_if {
    * ```
    * the result is `b`.
    */
-  Expr getCondition() { result = this.getChild(0) }
+  Expr getCondition() { result = this.getChild(1) }
 
   override Expr getControllingExpr() { result = this.getCondition() }
 
@@ -299,6 +319,28 @@ class IfStmt extends ConditionalStmt, @stmt_if {
 class ConstexprIfStmt extends ConditionalStmt, @stmt_constexpr_if {
   override string getAPrimaryQlClass() { result = "ConstexprIfStmt" }
 
+  /**
+   * Gets the initialization statement of this 'constexpr if' statement.
+   *
+   * For example, for
+   * ```
+   * if constexpr (x = y; b) { f(); }
+   * ```
+   * the result is `x = y;`.
+   *
+   * Does not hold if the initialization statement is missing or an empty statement, as in
+   * ```
+   * if constexpr (b) { f(); }
+   * ```
+   * or
+   * ```
+   * if constexpr (; b) { f(); }
+   * ```
+   */
+  Stmt getInitialization() {
+    constexpr_if_initialization(underlyingElement(this), unresolveElement(result))
+  }
+
   /**
    * Gets the condition expression of this 'constexpr if' statement.
    *
@@ -308,7 +350,7 @@ class ConstexprIfStmt extends ConditionalStmt, @stmt_constexpr_if {
    * ```
    * the result is `b`.
    */
-  Expr getCondition() { result = this.getChild(0) }
+  Expr getCondition() { result = this.getChild(1) }
 
   override Expr getControllingExpr() { result = this.getCondition() }
 
@@ -926,7 +968,7 @@ class ForStmt extends Loop, @stmt_for {
    *
    * Does not hold if the initialization statement is an empty statement, as in
    * ```
-   * for (; i < 10; i++) { j++ }
+   * for (; i < 10; i++) { j++; }
    * ```
    */
   Stmt getInitialization() { for_initialization(underlyingElement(this), unresolveElement(result)) }

cpp/ql/lib/semmlecode.cpp.dbscheme

@@ -1863,6 +1863,11 @@ variable_vla(
     int decl: @stmt_vla_decl ref
 );
 
+if_initialization(
+    unique int if_stmt: @stmt_if ref,
+    int init_id: @stmt ref
+);
+
 if_then(
     unique int if_stmt: @stmt_if ref,
     int then_id: @stmt ref
@@ -1873,6 +1878,11 @@ if_else(
     int else_id: @stmt ref
 );
 
+constexpr_if_initialization(
+    unique int constexpr_if_stmt: @stmt_constexpr_if ref,
+    int init_id: @stmt ref
+);
+
 constexpr_if_then(
     unique int constexpr_if_stmt: @stmt_constexpr_if ref,
     int then_id: @stmt ref

cpp/ql/src/Best Practices/Unused Entities/UnusedLocals.ql

@@ -57,6 +57,5 @@ where
   not declarationHasSideEffects(v) and
   not exists(AsmStmt s | f = s.getEnclosingFunction()) and
   not v.getAnAttribute().getName() = "unused" and
-  not any(ErrorExpr e).getEnclosingFunction() = f and // unextracted expr may use `v`
-  not any(ConditionDeclExpr cde).getEnclosingFunction() = f // this case can be removed when the `if (a = b; a)` and `switch (a = b; a)` test cases don't depend on this exclusion
+  not any(ErrorExpr e).getEnclosingFunction() = f // unextracted expr may use `v`
 select v, "Variable " + v.getName() + " is not used"