More fixes for the PR comments

Author: AndreiDiaconu1

csharp/ql/src/semmle/code/csharp/ir/implementation/raw/internal/IRConstruction.qll

@@ -139,7 +139,6 @@ private module Cached {
     // Compiler generated foreach while loop:
     // Same as above
     exists(TranslatedForeachWhile s |
-      s instanceof TranslatedForeachWhile and
       result = s.getFirstInstruction() and
       exists(TranslatedElement inBody, InstructionTag tag |
         result = inBody.getInstructionSuccessor(tag, kind) and

csharp/ql/src/semmle/code/csharp/ir/implementation/raw/internal/InstructionTag.qll

@@ -1,9 +1,6 @@
 import csharp
 import semmle.code.csharp.ir.Util
 
-//private predicate fieldIsInitialized(Field field) {
-//  exists(field.getInitializer())
-//}
 private predicate elementIsInitialized(int elementIndex) {
   exists(ArrayInitWithMod initList | initList.isInitialized(elementIndex))
 }

csharp/ql/src/semmle/code/csharp/ir/implementation/raw/internal/TranslatedCall.qll

@@ -5,21 +5,18 @@ private import InstructionTag
 private import TranslatedElement
 private import TranslatedExpr
 private import semmle.code.csharp.ir.Util
-private import semmle.code.csharp.ir.implementation.raw.internal.common.TranslatedCallBlueprint
+private import semmle.code.csharp.ir.implementation.raw.internal.common.TranslatedCallBase
 private import semmle.code.csharp.ir.internal.IRCSharpLanguage as Language
 
 /**
  * The IR translation of a call to a function. The function can be a normal function
- * (eg. `MethodCall`) or a constructor call (eg. `ObjectCreation`). Notice that the 
+ * (ie. `MethodCall`) or a constructor call (ie. `ObjectCreation`). Notice that the
  * AST generated translated calls are tied to an expression (unlike compiler generated ones,
  * which can be attached to either a statement or an expression).
  */
+abstract class TranslatedCall extends TranslatedExpr, TranslatedCallBase {
+  final override Instruction getResult() { result = TranslatedCallBase.super.getResult() }
 
-abstract class TranslatedCall extends TranslatedExpr, TranslatedCallBlueprint {
-  final override Instruction getResult() {
-    result = TranslatedCallBlueprint.super.getResult()
-  }
-  
   override Instruction getUnmodeledDefinitionInstruction() {
     result = this.getEnclosingFunction().getUnmodeledDefinitionInstruction()
   }
@@ -30,11 +27,10 @@ abstract class TranslatedCall extends TranslatedExpr, TranslatedCallBlueprint {
  * `MethodCall`, `LocalFunctionCall`, `AccessorCall`, `OperatorCall`.
  * Note that `DelegateCall`s are not treated here since they need to be desugared.
  */
-
 class TranslatedFunctionCall extends TranslatedNonConstantExpr, TranslatedCall {
   override Call expr;
-  
-  TranslatedFunctionCall() { 
+
+  TranslatedFunctionCall() {
     expr instanceof MethodCall or
     expr instanceof LocalFunctionCall or
     expr instanceof AccessorCall or
@@ -45,27 +41,19 @@ class TranslatedFunctionCall extends TranslatedNonConstantExpr, TranslatedCall {
     tag = CallTargetTag() and result = expr.getTarget()
   }
 
-  override predicate hasArguments() { 
-    exists(expr.getArgument(0)) 
-  }
-  
   override TranslatedExpr getArgument(int index) {
     result = getTranslatedExpr(expr.getArgument(index))
   }
-  
+
   override TranslatedExpr getQualifier() {
     expr instanceof QualifiableExpr and
     result = getTranslatedExpr(expr.(QualifiableExpr).getQualifier())
   }
-  
-  override Instruction getQualifierResult() {
-    result = this.getQualifier().getResult()
-  }
-  
-  override Type getCallResultType() { 
-    result = expr.getTarget().getReturnType()
-  }
-  
+
+  override Instruction getQualifierResult() { result = this.getQualifier().getResult() }
+
+  override Type getCallResultType() { result = expr.getTarget().getReturnType() }
+
   override predicate hasReadSideEffect() {
     not expr.getTarget().(SideEffectFunction).neverReadsMemory()
   }
@@ -83,34 +71,26 @@ class TranslatedFunctionCall extends TranslatedNonConstantExpr, TranslatedCall {
  */
 class TranslatedConstructorCall extends TranslatedNonConstantExpr, TranslatedCall {
   override Call expr;
-  
-  TranslatedConstructorCall() { 
+
+  TranslatedConstructorCall() {
     expr instanceof ObjectCreation or
     expr instanceof ConstructorInitializer
   }
 
   override Callable getInstructionFunction(InstructionTag tag) {
     tag = CallTargetTag() and result = expr.getTarget()
   }
-  
-  override predicate hasArguments() { 
-    exists(expr.getArgument(0)) 
-  }
-  
+
   override TranslatedExpr getArgument(int index) {
     result = getTranslatedExpr(expr.getArgument(index))
   }
-  
+
   // The qualifier for a constructor call has already been generated
   // (the `NewObj` instruction)
-  override TranslatedExpr getQualifier() {
-    none()
-  }
-  
-  override Type getCallResultType() { 
-    result instanceof VoidType
-  }
-  
+  override TranslatedExpr getQualifier() { none() }
+
+  override Type getCallResultType() { result instanceof VoidType }
+
   override Instruction getQualifierResult() {
     exists(ConstructorCallContext context |
       context = this.getParent() and

csharp/ql/src/semmle/code/csharp/ir/implementation/raw/internal/TranslatedCondition.qll

@@ -4,23 +4,19 @@ private import semmle.code.csharp.ir.implementation.internal.OperandTag
 private import InstructionTag
 private import TranslatedElement
 private import TranslatedExpr
-private import common.TranslatedConditionBlueprint
+private import common.TranslatedConditionBase
 private import semmle.code.csharp.ir.internal.IRCSharpLanguage as Language
 
-TranslatedCondition getTranslatedCondition(Expr expr) {
-  result.getExpr() = expr
-}
+TranslatedCondition getTranslatedCondition(Expr expr) { result.getExpr() = expr }
 
-abstract class TranslatedCondition extends ConditionBlueprint {
+abstract class TranslatedCondition extends ConditionBase {
   Expr expr;
 
   final override string toString() { result = expr.toString() }
 
   final override Language::AST getAST() { result = expr }
 
-  final Expr getExpr() {
-    result = expr
-  }
+  final Expr getExpr() { result = expr }
 
   final override Callable getFunction() { result = expr.getEnclosingCallable() }
 
@@ -53,12 +49,12 @@ abstract class TranslatedFlexibleCondition extends TranslatedCondition, Conditio
 class TranslatedParenthesisCondition extends TranslatedFlexibleCondition {
   override ParenthesizedExpr expr;
 
-  final override Instruction getChildTrueSuccessor(ConditionBlueprint child) {
+  final override Instruction getChildTrueSuccessor(ConditionBase child) {
     child = this.getOperand() and
     result = this.getConditionContext().getChildTrueSuccessor(this)
   }
 
-  final override Instruction getChildFalseSuccessor(ConditionBlueprint child) {
+  final override Instruction getChildFalseSuccessor(ConditionBase child) {
     child = this.getOperand() and
     result = this.getConditionContext().getChildFalseSuccessor(this)
   }
@@ -71,12 +67,12 @@ class TranslatedParenthesisCondition extends TranslatedFlexibleCondition {
 class TranslatedNotCondition extends TranslatedFlexibleCondition {
   override LogicalNotExpr expr;
 
-  override Instruction getChildTrueSuccessor(ConditionBlueprint child) {
+  override Instruction getChildTrueSuccessor(ConditionBase child) {
     child = this.getOperand() and
     result = this.getConditionContext().getChildFalseSuccessor(this)
   }
 
-  override Instruction getChildFalseSuccessor(ConditionBlueprint child) {
+  override Instruction getChildFalseSuccessor(ConditionBase child) {
     child = this.getOperand() and
     result = this.getConditionContext().getChildTrueSuccessor(this)
   }
@@ -128,18 +124,15 @@ abstract class TranslatedBinaryLogicalOperation extends TranslatedNativeConditio
 class TranslatedLogicalAndExpr extends TranslatedBinaryLogicalOperation {
   TranslatedLogicalAndExpr() { expr instanceof LogicalAndExpr }
 
-  override Instruction getChildTrueSuccessor(ConditionBlueprint child) {
-    (
-      child = this.getLeftOperand() and
-      result = this.getRightOperand().getFirstInstruction()
-    ) or
-    (
-      child = this.getRightOperand() and
-      result = this.getConditionContext().getChildTrueSuccessor(this)
-    )
+  override Instruction getChildTrueSuccessor(ConditionBase child) {
+    child = this.getLeftOperand() and
+    result = this.getRightOperand().getFirstInstruction()
+    or
+    child = this.getRightOperand() and
+    result = this.getConditionContext().getChildTrueSuccessor(this)
   }
 
-  override Instruction getChildFalseSuccessor(ConditionBlueprint child) {
+  override Instruction getChildFalseSuccessor(ConditionBase child) {
     child = this.getAnOperand() and
     result = this.getConditionContext().getChildFalseSuccessor(this)
   }
@@ -148,34 +141,25 @@ class TranslatedLogicalAndExpr extends TranslatedBinaryLogicalOperation {
 class TranslatedLogicalOrExpr extends TranslatedBinaryLogicalOperation {
   override LogicalOrExpr expr;
 
-  override Instruction getChildTrueSuccessor(ConditionBlueprint child) {
+  override Instruction getChildTrueSuccessor(ConditionBase child) {
     child = getAnOperand() and
     result = this.getConditionContext().getChildTrueSuccessor(this)
   }
 
-  override Instruction getChildFalseSuccessor(ConditionBlueprint child) {
-    (
-      child = this.getLeftOperand() and
-      result = getRightOperand().getFirstInstruction()
-    ) or
-    (
-      child = this.getRightOperand() and
-      result = this.getConditionContext().getChildFalseSuccessor(this)
-    )
+  override Instruction getChildFalseSuccessor(ConditionBase child) {
+    child = this.getLeftOperand() and
+    result = getRightOperand().getFirstInstruction()
+    or
+    child = this.getRightOperand() and
+    result = this.getConditionContext().getChildFalseSuccessor(this)
   }
 }
 
-class TranslatedValueCondition extends TranslatedCondition, ValueConditionBlueprint,
-                                       TTranslatedValueCondition {
-  TranslatedValueCondition() {
-    this = TTranslatedValueCondition(expr)
-  }
-  
-  override TranslatedExpr getValueExpr() {
-    result = getTranslatedExpr(expr)
-  }
-  
-  override Instruction valueExprResult() {
-    result = this.getValueExpr().getResult()
-  }
+class TranslatedValueCondition extends TranslatedCondition, ValueConditionBase,
+  TTranslatedValueCondition {
+  TranslatedValueCondition() { this = TTranslatedValueCondition(expr) }
+
+  override TranslatedExpr getValueExpr() { result = getTranslatedExpr(expr) }
+
+  override Instruction valueExprResult() { result = this.getValueExpr().getResult() }
 }

csharp/ql/src/semmle/code/csharp/ir/implementation/raw/internal/TranslatedDeclaration.qll

@@ -7,7 +7,7 @@ private import TranslatedElement
 private import TranslatedExpr
 private import TranslatedInitialization
 private import semmle.code.csharp.ir.internal.IRCSharpLanguage as Language
-private import common.TranslatedDeclarationBlueprint
+private import common.TranslatedDeclarationBase
 
 /**
  * Gets the `TranslatedDeclaration` that represents the declaration
@@ -36,29 +36,21 @@ abstract class TranslatedLocalDeclaration extends TranslatedElement, TTranslated
  * Represents the IR translation of the declaration of a local variable,
  * including its initialization, if any.
  */
-class TranslatedLocalVariableDeclaration extends TranslatedLocalDeclaration, LocalVariableDeclarationBlueprint,
-                                                 InitializationContext {
+class TranslatedLocalVariableDeclaration extends TranslatedLocalDeclaration,
+  LocalVariableDeclarationBase, InitializationContext {
   LocalVariable var;
-  
-  TranslatedLocalVariableDeclaration() {
-    var = expr.getVariable()
-  }
-  
+
+  TranslatedLocalVariableDeclaration() { var = expr.getVariable() }
+
   override Instruction getTargetAddress() {
     result = this.getInstruction(InitializerVariableAddressTag())
   }
 
-  override LocalVariable getDeclVar() {
-    result = var
-  }
+  override LocalVariable getDeclVar() { result = var }
 
-  override Type getVarType() {
-    result = getVariableType(getDeclVar()) 
-  }
+  override Type getVarType() { result = getVariableType(getDeclVar()) }
 
-  override Type getTargetType() {
-    result = getVariableType(var)
-  }
+  override Type getTargetType() { result = getVariableType(var) }
 
   override IRVariable getInstructionVariable(InstructionTag tag) {
     (
@@ -68,7 +60,7 @@ class TranslatedLocalVariableDeclaration extends TranslatedLocalDeclaration, Loc
     ) and
     result = getIRUserVariable(getFunction(), getDeclVar())
   }
-  
+
   override TranslatedInitialization getInitialization() {
     // First complex initializations
     if var.getInitializer() instanceof ArrayCreation
@@ -81,7 +73,5 @@ class TranslatedLocalVariableDeclaration extends TranslatedLocalDeclaration, Loc
         result = getTranslatedInitialization(var.getInitializer())
   }
 
-  override predicate isInitializedByElement() {
-    expr.getParent() instanceof IsExpr
-  }
+  override predicate isInitializedByElement() { expr.getParent() instanceof IsExpr }
 }

csharp/ql/src/semmle/code/csharp/ir/implementation/raw/internal/TranslatedElement.qll

@@ -22,12 +22,17 @@ ArrayType getArrayOfDim(int dim, Type type) {
   result.getElementType() = type
 }
 
-predicate canCreateCompilerGeneratedElement (int nth) {
+private predicate canCreateCompilerGeneratedElement(Element generatedBy, int nth) {
+  (
+    generatedBy instanceof ForeachStmt or
+    generatedBy instanceof LockStmt or
+    generatedBy instanceof DelegateCreation or
+    generatedBy instanceof DelegateCall
+  ) and
   // For now we allow a max of 15 compiler generated elements
   nth in [0 .. 14]
 }
 
-
 /**
  * Gets the "real" parent of `expr`. This predicate treats conversions as if
  * they were explicit nodes in the expression tree, rather than as implicit
@@ -63,10 +68,8 @@ private predicate ignoreExprAndDescendants(Expr expr) {
   or
   // Ignore the local declaration done by a `ForeachStmt`
   // since we desugar it
-  (
-    expr instanceof LocalVariableDeclExpr and
-    expr.getParent().getParent() instanceof ForeachStmt
-  )
+  expr instanceof LocalVariableDeclExpr and
+  expr.getParent() instanceof ForeachStmt
   or
   ignoreExprAndDescendants(getRealParent(expr)) // recursive case
 }
@@ -272,14 +275,14 @@ newtype TTranslatedElement =
   TTranslatedDeclaration(LocalVariableDeclExpr entry) {
     // foreach var decl and init is treated separately,
     // because foreach needs desugaring
-    not entry.getParent() instanceof ForeachStmt
+    not ignoreExprAndDescendants(entry)
   } or
-  // A compiler generated element, generated by `generatedBy` during the 
+  // A compiler generated element, generated by `generatedBy` during the
   // desugaring process
   TTranslatedCompilerGeneratedElement(Element generatedBy, int index) {
-    canCreateCompilerGeneratedElement(index)
+    canCreateCompilerGeneratedElement(generatedBy, index)
   }
-  
+
 /**
  * Gets the index of the first explicitly initialized element in `initList`
  * whose index is greater than `afterElementIndex`, where `afterElementIndex`