Address review comments

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll

@@ -391,14 +391,11 @@ private module Cached {
 
   /**
    * Holds if `p` is the parameter of a viable dispatch target of `call`,
-   * and `p` matches arguments at position `apos`.
+   * and `p` has position `ppos`.
    */
   pragma[nomagic]
-  private predicate viableParam(DataFlowCall call, ArgumentPosition apos, ParamNode p) {
-    exists(ParameterPosition ppos |
-      p.isParameterOf(viableCallableExt(call), ppos) and
-      parameterMatch(ppos, apos)
-    )
+  private predicate viableParam(DataFlowCall call, ParameterPosition ppos, ParamNode p) {
+    p.isParameterOf(viableCallableExt(call), ppos)
   }
 
   /**
@@ -407,9 +404,9 @@ private module Cached {
    */
   cached
   predicate viableParamArg(DataFlowCall call, ParamNode p, ArgNode arg) {
-    exists(ArgumentPosition pos |
-      viableParam(call, pos, p) and
-      arg.argumentOf(call, pos) and
+    exists(ParameterPosition ppos |
+      viableParam(call, ppos, p) and
+      argumentPositionMatch(call, arg, ppos) and
       compatibleTypes(getNodeDataFlowType(arg), getNodeDataFlowType(p))
     )
   }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll

@@ -391,14 +391,11 @@ private module Cached {
 
   /**
    * Holds if `p` is the parameter of a viable dispatch target of `call`,
-   * and `p` matches arguments at position `apos`.
+   * and `p` has position `ppos`.
    */
   pragma[nomagic]
-  private predicate viableParam(DataFlowCall call, ArgumentPosition apos, ParamNode p) {
-    exists(ParameterPosition ppos |
-      p.isParameterOf(viableCallableExt(call), ppos) and
-      parameterMatch(ppos, apos)
-    )
+  private predicate viableParam(DataFlowCall call, ParameterPosition ppos, ParamNode p) {
+    p.isParameterOf(viableCallableExt(call), ppos)
   }
 
   /**
@@ -407,9 +404,9 @@ private module Cached {
    */
   cached
   predicate viableParamArg(DataFlowCall call, ParamNode p, ArgNode arg) {
-    exists(ArgumentPosition pos |
-      viableParam(call, pos, p) and
-      arg.argumentOf(call, pos) and
+    exists(ParameterPosition ppos |
+      viableParam(call, ppos, p) and
+      argumentPositionMatch(call, arg, ppos) and
       compatibleTypes(getNodeDataFlowType(arg), getNodeDataFlowType(p))
     )
   }

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll

@@ -391,14 +391,11 @@ private module Cached {
 
   /**
    * Holds if `p` is the parameter of a viable dispatch target of `call`,
-   * and `p` matches arguments at position `apos`.
+   * and `p` has position `ppos`.
    */
   pragma[nomagic]
-  private predicate viableParam(DataFlowCall call, ArgumentPosition apos, ParamNode p) {
-    exists(ParameterPosition ppos |
-      p.isParameterOf(viableCallableExt(call), ppos) and
-      parameterMatch(ppos, apos)
-    )
+  private predicate viableParam(DataFlowCall call, ParameterPosition ppos, ParamNode p) {
+    p.isParameterOf(viableCallableExt(call), ppos)
   }
 
   /**
@@ -407,9 +404,9 @@ private module Cached {
    */
   cached
   predicate viableParamArg(DataFlowCall call, ParamNode p, ArgNode arg) {
-    exists(ArgumentPosition pos |
-      viableParam(call, pos, p) and
-      arg.argumentOf(call, pos) and
+    exists(ParameterPosition ppos |
+      viableParam(call, ppos, p) and
+      argumentPositionMatch(call, arg, ppos) and
       compatibleTypes(getNodeDataFlowType(arg), getNodeDataFlowType(p))
     )
   }

csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll

@@ -144,11 +144,13 @@ module Public {
     noComponentSpecificCsv(sc) and
     (
       exists(ArgumentPosition pos |
-        sc = TParameterSummaryComponent(pos) and result = "Parameter[" + pos + "]"
+        sc = TParameterSummaryComponent(pos) and
+        result = "Parameter[" + getArgumentPositionCsv(pos) + "]"
       )
       or
       exists(ParameterPosition pos |
-        sc = TArgumentSummaryComponent(pos) and result = "Argument[" + pos + "]"
+        sc = TArgumentSummaryComponent(pos) and
+        result = "Argument[" + getParameterPositionCsv(pos) + "]"
       )
       or
       sc = TReturnSummaryComponent(getReturnValueKind()) and result = "ReturnValue"

csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImplSpecific.qll

@@ -156,6 +156,12 @@ string getComponentSpecificCsv(SummaryComponent sc) {
   )
 }
 
+/** Gets the textual representation of a parameter position in the format used for flow summaries. */
+string getParameterPositionCsv(ParameterPosition pos) { result = pos.toString() }
+
+/** Gets the textual representation of an argument position in the format used for flow summaries. */
+string getArgumentPositionCsv(ArgumentPosition pos) { result = pos.toString() }
+
 class SourceOrSinkElement = Element;
 
 /** Gets the return kind corresponding to specification `"ReturnValue"`. */
@@ -222,18 +228,21 @@ predicate interpretInputSpecific(string c, InterpretNode mid, InterpretNode n) {
   )
 }
 
-/** Gets the argument position obtained by parsing `X` in `Parameter[X]`. */
 bindingset[s]
-ArgumentPosition parseParamBody(string s) {
-  result.getPosition() = s.regexpCapture("([-0-9]+)", 1).toInt()
+private int parsePosition(string s) {
+  result = s.regexpCapture("([-0-9]+)", 1).toInt()
   or
   exists(int n1, int n2 |
     s.regexpCapture("([-0-9]+)\\.\\.([0-9]+)", 1).toInt() = n1 and
     s.regexpCapture("([-0-9]+)\\.\\.([0-9]+)", 2).toInt() = n2 and
-    result.getPosition() in [n1 .. n2]
+    result in [n1 .. n2]
   )
 }
 
+/** Gets the argument position obtained by parsing `X` in `Parameter[X]`. */
+bindingset[s]
+ArgumentPosition parseParamBody(string s) { result.getPosition() = parsePosition(s) }
+
 /** Gets the parameter position obtained by parsing `X` in `Argument[X]`. */
 bindingset[s]
-ParameterPosition parseArgBody(string s) { result.getPosition() = parseParamBody(s).getPosition() }
+ParameterPosition parseArgBody(string s) { result.getPosition() = parsePosition(s) }

docs/ql-libraries/dataflow/dataflow.md

@@ -165,7 +165,7 @@ language-defined classes `ParameterPosition` and `ArgumentPosition`,
 so these three predicates must be defined:
 ```ql
 /** Holds if `p` is a `ParameterNode` of `c` with position `pos`. */
-predicate isParameterNode(ParameterNodeImpl p, DataFlowCallable c, ParameterPosition pos)
+predicate isParameterNode(ParameterNode p, DataFlowCallable c, ParameterPosition pos)
 
 /** Holds if `arg` is an `ArgumentNode` of `c` with position `pos`. */
 predicate isArgumentNode(ArgumentNode arg, DataFlowCall c, ArgumentPosition pos)

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll

@@ -391,14 +391,11 @@ private module Cached {
 
   /**
    * Holds if `p` is the parameter of a viable dispatch target of `call`,
-   * and `p` matches arguments at position `apos`.
+   * and `p` has position `ppos`.
    */
   pragma[nomagic]
-  private predicate viableParam(DataFlowCall call, ArgumentPosition apos, ParamNode p) {
-    exists(ParameterPosition ppos |
-      p.isParameterOf(viableCallableExt(call), ppos) and
-      parameterMatch(ppos, apos)
-    )
+  private predicate viableParam(DataFlowCall call, ParameterPosition ppos, ParamNode p) {
+    p.isParameterOf(viableCallableExt(call), ppos)
   }
 
   /**
@@ -407,9 +404,9 @@ private module Cached {
    */
   cached
   predicate viableParamArg(DataFlowCall call, ParamNode p, ArgNode arg) {
-    exists(ArgumentPosition pos |
-      viableParam(call, pos, p) and
-      arg.argumentOf(call, pos) and
+    exists(ParameterPosition ppos |
+      viableParam(call, ppos, p) and
+      argumentPositionMatch(call, arg, ppos) and
       compatibleTypes(getNodeDataFlowType(arg), getNodeDataFlowType(p))
     )
   }

java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll

@@ -144,11 +144,13 @@ module Public {
     noComponentSpecificCsv(sc) and
     (
       exists(ArgumentPosition pos |
-        sc = TParameterSummaryComponent(pos) and result = "Parameter[" + pos + "]"
+        sc = TParameterSummaryComponent(pos) and
+        result = "Parameter[" + getArgumentPositionCsv(pos) + "]"
       )
       or
       exists(ParameterPosition pos |
-        sc = TArgumentSummaryComponent(pos) and result = "Argument[" + pos + "]"
+        sc = TArgumentSummaryComponent(pos) and
+        result = "Argument[" + getParameterPositionCsv(pos) + "]"
       )
       or
       sc = TReturnSummaryComponent(getReturnValueKind()) and result = "ReturnValue"

java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImplSpecific.qll

@@ -96,6 +96,12 @@ string getComponentSpecificCsv(SummaryComponent sc) {
   exists(Content c | sc = TContentSummaryComponent(c) and result = getContentSpecificCsv(c))
 }
 
+/** Gets the textual representation of a parameter position in the format used for flow summaries. */
+string getParameterPositionCsv(ParameterPosition pos) { result = pos.toString() }
+
+/** Gets the textual representation of an argument position in the format used for flow summaries. */
+string getArgumentPositionCsv(ArgumentPosition pos) { result = pos.toString() }
+
 class SourceOrSinkElement = Top;
 
 /**
@@ -188,9 +194,8 @@ predicate interpretInputSpecific(string c, InterpretNode mid, InterpretNode n) {
   )
 }
 
-/** Gets the argument position obtained by parsing `X` in `Parameter[X]`. */
 bindingset[s]
-ArgumentPosition parseParamBody(string s) {
+private int parsePosition(string s) {
   result = s.regexpCapture("([-0-9]+)", 1).toInt()
   or
   exists(int n1, int n2 |
@@ -200,6 +205,10 @@ ArgumentPosition parseParamBody(string s) {
   )
 }
 
+/** Gets the argument position obtained by parsing `X` in `Parameter[X]`. */
+bindingset[s]
+ArgumentPosition parseParamBody(string s) { result = parsePosition(s) }
+
 /** Gets the parameter position obtained by parsing `X` in `Argument[X]`. */
 bindingset[s]
-ParameterPosition parseArgBody(string s) { result = parseParamBody(s) }
+ParameterPosition parseArgBody(string s) { result = parsePosition(s) }

python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplCommon.qll

@@ -391,14 +391,11 @@ private module Cached {
 
   /**
    * Holds if `p` is the parameter of a viable dispatch target of `call`,
-   * and `p` matches arguments at position `apos`.
+   * and `p` has position `ppos`.
    */
   pragma[nomagic]
-  private predicate viableParam(DataFlowCall call, ArgumentPosition apos, ParamNode p) {
-    exists(ParameterPosition ppos |
-      p.isParameterOf(viableCallableExt(call), ppos) and
-      parameterMatch(ppos, apos)
-    )
+  private predicate viableParam(DataFlowCall call, ParameterPosition ppos, ParamNode p) {
+    p.isParameterOf(viableCallableExt(call), ppos)
   }
 
   /**
@@ -407,9 +404,9 @@ private module Cached {
    */
   cached
   predicate viableParamArg(DataFlowCall call, ParamNode p, ArgNode arg) {
-    exists(ArgumentPosition pos |
-      viableParam(call, pos, p) and
-      arg.argumentOf(call, pos) and
+    exists(ParameterPosition ppos |
+      viableParam(call, ppos, p) and
+      argumentPositionMatch(call, arg, ppos) and
       compatibleTypes(getNodeDataFlowType(arg), getNodeDataFlowType(p))
     )
   }

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplCommon.qll

@@ -391,14 +391,11 @@ private module Cached {
 
   /**
    * Holds if `p` is the parameter of a viable dispatch target of `call`,
-   * and `p` matches arguments at position `apos`.
+   * and `p` has position `ppos`.
    */
   pragma[nomagic]
-  private predicate viableParam(DataFlowCall call, ArgumentPosition apos, ParamNode p) {
-    exists(ParameterPosition ppos |
-      p.isParameterOf(viableCallableExt(call), ppos) and
-      parameterMatch(ppos, apos)
-    )
+  private predicate viableParam(DataFlowCall call, ParameterPosition ppos, ParamNode p) {
+    p.isParameterOf(viableCallableExt(call), ppos)
   }
 
   /**
@@ -407,9 +404,9 @@ private module Cached {
    */
   cached
   predicate viableParamArg(DataFlowCall call, ParamNode p, ArgNode arg) {
-    exists(ArgumentPosition pos |
-      viableParam(call, pos, p) and
-      arg.argumentOf(call, pos) and
+    exists(ParameterPosition ppos |
+      viableParam(call, ppos, p) and
+      argumentPositionMatch(call, arg, ppos) and
       compatibleTypes(getNodeDataFlowType(arg), getNodeDataFlowType(p))
     )
   }

ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImpl.qll

@@ -144,11 +144,13 @@ module Public {
     noComponentSpecificCsv(sc) and
     (
       exists(ArgumentPosition pos |
-        sc = TParameterSummaryComponent(pos) and result = "Parameter[" + pos + "]"
+        sc = TParameterSummaryComponent(pos) and
+        result = "Parameter[" + getArgumentPositionCsv(pos) + "]"
       )
       or
       exists(ParameterPosition pos |
-        sc = TArgumentSummaryComponent(pos) and result = "Argument[" + pos + "]"
+        sc = TArgumentSummaryComponent(pos) and
+        result = "Argument[" + getParameterPositionCsv(pos) + "]"
       )
       or
       sc = TReturnSummaryComponent(getReturnValueKind()) and result = "ReturnValue"

ruby/ql/lib/codeql/ruby/dataflow/internal/FlowSummaryImplSpecific.qll

@@ -71,6 +71,12 @@ string getComponentSpecificCsv(SummaryComponent sc) {
   sc = TArgumentSummaryComponent(-2) and result = "BlockArgument"
 }
 
+/** Gets the textual representation of a parameter position in the format used for flow summaries. */
+string getParameterPositionCsv(ParameterPosition pos) { result = pos.toString() }
+
+/** Gets the textual representation of an argument position in the format used for flow summaries. */
+string getArgumentPositionCsv(ArgumentPosition pos) { result = pos.toString() }
+
 /** Gets the return kind corresponding to specification `"ReturnValue"`. */
 NormalReturnKind getReturnValueKind() { any() }
 
@@ -121,9 +127,8 @@ private module UnusedSourceSinkInterpretation {
 
 import UnusedSourceSinkInterpretation
 
-/** Gets the argument position obtained by parsing `X` in `Parameter[X]`. */
 bindingset[s]
-ArgumentPosition parseParamBody(string s) {
+private int parsePosition(string s) {
   result = s.regexpCapture("([-0-9]+)", 1).toInt()
   or
   exists(int n1, int n2 |
@@ -133,6 +138,10 @@ ArgumentPosition parseParamBody(string s) {
   )
 }
 
+/** Gets the argument position obtained by parsing `X` in `Parameter[X]`. */
+bindingset[s]
+ArgumentPosition parseParamBody(string s) { result = parsePosition(s) }
+
 /** Gets the parameter position obtained by parsing `X` in `Argument[X]`. */
 bindingset[s]
-ParameterPosition parseArgBody(string s) { result = parseParamBody(s) }
+ParameterPosition parseArgBody(string s) { result = parsePosition(s) }