Address review comments

Author: hvitved

rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

@@ -213,6 +213,20 @@ private ExprCfgNode getALastEvalNode(ExprCfgNode e) {
   result.(BreakExprCfgNode).getTarget() = e
 }
 
+/**
+ * Holds if a reverse local flow step should be added from the post-update node
+ * for `e` to the post-update node for the result.
+ *
+ * This is needed to allow for side-effects on compound expressions to propagate
+ * to sub components. For example, in
+ *
+ * ```rust
+ * ({ foo(); &mut a}).set_data(taint);
+ * ```
+ *
+ * we add a reverse flow step from `[post] { foo(); &mut a}` to `[post] &mut a`,
+ * in order for the side-effect of `set_data` to reach `&mut a`.
+ */
 ExprCfgNode getPostUpdateReverseStep(ExprCfgNode e, boolean preservesValue) {
   result = getALastEvalNode(e) and
   preservesValue = true

rust/ql/lib/codeql/rust/dataflow/internal/Node.qll

@@ -29,7 +29,7 @@ abstract class NodePublic extends TNode {
   /**
    * Gets the expression that corresponds to this node, if any.
    */
-  ExprCfgNode asExpr() { none() }
+  final ExprCfgNode asExpr() { this = TExprNode(result) }
 
   /**
    * Gets the parameter that corresponds to this node, if any.
@@ -39,7 +39,7 @@ abstract class NodePublic extends TNode {
   /**
    * Gets the pattern that corresponds to this node, if any.
    */
-  PatCfgNode asPat() { none() }
+  final PatCfgNode asPat() { this = TPatNode(result) }
 }
 
 abstract class Node extends NodePublic {
@@ -144,16 +144,12 @@ class ExprNode extends AstCfgFlowNode, TExprNode {
   override ExprCfgNode n;
 
   ExprNode() { this = TExprNode(n) }
-
-  override ExprCfgNode asExpr() { result = n }
 }
 
 final class PatNode extends AstCfgFlowNode, TPatNode {
   override PatCfgNode n;
 
   PatNode() { this = TPatNode(n) }
-
-  override PatCfgNode asPat() { result = n }
 }
 
 /** A data flow node that corresponds to a name node in the CFG. */
@@ -467,16 +463,13 @@ newtype TNode =
     or
     e =
       [
-        any(IndexExprCfgNode i).getBase(), any(FieldExprCfgNode access).getExpr(),
-        any(TryExprCfgNode try).getExpr(),
-        any(PrefixExprCfgNode pe | pe.getOperatorName() = "*").getExpr(),
-        any(AwaitExprCfgNode a).getExpr(), any(MethodCallExprCfgNode mc).getReceiver()
+        any(IndexExprCfgNode i).getBase(), //
+        any(FieldExprCfgNode access).getExpr(), //
+        any(TryExprCfgNode try).getExpr(), //
+        any(PrefixExprCfgNode pe | pe.getOperatorName() = "*").getExpr(), //
+        any(AwaitExprCfgNode a).getExpr(), any(MethodCallExprCfgNode mc).getReceiver(), //
+        getPostUpdateReverseStep(any(PostUpdateNode n).getPreUpdateNode().asExpr(), _)
       ]
-    or
-    exists(ExprCfgNode pred |
-      exists(TExprPostUpdateNode(pred)) and
-      e = getPostUpdateReverseStep(pred, _)
-    )
   } or
   TReceiverNode(MethodCallExprCfgNode mc, Boolean isPost) or
   TSsaNode(SsaImpl::DataFlowIntegration::SsaNode node) or