Skip to content

Commit d93dce0

Browse files
tamasvajktamasvajk
committed
Kotlin: Fix extension and dispatch parameter order in $default functions
1 parent 6457e05 commit d93dce0

File tree

8 files changed

+71
-50
lines changed

8 files changed

+71
-50
lines changed

‎java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -957,7 +957,7 @@ open class KotlinFileExtractor(
957957
val locId = getLocation(f, null)
958958
val extReceiver = f.extensionReceiverParameter
959959
val dispatchReceiver = if (f.shouldExtractAsStatic) null else f.dispatchReceiverParameter
960-
val parameterTypes = listOfNotNull(extReceiver?.let { erase(it.type) }) + getDefaultsMethodArgTypes(f)
960+
val parameterTypes = getDefaultsMethodArgTypes(f)
961961
val allParamTypeResults = parameterTypes.mapIndexed { i, paramType ->
962962
val paramId = tw.getLabelFor<DbParam>(getValueParameterLabel(id, i))
963963
extractValueParameter(paramId, paramType, "p$i", locId, id, i, paramId, isVararg = false, syntheticParameterNames = true, isCrossinline = false, isNoinline = false).also {
@@ -976,7 +976,8 @@ open class KotlinFileExtractor(
976976
extractMethod(methodId, locId, shortName, erase(f.returnType), paramsSignature, parentId, methodId, origin = null, extractTypeAccess = extractMethodAndParameterTypeAccesses)
977977
addModifiers(id, "static")
978978
if (extReceiver != null) {
979-
val extendedType = allParamTypeResults[0] // TODO: this is not correct for member extension methods, where the dispatch receiver is the first parameter
979+
val idx = if (dispatchReceiver != null) 1 else 0
980+
val extendedType = allParamTypeResults[idx]
980981
tw.writeKtExtensionFunctions(methodId, extendedType.javaResult.id, extendedType.kotlinResult.id)
981982
}
982983
}
@@ -1044,8 +1045,8 @@ open class KotlinFileExtractor(
10441045
val realFnIdxOffset = if (f.extensionReceiverParameter != null) 1 else 0
10451046
val paramMappings = f.valueParameters.mapIndexed { idx, param -> Triple(param.type, idx + paramIdxOffset, idx + realFnIdxOffset) } +
10461047
listOfNotNull(
1047-
dispatchReceiver?.let { Triple(it.type, realFnIdxOffset, -1) },
1048-
extReceiver?.let { Triple(it.type, 0, 0) }
1048+
dispatchReceiver?.let { Triple(it.type, 0, -1) },
1049+
extReceiver?.let { Triple(it.type, if (dispatchReceiver != null) 1 else 0, 0) }
10491050
)
10501051
paramMappings.forEach { (type, fromIdx, toIdx) ->
10511052
extractVariableAccess(tw.getLabelFor<DbParam>(getValueParameterLabel(id, fromIdx)), type, locId, thisCallId, toIdx, id, returnId)
@@ -1188,6 +1189,7 @@ open class KotlinFileExtractor(
11881189
id
11891190

11901191
val extReceiver = f.extensionReceiverParameter
1192+
// The following parameter order is correct, because member $default methods (where the order would be [dispatchParam], [extensionParam], normalParams) are not extracted here
11911193
val fParameters = listOfNotNull(extReceiver) + (overriddenAttributes?.valueParameters ?: f.valueParameters)
11921194
val paramTypes = fParameters.mapIndexed { i, vp ->
11931195
extractValueParameter(vp, id, i, typeSubstitution, sourceDeclaration, classTypeArgsIncludingOuterClasses, extractTypeAccess = extractMethodAndParameterTypeAccesses, overriddenAttributes?.sourceLoc)
@@ -1797,11 +1799,12 @@ open class KotlinFileExtractor(
17971799
) ?: pluginContext.irBuiltIns.anyType
17981800

17991801
private fun getDefaultsMethodArgTypes(f: IrFunction) =
1800-
// The $default method has type ([extensionReceiver], [dispatchReceiver], paramTypes..., int, Object)
1802+
// The $default method has type ([dispatchReceiver], [extensionReceiver], paramTypes..., int, Object)
18011803
// All parameter types are erased. The trailing int is a mask indicating which parameter values are real
18021804
// and which should be replaced by defaults. The final Object parameter is apparently always null.
18031805
(
18041806
listOfNotNull(if (f.shouldExtractAsStatic) null else f.dispatchReceiverParameter?.type) +
1807+
listOfNotNull(f.extensionReceiverParameter?.type) +
18051808
f.valueParameters.map { it.type } +
18061809
listOf(pluginContext.irBuiltIns.intType, getDefaultsMethodLastArgType(f))
18071810
).map { erase(it) }
@@ -1820,17 +1823,16 @@ open class KotlinFileExtractor(
18201823

18211824
private fun getDefaultsMethodLabel(f: IrFunction): Label<out DbCallable> {
18221825
val defaultsMethodName = if (f is IrConstructor) "<init>" else getDefaultsMethodName(f)
1823-
val normalArgTypes = getDefaultsMethodArgTypes(f)
1824-
val extensionParamType = f.extensionReceiverParameter?.let { erase(it.type) }
1826+
val argTypes = getDefaultsMethodArgTypes(f)
18251827

18261828
val defaultMethodLabelStr = getFunctionLabel(
18271829
f.parent,
18281830
maybeParentId = null,
18291831
defaultsMethodName,
1830-
normalArgTypes,
1832+
argTypes,
18311833
erase(f.returnType),
1832-
extensionParamType,
1833-
listOf(),
1834+
extensionParamType = null, // if there's any, that's included already in argTypes
1835+
functionTypeParameters = listOf(),
18341836
classTypeArgsIncludingOuterClasses = null,
18351837
overridesCollectionsMethod = false,
18361838
javaSignature = null,
@@ -1890,13 +1892,14 @@ open class KotlinFileExtractor(
18901892
extensionReceiver: IrExpression?
18911893
) {
18921894
var nextIdx = 0
1893-
if (extensionReceiver != null) {
1894-
extractExpressionExpr(extensionReceiver, enclosingCallable, id, nextIdx++, enclosingStmt)
1895-
}
18961895
if (dispatchReceiver != null && !callTarget.shouldExtractAsStatic) {
18971896
extractExpressionExpr(dispatchReceiver, enclosingCallable, id, nextIdx++, enclosingStmt)
18981897
}
18991898

1899+
if (extensionReceiver != null) {
1900+
extractExpressionExpr(extensionReceiver, enclosingCallable, id, nextIdx++, enclosingStmt)
1901+
}
1902+
19001903
val valueArgsWithDummies = valueArguments.zip(callTarget.valueParameters).map {
19011904
(expr, param) -> expr ?: IrConstImpl.defaultValueForType(0, 0, param.type)
19021905
}
@@ -4050,8 +4053,7 @@ open class KotlinFileExtractor(
40504053
// Use of 'this' in a function where the dispatch receiver is passed like an ordinary parameter,
40514054
// such as a `$default` static function that substitutes in default arguments as needed.
40524055
val paramDeclarerId = overriddenAttributes.id ?: useDeclarationParent(thisParamParent, false)
4053-
val extensionParamOffset = if (thisParamParent.extensionReceiverParameter != null) 1 else 0
4054-
val replacementParamId = tw.getLabelFor<DbParam>(getValueParameterLabel(paramDeclarerId, replaceWithParamIdx + extensionParamOffset))
4056+
val replacementParamId = tw.getLabelFor<DbParam>(getValueParameterLabel(paramDeclarerId, replaceWithParamIdx))
40554057
extractVariableAccess(replacementParamId, e.type, locId, exprParent.parent, exprParent.idx, callable, exprParent.enclosingStmt)
40564058
return
40574059
}

‎java/ql/lib/semmle/code/java/Expr.qll‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1896,7 +1896,8 @@ class VarAccess extends Expr, @varaccess {
18961896
class ExtensionReceiverAccess extends VarAccess {
18971897
ExtensionReceiverAccess() {
18981898
exists(Parameter p |
1899-
this.getVariable() = p and p.getPosition() = 0 and p.getCallable() instanceof ExtensionMethod
1899+
this.getVariable() = p and
1900+
p.isExtensionParameter()
19001901
)
19011902
}
19021903

‎java/ql/lib/semmle/code/java/Member.qll‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -327,18 +327,8 @@ class Callable extends StmtParent, Member, @callable {
327327
this instanceof Method and
328328
result instanceof Method and
329329
this.getName() + "$default" = result.getName() and
330-
extraLeadingParams <= 1 and
331-
(
332-
if ktExtensionFunctions(this, _, _)
333-
then
334-
// Both extension receivers are expected to occur at arg0, with any
335-
// dispatch receiver inserted afterwards in the $default proxy's parameter list.
336-
// Check the extension receiver matches here, and note regular args
337-
// are bumped one position to the right.
338-
regularParamsStartIdx = extraLeadingParams + 1 and
339-
this.getParameterType(0).getErasure() = eraseRaw(result.getParameterType(0))
340-
else regularParamsStartIdx = extraLeadingParams
341-
) and
330+
extraLeadingParams <= 1 and // 0 for static methods, 1 for instance methods
331+
regularParamsStartIdx = extraLeadingParams and
342332
lastParamType instanceof TypeObject
343333
)
344334
|
@@ -824,4 +814,19 @@ class ExtensionMethod extends Method {
824814
KotlinType getExtendedKotlinType() { result = extendedKotlinType }
825815

826816
override string getAPrimaryQlClass() { result = "ExtensionMethod" }
817+
818+
/**
819+
* Gets the index of the parameter that is the extension receiver. This is typically index 0. In case of `$default`
820+
* extension methods that are defined as members, the index is 1. Index 0 is the dispatch receiver of the `$default`
821+
* method.
822+
*/
823+
int getExtensionParameterIndex() {
824+
if
825+
exists(Method src |
826+
this = src.getKotlinParameterDefaultsProxy() and
827+
src.getNumberOfParameters() = this.getNumberOfParameters() - 3 // 2 extra parameters + 1 dispatch receiver
828+
)
829+
then result = 1
830+
else result = 0
831+
}
827832
}

‎java/ql/lib/semmle/code/java/Variable.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ class Parameter extends Element, @param, LocalScopeVariable {
9191

9292
/** Holds if this formal parameter is a parameter representing the dispatch receiver in an extension method. */
9393
predicate isExtensionParameter() {
94-
this.getPosition() = 0 and this.getCallable() instanceof ExtensionMethod
94+
this.getPosition() = this.getCallable().(ExtensionMethod).getExtensionParameterIndex()
9595
}
9696

9797
/**

‎java/ql/test/kotlin/library-tests/methods/exprs.expected‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@
326326
| methods3.kt:6:5:6:45 | fooBarMethodExt(...) | MethodAccess |
327327
| methods3.kt:6:5:6:45 | int | TypeAccess |
328328
| methods3.kt:6:5:6:45 | int | TypeAccess |
329-
| methods3.kt:6:5:6:45 | p1 | VarAccess |
329+
| methods3.kt:6:5:6:45 | p0 | VarAccess |
330330
| methods3.kt:6:5:6:45 | p2 | VarAccess |
331331
| methods3.kt:6:5:6:45 | p2 | VarAccess |
332332
| methods3.kt:6:5:6:45 | p3 | VarAccess |

‎java/ql/test/kotlin/library-tests/methods/methods.expected‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ methods
4141
| methods3.kt:0:0:0:0 | Methods3Kt | methods3.kt:3:1:3:49 | fooBarTopLevelMethodExt | fooBarTopLevelMethodExt(java.lang.String,int) | final, public, static | |
4242
| methods3.kt:0:0:0:0 | Methods3Kt | methods3.kt:3:1:3:49 | fooBarTopLevelMethodExt$default | fooBarTopLevelMethodExt$default(java.lang.String,int,int,java.lang.Object) | public, static | Compiler generated |
4343
| methods3.kt:5:1:7:1 | Class3 | methods3.kt:6:5:6:45 | fooBarMethodExt | fooBarMethodExt(java.lang.String,int) | final, public | |
44-
| methods3.kt:5:1:7:1 | Class3 | methods3.kt:6:5:6:45 | fooBarMethodExt$default | fooBarMethodExt$default(java.lang.String,foo.bar.Class3,int,int,java.lang.Object) | public, static | Compiler generated |
44+
| methods3.kt:5:1:7:1 | Class3 | methods3.kt:6:5:6:45 | fooBarMethodExt$default | fooBarMethodExt$default(foo.bar.Class3,java.lang.String,int,int,java.lang.Object) | public, static | Compiler generated |
4545
| methods4.kt:5:3:9:3 | InsideNestedTest | methods4.kt:7:5:7:34 | m | m(foo.bar.NestedTest.InsideNestedTest) | final, public | |
4646
| methods5.kt:0:0:0:0 | Methods5Kt | methods5.kt:3:1:11:1 | x | x() | final, public, static | |
4747
| methods5.kt:5:3:5:27 | | methods5.kt:5:3:5:27 | a | a(int) | final, public | |
@@ -82,3 +82,9 @@ extensions
8282
| methods3.kt:6:5:6:45 | fooBarMethodExt$default | file:///modules/java.base/java/lang/String.class:0:0:0:0 | String |
8383
| methods5.kt:9:3:9:32 | f1 | file:///!unknown-binary-location/foo/bar/C1.class:0:0:0:0 | C1<T1> |
8484
extensionsMismatch
85+
extensionIndex
86+
| methods3.kt:3:1:3:49 | fooBarTopLevelMethodExt | 0 | file:///modules/java.base/java/lang/String.class:0:0:0:0 | String |
87+
| methods3.kt:3:1:3:49 | fooBarTopLevelMethodExt$default | 0 | file:///modules/java.base/java/lang/String.class:0:0:0:0 | String |
88+
| methods3.kt:6:5:6:45 | fooBarMethodExt | 0 | file:///modules/java.base/java/lang/String.class:0:0:0:0 | String |
89+
| methods3.kt:6:5:6:45 | fooBarMethodExt$default | 1 | file:///modules/java.base/java/lang/String.class:0:0:0:0 | String |
90+
| methods5.kt:9:3:9:32 | f1 | 0 | file:///!unknown-binary-location/foo/bar/C1.class:0:0:0:0 | C1<T1> |

‎java/ql/test/kotlin/library-tests/methods/methods.ql‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,10 @@ query predicate extensionsMismatch(Method src, Method def) {
2626
def instanceof ExtensionMethod and not src instanceof ExtensionMethod
2727
)
2828
}
29+
30+
query predicate extensionIndex(ExtensionMethod m, int i, Type t) {
31+
m.fromSource() and
32+
m.getExtensionParameterIndex() = i and
33+
m.getExtendedType() = t and
34+
m.getParameter(i).getType() = t
35+
}

‎java/ql/test/kotlin/library-tests/parameter-defaults/PrintAst.expected‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -525,13 +525,13 @@ test.kt:
525525
# 21| 0: [MethodAccess] sink(...)
526526
# 21| -1: [TypeAccess] TestKt
527527
# 21| 0: [VarAccess] y
528-
# 19| 3: [Method] f$default
528+
# 19| 3: [ExtensionMethod] f$default
529529
# 19| 3: [TypeAccess] Unit
530530
#-----| 4: (Parameters)
531531
# 19| 0: [Parameter] p0
532-
# 19| 0: [TypeAccess] String
533-
# 19| 1: [Parameter] p1
534532
# 19| 0: [TypeAccess] TestExtensionMember
533+
# 19| 1: [Parameter] p1
534+
# 19| 0: [TypeAccess] String
535535
# 19| 2: [Parameter] p2
536536
# 19| 0: [TypeAccess] String
537537
# 19| 3: [Parameter] p3
@@ -565,8 +565,8 @@ test.kt:
565565
# 19| 1: [StringLiteral] "hello world"
566566
# 19| 2: [ReturnStmt] return ...
567567
# 19| 0: [MethodAccess] f(...)
568-
# 19| -1: [VarAccess] p1
569-
# 19| 0: [VarAccess] p0
568+
# 19| -1: [VarAccess] p0
569+
# 19| 0: [ExtensionReceiverAccess] this
570570
# 19| 1: [VarAccess] p2
571571
# 19| 2: [VarAccess] p3
572572
# 19| 3: [VarAccess] p4
@@ -579,8 +579,8 @@ test.kt:
579579
# 25| 0: [ExprStmt] <Expr>;
580580
# 25| 0: [MethodAccess] f$default(...)
581581
# 25| -1: [TypeAccess] TestExtensionMember
582-
# 25| 0: [VarAccess] sunk
583-
# 25| 1: [ThisAccess] this
582+
# 25| 0: [ThisAccess] this
583+
# 25| 1: [VarAccess] sunk
584584
# 25| 2: [StringLiteral] "extension sunk"
585585
# 1| 3: [NullLiteral] null
586586
# 1| 4: [NullLiteral] null
@@ -589,8 +589,8 @@ test.kt:
589589
# 26| 1: [ExprStmt] <Expr>;
590590
# 26| 0: [MethodAccess] f$default(...)
591591
# 26| -1: [TypeAccess] TestExtensionMember
592-
# 26| 0: [VarAccess] sunk
593-
# 26| 1: [ThisAccess] this
592+
# 26| 0: [ThisAccess] this
593+
# 26| 1: [VarAccess] sunk
594594
# 26| 2: [StringLiteral] "extension sunk fp"
595595
# 26| 3: [StringLiteral] "extension sunk 2"
596596
# 1| 4: [NullLiteral] null
@@ -729,13 +729,13 @@ test.kt:
729729
# 57| 0: [MethodAccess] sink(...)
730730
# 57| -1: [TypeAccess] TestKt
731731
# 57| 0: [VarAccess] y
732-
# 56| 4: [Method] test$default
732+
# 56| 4: [ExtensionMethod] test$default
733733
# 56| 3: [TypeAccess] Unit
734734
#-----| 4: (Parameters)
735735
# 56| 0: [Parameter] p0
736-
# 56| 0: [TypeAccess] ExtendMe
737-
# 56| 1: [Parameter] p1
738736
# 56| 0: [TypeAccess] TestReceiverReferences
737+
# 56| 1: [Parameter] p1
738+
# 56| 0: [TypeAccess] ExtendMe
739739
# 56| 2: [Parameter] p2
740740
# 56| 0: [TypeAccess] String
741741
# 56| 3: [Parameter] p3
@@ -759,7 +759,7 @@ test.kt:
759759
# 56| 1: [MethodAccess] f(...)
760760
# 56| -1: [VarAccess] p0
761761
# 56| 0: [MethodAccess] g(...)
762-
# 56| -1: [VarAccess] p1
762+
# 56| -1: [VarAccess] p0
763763
# 56| 0: [VarAccess] p2
764764
# 56| 1: [IfStmt] if (...)
765765
# 56| 0: [EQExpr] ... == ...
@@ -773,8 +773,8 @@ test.kt:
773773
# 56| 1: [StringLiteral] "hello world"
774774
# 56| 2: [ReturnStmt] return ...
775775
# 56| 0: [MethodAccess] test(...)
776-
# 56| -1: [VarAccess] p1
777-
# 56| 0: [VarAccess] p0
776+
# 56| -1: [VarAccess] p0
777+
# 56| 0: [ExtensionReceiverAccess] this
778778
# 56| 1: [VarAccess] p2
779779
# 56| 2: [VarAccess] p3
780780
# 56| 3: [VarAccess] p4
@@ -787,8 +787,8 @@ test.kt:
787787
# 61| 0: [ExprStmt] <Expr>;
788788
# 61| 0: [MethodAccess] test$default(...)
789789
# 61| -1: [TypeAccess] TestReceiverReferences
790-
# 61| 0: [VarAccess] t
791-
# 61| 1: [ThisAccess] this
790+
# 61| 0: [ThisAccess] this
791+
# 61| 1: [VarAccess] t
792792
# 61| 2: [StringLiteral] "receiver refs sunk"
793793
# 1| 3: [NullLiteral] null
794794
# 1| 4: [NullLiteral] null
@@ -797,8 +797,8 @@ test.kt:
797797
# 62| 1: [ExprStmt] <Expr>;
798798
# 62| 0: [MethodAccess] test$default(...)
799799
# 62| -1: [TypeAccess] TestReceiverReferences
800-
# 62| 0: [VarAccess] t
801-
# 62| 1: [ThisAccess] this
800+
# 62| 0: [ThisAccess] this
801+
# 62| 1: [VarAccess] t
802802
# 62| 2: [StringLiteral] "receiver refs sunk fp"
803803
# 62| 3: [StringLiteral] "receiver refs sunk 2"
804804
# 1| 4: [NullLiteral] null

0 commit comments

Comments
 (0)