C++: QLDoc and naming updates for implicit destructors in IR

Author: rdmarsh2

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

@@ -47,8 +47,8 @@ abstract class TranslatedCall extends TranslatedExpr {
     else result = this.getFirstCallTargetInstruction(kind)
   }
 
-  override Instruction getLastInstructionInternal() {
-    result = this.getSideEffects().getLastInstruction()
+  override Instruction getALastInstructionInternal() {
+    result = this.getSideEffects().getALastInstruction()
   }
 
   override TranslatedElement getLastChild() { result = this.getSideEffects() }
@@ -94,7 +94,6 @@ abstract class TranslatedCall extends TranslatedExpr {
   }
 
   override Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind) {
-    kind instanceof GotoEdge and
     tag = CallTag() and
     result = this.getSideEffects().getFirstInstruction(kind)
   }
@@ -257,9 +256,9 @@ abstract class TranslatedSideEffects extends TranslatedElement {
     result = this.getParent().getChildSuccessor(this, kind)
   }
 
-  override Instruction getLastInstructionInternal() {
+  override Instruction getALastInstructionInternal() {
     if exists(this.getAChild())
-    then result = this.getChild(max(int i | exists(this.getChild(i)))).getLastInstruction()
+    then result = this.getChild(max(int i | exists(this.getChild(i)))).getALastInstruction()
     else
       // If there are no side effects, the "last" instruction should be the parent call's last
       // instruction, so that implicit destructors can be inserted in the right place.
@@ -455,7 +454,7 @@ abstract class TranslatedSideEffect extends TranslatedElement {
     kind instanceof GotoEdge
   }
 
-  override Instruction getLastInstructionInternal() {
+  override Instruction getALastInstructionInternal() {
     result = this.getInstruction(OnlyInstructionTag())
   }
 

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

@@ -58,8 +58,8 @@ abstract class TranslatedFlexibleCondition extends TranslatedCondition, Conditio
     result = this.getOperand().getFirstInstruction(kind)
   }
 
-  final override Instruction getLastInstructionInternal() {
-    result = this.getOperand().getLastInstruction()
+  final override Instruction getALastInstructionInternal() {
+    result = this.getOperand().getALastInstruction()
   }
 
   final override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
@@ -70,7 +70,7 @@ abstract class TranslatedFlexibleCondition extends TranslatedCondition, Conditio
     none()
   }
 
-  final override Instruction getChildSuccessor(TranslatedElement child, EdgeKind kind) { none() }
+  final override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) { none() }
 
   abstract TranslatedCondition getOperand();
 }
@@ -96,7 +96,7 @@ class TranslatedParenthesisCondition extends TranslatedFlexibleCondition {
 abstract class TranslatedNativeCondition extends TranslatedCondition, TTranslatedNativeCondition {
   TranslatedNativeCondition() { this = TTranslatedNativeCondition(expr) }
 
-  final override Instruction getChildSuccessor(TranslatedElement child, EdgeKind kind) { none() }
+  final override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) { none() }
 }
 
 abstract class TranslatedBinaryLogicalOperation extends TranslatedNativeCondition, ConditionContext {
@@ -114,10 +114,10 @@ abstract class TranslatedBinaryLogicalOperation extends TranslatedNativeConditio
     result = this.getLeftOperand().getFirstInstruction(kind)
   }
 
-  final override Instruction getLastInstructionInternal() {
-    result = this.getLeftOperand().getLastInstruction()
+  final override Instruction getALastInstructionInternal() {
+    result = this.getLeftOperand().getALastInstruction()
     or
-    result = this.getRightOperand().getLastInstruction()
+    result = this.getRightOperand().getALastInstruction()
   }
 
   final override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
@@ -180,7 +180,7 @@ class TranslatedValueCondition extends TranslatedCondition, TTranslatedValueCond
     result = this.getValueExpr().getFirstInstruction(kind)
   }
 
-  override Instruction getLastInstructionInternal() {
+  override Instruction getALastInstructionInternal() {
     result = this.getInstruction(ValueConditionConditionalBranchTag())
   }
 
@@ -192,7 +192,7 @@ class TranslatedValueCondition extends TranslatedCondition, TTranslatedValueCond
     resultType = getVoidType()
   }
 
-  override Instruction getChildSuccessor(TranslatedElement child, EdgeKind kind) {
+  override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
     child = this.getValueExpr() and
     result = this.getInstruction(ValueConditionConditionalBranchTag()) and
     kind instanceof GotoEdge

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

@@ -156,7 +156,7 @@ class TranslatedStaticLocalVariableDeclarationEntry extends TranslatedDeclaratio
     kind instanceof GotoEdge
   }
 
-  final override Instruction getLastInstructionInternal() {
+  final override Instruction getALastInstructionInternal() {
     result = this.getInstruction(DynamicInitializationConditionalBranchTag())
     or
     result = this.getInstruction(DynamicInitializationFlagStoreTag())
@@ -188,7 +188,7 @@ class TranslatedStaticLocalVariableDeclarationEntry extends TranslatedDeclaratio
     result = this.getParent().getChildSuccessor(this, kind)
   }
 
-  final override Instruction getChildSuccessor(TranslatedElement child, EdgeKind kind) {
+  final override Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) {
     child = this.getInitialization() and
     result = this.getInstruction(DynamicInitializationFlagConstantTag()) and
     kind instanceof GotoEdge

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

@@ -23,15 +23,6 @@ private import SideEffects
 Element getRealParent(Expr expr) {
   result = expr.getParentWithConversions()
   or
-  /*
-   * exists(Stmt destructorParent, DestructorCall dc |
-   *    destructorParent.getAnImplicitDestructorCall() = dc and
-   *    dc.getQualifier() = expr and
-   *    result = dc
-   *  )
-   *  or
-   */
-
   result.(Destructor).getADestruction() = expr
   or
   result.(Expr).getAnImplicitDestructorCall() = expr
@@ -873,12 +864,18 @@ abstract class TranslatedElement extends TTranslatedElement {
   /**
    * Holds if this element has implicit destructor calls that should follow it.
    */
-  predicate hasImplicitDestructorCalls() { none() }
+  predicate hasAnImplicitDestructorCall() { none() }
 
   /**
+   * Gets the child index of the first destructor call that should be executed after this `TranslatedElement`
    */
   int getFirstDestructorCallIndex() { none() }
 
+  /**
+   * Holds if this `TranslatedElement` includes any destructor calls that must be performed after
+   * it in its `getChildSuccessorInternal`, `getInstructionSuccessorInternal`, and
+   * `getALastInstructionInternal` relations, rather than needing them inserted.
+   */
   predicate handlesDestructorsExplicitly() { none() }
 
   private int getUniqueId() {
@@ -916,42 +913,65 @@ abstract class TranslatedElement extends TTranslatedElement {
   /**
    * Gets the successor instruction of the instruction that was generated by
    * this element for tag `tag`. The successor edge kind is specified by `kind`.
+   * This predicate does not usually include destructors, which are inserted as
+   * part of `getInstructionSuccessor` unless `handlesDestructorsExplicitly`
+   * holds.
    */
   abstract Instruction getInstructionSuccessorInternal(InstructionTag tag, EdgeKind kind);
-
-  Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
+  /**
+   * Gets the successor instruction of the instruction that was generated by
+   * this element for tag `tag`. The successor edge kind is specified by `kind`.
+   */
+  final Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) {
     if
-      this.hasImplicitDestructorCalls() and
-      this.getInstruction(tag) = this.getLastInstructionInternal() and
+      this.hasAnImplicitDestructorCall() and
+      this.getInstruction(tag) = this.getALastInstructionInternal() and
       not this.handlesDestructorsExplicitly()
     then
       result = this.getChild(this.getFirstDestructorCallIndex()).getFirstInstruction(kind) and
       kind instanceof GotoEdge
     else result = this.getInstructionSuccessorInternal(tag, kind)
   }
 
-  final Instruction getLastInstruction() {
-    if this.hasImplicitDestructorCalls() and not this.handlesDestructorsExplicitly()
-    then result = this.getChild(max(int n | exists(this.getChild(n)))).getLastInstruction() // last destructor
-    else result = this.getLastInstructionInternal()
+  /**
+   * Gets an instruction within this `TranslatedElement` (including its transitive children) which
+   * will be followed by an instruction outside the `TranslatedElement`.
+   */
+  final Instruction getALastInstruction() {
+    if this.hasAnImplicitDestructorCall() and not this.handlesDestructorsExplicitly()
+    then result = this.getChild(max(int n | exists(this.getChild(n)))).getALastInstruction() // last destructor
+    else result = this.getALastInstructionInternal()
   }
 
-  abstract Instruction getLastInstructionInternal();
+  /**
+   * Gets an instruction within this `TranslatedElement` (including its transitive children) which
+   * will be followed by an instruction outside the `TranslatedElement`.
+   * This predicate does not usually include destructors, which are inserted as
+   * part of `getALastInstruction` unless `handlesDestructorsExplicitly` holds.
+   */
+  abstract Instruction getALastInstructionInternal();
 
   TranslatedElement getLastChild() { none() }
 
   /**
    * Gets the successor instruction to which control should flow after the
    * child element specified by `child` has finished execution. The successor
    * edge kind is specified by `kind`.
+   * This predicate does not usually include destructors, which are inserted as
+   * part of `getChildSuccessor` unless `handlesDestructorsExplicitly` holds.
    */
   Instruction getChildSuccessorInternal(TranslatedElement child, EdgeKind kind) { none() }
 
-  Instruction getChildSuccessor(TranslatedElement child, EdgeKind kind) {
+  /**
+   * Gets the successor instruction to which control should flow after the
+   * child element specified by `child` has finished execution. The successor
+   * edge kind is specified by `kind`.
+   */
+  final Instruction getChildSuccessor(TranslatedElement child, EdgeKind kind) {
     (
       if
         // this is the last child and we need to handle destructors for it
-        this.hasImplicitDestructorCalls() and
+        this.hasAnImplicitDestructorCall() and
         not this.handlesDestructorsExplicitly() and
         child = this.getLastChild()
       then result = this.getChild(this.getFirstDestructorCallIndex()).getFirstInstruction(kind)