Java: Deprecate redundant basic block predicates.

Author: aschackmull

cpp/ql/lib/semmle/code/cpp/rangeanalysis/new/internal/semantic/SemanticCFG.qll

@@ -10,7 +10,7 @@ private import SemanticExprSpecific::SemanticExprConfig as Specific
  */
 class SemBasicBlock extends Specific::BasicBlock {
   /** Holds if this block (transitively) dominates `otherblock`. */
-  final predicate bbDominates(SemBasicBlock otherBlock) { Specific::bbDominates(this, otherBlock) }
+  final predicate dominates(SemBasicBlock otherBlock) { Specific::bbDominates(this, otherBlock) }
 
   /** Gets an expression that is evaluated in this basic block. */
   final SemExpr getAnExpr() { result.getBasicBlock() = this }

java/ql/lib/semmle/code/java/controlflow/BasicBlocks.qll

@@ -79,23 +79,47 @@ class BasicBlock extends BbImpl::BasicBlock {
   /** Gets the immediately enclosing callable whose body contains this node. */
   Callable getEnclosingCallable() { result = this.getScope() }
 
-  /** Gets an immediate successor of this basic block. */
-  BasicBlock getABBSuccessor() { result = this.getASuccessor() }
+  /**
+   * DEPRECATED: Use `getASuccessor` instead.
+   *
+   * Gets an immediate successor of this basic block.
+   */
+  deprecated BasicBlock getABBSuccessor() { result = this.getASuccessor() }
 
-  /** Gets an immediate predecessor of this basic block. */
-  BasicBlock getABBPredecessor() { result.getABBSuccessor() = this }
+  /**
+   * DEPRECATED: Use `getAPredecessor` instead.
+   *
+   * Gets an immediate predecessor of this basic block.
+   */
+  deprecated BasicBlock getABBPredecessor() { result.getASuccessor() = this }
 
-  /** Holds if this basic block strictly dominates `node`. */
-  predicate bbStrictlyDominates(BasicBlock node) { this.strictlyDominates(node) }
+  /**
+   * DEPRECATED: Use `strictlyDominates` instead.
+   *
+   * Holds if this basic block strictly dominates `node`.
+   */
+  deprecated predicate bbStrictlyDominates(BasicBlock node) { this.strictlyDominates(node) }
 
-  /** Holds if this basic block dominates `node`. (This is reflexive.) */
-  predicate bbDominates(BasicBlock node) { this.dominates(node) }
+  /**
+   * DEPRECATED: Use `dominates` instead.
+   *
+   * Holds if this basic block dominates `node`. (This is reflexive.)
+   */
+  deprecated predicate bbDominates(BasicBlock node) { this.dominates(node) }
 
-  /** Holds if this basic block strictly post-dominates `node`. */
-  predicate bbStrictlyPostDominates(BasicBlock node) { this.strictlyPostDominates(node) }
+  /**
+   * DEPRECATED: Use `strictlyPostDominates` instead.
+   *
+   * Holds if this basic block strictly post-dominates `node`.
+   */
+  deprecated predicate bbStrictlyPostDominates(BasicBlock node) { this.strictlyPostDominates(node) }
 
-  /** Holds if this basic block post-dominates `node`. (This is reflexive.) */
-  predicate bbPostDominates(BasicBlock node) { this.postDominates(node) }
+  /**
+   * DEPRECATED: Use `postDominates` instead.
+   *
+   * Holds if this basic block post-dominates `node`. (This is reflexive.)
+   */
+  deprecated predicate bbPostDominates(BasicBlock node) { this.postDominates(node) }
 }
 
 /** A basic block that ends in an exit node. */

java/ql/lib/semmle/code/java/controlflow/Dominance.qll

@@ -8,51 +8,72 @@ import java
  * Predicates for basic-block-level dominance.
  */
 
-/** The immediate dominance relation for basic blocks. */
-predicate bbIDominates(BasicBlock dom, BasicBlock node) { dom.immediatelyDominates(node) }
+/**
+ * DEPRECATED: Use `BasicBlock::immediatelyDominates` instead.
+ *
+ * The immediate dominance relation for basic blocks.
+ */
+deprecated predicate bbIDominates(BasicBlock dom, BasicBlock node) {
+  dom.immediatelyDominates(node)
+}
 
 /** Exit points for basic-block control-flow. */
 private predicate bbSink(BasicBlock exit) { exit.getLastNode() instanceof ControlFlow::ExitNode }
 
 /** Reversed `bbSucc`. */
-private predicate bbPred(BasicBlock post, BasicBlock pre) { post = pre.getABBSuccessor() }
+private predicate bbPred(BasicBlock post, BasicBlock pre) { post = pre.getASuccessor() }
 
 /** The immediate post-dominance relation on basic blocks. */
-cached
-predicate bbIPostDominates(BasicBlock dominator, BasicBlock node) =
+deprecated predicate bbIPostDominates(BasicBlock dominator, BasicBlock node) =
   idominance(bbSink/1, bbPred/2)(_, dominator, node)
 
-/** Holds if `dom` strictly dominates `node`. */
-predicate bbStrictlyDominates(BasicBlock dom, BasicBlock node) { bbIDominates+(dom, node) }
-
-/** Holds if `dom` dominates `node`. (This is reflexive.) */
-predicate bbDominates(BasicBlock dom, BasicBlock node) {
-  bbStrictlyDominates(dom, node) or dom = node
+/**
+ * DEPRECATED: Use `BasicBlock::strictlyDominates` instead.
+ *
+ * Holds if `dom` strictly dominates `node`.
+ */
+deprecated predicate bbStrictlyDominates(BasicBlock dom, BasicBlock node) {
+  dom.strictlyDominates(node)
 }
 
-/** Holds if `dom` strictly post-dominates `node`. */
-predicate bbStrictlyPostDominates(BasicBlock dom, BasicBlock node) { bbIPostDominates+(dom, node) }
+/**
+ * DEPRECATED: Use `BasicBlock::dominates` instead.
+ *
+ * Holds if `dom` dominates `node`. (This is reflexive.)
+ */
+deprecated predicate bbDominates(BasicBlock dom, BasicBlock node) { dom.dominates(node) }
 
-/** Holds if `dom` post-dominates `node`. (This is reflexive.) */
-predicate bbPostDominates(BasicBlock dom, BasicBlock node) {
-  bbStrictlyPostDominates(dom, node) or dom = node
+/**
+ * DEPRECATED: Use `BasicBlock::strictlyPostDominates` instead.
+ *
+ * Holds if `dom` strictly post-dominates `node`.
+ */
+deprecated predicate bbStrictlyPostDominates(BasicBlock dom, BasicBlock node) {
+  dom.strictlyPostDominates(node)
 }
 
+/**
+ * DEPRECATED: Use `BasicBlock::postDominates` instead.
+ *
+ * Holds if `dom` post-dominates `node`. (This is reflexive.)
+ */
+deprecated predicate bbPostDominates(BasicBlock dom, BasicBlock node) { dom.postDominates(node) }
+
 /**
  * The dominance frontier relation for basic blocks.
  *
  * This is equivalent to:
  *
  * ```
- *   bbDominates(x, w.getABBPredecessor()) and not bbStrictlyDominates(x, w)
+ *   x.dominates(w.getAPredecessor()) and not x.strictlyDominates(w)
  * ```
  */
 predicate dominanceFrontier(BasicBlock x, BasicBlock w) {
-  x = w.getABBPredecessor() and not bbIDominates(x, w)
+  x = w.getAPredecessor() and not x.immediatelyDominates(w)
   or
   exists(BasicBlock prev | dominanceFrontier(prev, w) |
-    bbIDominates(x, prev) and
-    not bbIDominates(x, w)
+    x.immediatelyDominates(prev) and
+    not x.immediatelyDominates(w)
   )
 }
 
@@ -65,7 +86,7 @@ predicate iDominates(ControlFlowNode dominator, ControlFlowNode node) {
   exists(BasicBlock bb, int i | dominator = bb.getNode(i) and node = bb.getNode(i + 1))
   or
   exists(BasicBlock dom, BasicBlock bb |
-    bbIDominates(dom, bb) and
+    dom.immediatelyDominates(bb) and
     dominator = dom.getLastNode() and
     node = bb.getFirstNode()
   )
@@ -75,7 +96,7 @@ predicate iDominates(ControlFlowNode dominator, ControlFlowNode node) {
 pragma[inline]
 predicate strictlyDominates(ControlFlowNode dom, ControlFlowNode node) {
   // This predicate is gigantic, so it must be inlined.
-  bbStrictlyDominates(dom.getBasicBlock(), node.getBasicBlock())
+  dom.getBasicBlock().strictlyDominates(node.getBasicBlock())
   or
   exists(BasicBlock b, int i, int j | dom = b.getNode(i) and node = b.getNode(j) and i < j)
 }
@@ -84,7 +105,7 @@ predicate strictlyDominates(ControlFlowNode dom, ControlFlowNode node) {
 pragma[inline]
 predicate dominates(ControlFlowNode dom, ControlFlowNode node) {
   // This predicate is gigantic, so it must be inlined.
-  bbStrictlyDominates(dom.getBasicBlock(), node.getBasicBlock())
+  dom.getBasicBlock().strictlyDominates(node.getBasicBlock())
   or
   exists(BasicBlock b, int i, int j | dom = b.getNode(i) and node = b.getNode(j) and i <= j)
 }
@@ -93,7 +114,7 @@ predicate dominates(ControlFlowNode dom, ControlFlowNode node) {
 pragma[inline]
 predicate strictlyPostDominates(ControlFlowNode dom, ControlFlowNode node) {
   // This predicate is gigantic, so it must be inlined.
-  bbStrictlyPostDominates(dom.getBasicBlock(), node.getBasicBlock())
+  dom.getBasicBlock().strictlyPostDominates(node.getBasicBlock())
   or
   exists(BasicBlock b, int i, int j | dom = b.getNode(i) and node = b.getNode(j) and i > j)
 }
@@ -102,7 +123,7 @@ predicate strictlyPostDominates(ControlFlowNode dom, ControlFlowNode node) {
 pragma[inline]
 predicate postDominates(ControlFlowNode dom, ControlFlowNode node) {
   // This predicate is gigantic, so it must be inlined.
-  bbStrictlyPostDominates(dom.getBasicBlock(), node.getBasicBlock())
+  dom.getBasicBlock().strictlyPostDominates(node.getBasicBlock())
   or
   exists(BasicBlock b, int i, int j | dom = b.getNode(i) and node = b.getNode(j) and i >= j)
 }

java/ql/lib/semmle/code/java/controlflow/Guards.qll

@@ -68,7 +68,7 @@ class ConditionBlock extends BasicBlock {
     exists(BasicBlock succ |
       succ = this.getTestSuccessor(testIsTrue) and
       dominatingEdge(this, succ) and
-      succ.bbDominates(controlled)
+      succ.dominates(controlled)
     )
   }
 }
@@ -287,7 +287,7 @@ private predicate switchCaseControls(SwitchCase sc, BasicBlock bb) {
     // Pattern cases are handled as condition blocks
     not sc instanceof PatternCase and
     caseblock.getFirstNode() = sc.getControlFlowNode() and
-    caseblock.bbDominates(bb) and
+    caseblock.dominates(bb) and
     // Check we can't fall through from a previous block:
     forall(ControlFlowNode pred | pred = sc.getControlFlowNode().getAPredecessor() |
       isNonFallThroughPredecessor(sc, pred)
@@ -307,7 +307,7 @@ private predicate preconditionControls(MethodCall ma, BasicBlock controlled, boo
   exists(BasicBlock check, BasicBlock succ |
     preconditionBranchEdge(ma, check, succ, branch) and
     dominatingEdge(check, succ) and
-    succ.bbDominates(controlled)
+    succ.dominates(controlled)
   )
 }
 

java/ql/lib/semmle/code/java/controlflow/Paths.qll

@@ -47,7 +47,7 @@ private predicate callAlwaysPerformsAction(Call call, ActionConfiguration conf)
 private predicate actionDominatesExit(Callable callable, ActionConfiguration conf) {
   exists(ExitBlock exit |
     exit.getEnclosingCallable() = callable and
-    actionBlock(conf).bbDominates(exit)
+    actionBlock(conf).dominates(exit)
   )
 }
 
@@ -56,12 +56,12 @@ private BasicBlock nonDominatingActionBlock(ActionConfiguration conf) {
   exists(ExitBlock exit |
     result = actionBlock(conf) and
     exit.getEnclosingCallable() = result.getEnclosingCallable() and
-    not result.bbDominates(exit)
+    not result.dominates(exit)
   )
 }
 
 private class JoinBlock extends BasicBlock {
-  JoinBlock() { 2 <= strictcount(this.getABBPredecessor()) }
+  JoinBlock() { 2 <= strictcount(this.getAPredecessor()) }
 }
 
 /**
@@ -72,8 +72,8 @@ private predicate postActionBlock(BasicBlock bb, ActionConfiguration conf) {
   bb = nonDominatingActionBlock(conf)
   or
   if bb instanceof JoinBlock
-  then forall(BasicBlock pred | pred = bb.getABBPredecessor() | postActionBlock(pred, conf))
-  else postActionBlock(bb.getABBPredecessor(), conf)
+  then forall(BasicBlock pred | pred = bb.getAPredecessor() | postActionBlock(pred, conf))
+  else postActionBlock(bb.getAPredecessor(), conf)
 }
 
 /** Holds if every path through `callable` goes through at least one action node. */

java/ql/lib/semmle/code/java/controlflow/UnreachableBlocks.qll

@@ -209,7 +209,7 @@ class UnreachableBasicBlock extends BasicBlock {
     or
     // This block is not reachable in the CFG, and is not the entrypoint in a callable, an
     // expression in an assert statement, or a catch clause.
-    forall(BasicBlock bb | bb = this.getABBPredecessor() | bb instanceof UnreachableBasicBlock) and
+    forall(BasicBlock bb | bb = this.getAPredecessor() | bb instanceof UnreachableBasicBlock) and
     not exists(Callable c | c.getBody().getControlFlowNode() = this.getFirstNode()) and
     not this.getFirstNode().asExpr().getEnclosingStmt() instanceof AssertStmt and
     not this.getFirstNode().asStmt() instanceof CatchClause
@@ -219,11 +219,10 @@ class UnreachableBasicBlock extends BasicBlock {
       // Not accessible from the switch expression
       unreachableCaseBlock = constSwitchStmt.getAFailingCase().getBasicBlock() and
       // Not accessible from the successful case
-      not constSwitchStmt.getMatchingCase().getBasicBlock().getABBSuccessor*() =
-        unreachableCaseBlock
+      not constSwitchStmt.getMatchingCase().getBasicBlock().getASuccessor*() = unreachableCaseBlock
     |
       // Blocks dominated by an unreachable case block are unreachable
-      unreachableCaseBlock.bbDominates(this)
+      unreachableCaseBlock.dominates(this)
     )
   }
 }

java/ql/lib/semmle/code/java/controlflow/internal/GuardsLogic.qll

@@ -239,7 +239,7 @@ SsaVariable getADefinition(SsaVariable v, boolean fromBackEdge) {
   exists(SsaVariable inp, BasicBlock bb, boolean fbe |
     v.(SsaPhiNode).hasInputFromBlock(inp, bb) and
     result = getADefinition(inp, fbe) and
-    (if v.getBasicBlock().bbDominates(bb) then fromBackEdge = true else fromBackEdge = fbe)
+    (if v.getBasicBlock().dominates(bb) then fromBackEdge = true else fromBackEdge = fbe)
   )
 }
 
@@ -306,7 +306,7 @@ private predicate guardControlsPhiBranch(
   guard.directlyControls(upd.getBasicBlock(), branch) and
   upd.getDefiningExpr().(VariableAssign).getSource() = e and
   upd = phi.getAPhiInput() and
-  guard.getBasicBlock().bbStrictlyDominates(phi.getBasicBlock())
+  guard.getBasicBlock().strictlyDominates(phi.getBasicBlock())
 }
 
 /**
@@ -331,7 +331,7 @@ private predicate conditionalAssign(SsaVariable v, Guard guard, boolean branch,
     forall(SsaVariable other | other != upd and other = phi.getAPhiInput() |
       guard.directlyControls(other.getBasicBlock(), branch.booleanNot())
       or
-      other.getBasicBlock().bbDominates(guard.getBasicBlock()) and
+      other.getBasicBlock().dominates(guard.getBasicBlock()) and
       not other.isLiveAtEndOfBlock(getAGuardBranchSuccessor(guard, branch))
     )
   )

java/ql/lib/semmle/code/java/dataflow/Nullness.qll

@@ -298,7 +298,7 @@ private predicate impossibleEdge(BasicBlock bb1, BasicBlock bb2) {
 private predicate leavingFinally(BasicBlock bb1, BasicBlock bb2, boolean normaledge) {
   exists(TryStmt try, BlockStmt finally |
     try.getFinally() = finally and
-    bb1.getABBSuccessor() = bb2 and
+    bb1.getASuccessor() = bb2 and
     bb1.getFirstNode().getEnclosingStmt().getEnclosingStmt*() = finally and
     not bb2.getFirstNode().getEnclosingStmt().getEnclosingStmt*() = finally and
     if bb1.getLastNode().getANormalSuccessor() = bb2.getFirstNode()
@@ -339,7 +339,7 @@ private predicate nullVarStep(
   midssa.isLiveAtEndOfBlock(mid) and
   not ensureNotNull(midssa).getBasicBlock() = mid and
   not assertFail(mid, _) and
-  bb = mid.getABBSuccessor() and
+  bb = mid.getASuccessor() and
   not impossibleEdge(mid, bb) and
   not exists(boolean branch | nullGuard(midssa, branch, false).hasBranchEdge(mid, bb, branch)) and
   not (leavingFinally(mid, bb, true) and midstoredcompletion = true) and

java/ql/lib/semmle/code/java/dataflow/RangeAnalysis.qll

@@ -209,7 +209,7 @@ module Sem implements Semantic<Location> {
 
   class BasicBlock = J::BasicBlock;
 
-  BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getABBSuccessor() }
+  BasicBlock getABasicBlockSuccessor(BasicBlock bb) { result = bb.getASuccessor() }
 
   private predicate id(ExprParent x, ExprParent y) { x = y }
 

java/ql/lib/semmle/code/java/dataflow/TypeFlow.qll

@@ -321,14 +321,14 @@ private module Input implements TypeFlowInput<Location> {
    */
   private predicate instanceofDisjunct(InstanceOfExpr ioe, BasicBlock bb, BaseSsaVariable v) {
     ioe.getExpr() = v.getAUse() and
-    strictcount(bb.getABBPredecessor()) > 1 and
+    strictcount(bb.getAPredecessor()) > 1 and
     exists(ConditionBlock cb | cb.getCondition() = ioe and cb.getTestSuccessor(true) = bb)
   }
 
   /** Holds if `bb` is disjunctively guarded by multiple `instanceof` tests on `v`. */
   private predicate instanceofDisjunction(BasicBlock bb, BaseSsaVariable v) {
     strictcount(InstanceOfExpr ioe | instanceofDisjunct(ioe, bb, v)) =
-      strictcount(bb.getABBPredecessor())
+      strictcount(bb.getAPredecessor())
   }
 
   /**
@@ -338,7 +338,7 @@ private module Input implements TypeFlowInput<Location> {
   predicate instanceofDisjunctionGuarded(TypeFlowNode n, RefType t) {
     exists(BasicBlock bb, InstanceOfExpr ioe, BaseSsaVariable v, VarAccess va |
       instanceofDisjunction(bb, v) and
-      bb.bbDominates(va.getBasicBlock()) and
+      bb.dominates(va.getBasicBlock()) and
       va = v.getAUse() and
       instanceofDisjunct(ioe, bb, v) and
       t = ioe.getSyntacticCheckedType() and