Skip to content

Commit 22010f6

Browse files
sjrdsjrd
committed
Fix #9462: Identify inserted apply's more reliably in the Dynamic treatment.
Previously, identifying when there was an inserted apply, which had to be carried through the Dynamic treatment, was done on a syntactical basis. This would mishandle cases written as someContainer.someDynamic() as it would think that this was an explicit dynamic call for `"someDynamic"` on `someContainer` (although `someContainer` is not even a `scala.Dynamic` itself, but `someContainer.someDynamic` is). We now reliably detect inserted applys by looking at the result of the pre-type-checking of the function part, and testing whether there is a synthetic `Select(_, nme.apply)` node.
1 parent d5efc05 commit 22010f6

File tree

4 files changed

+99
-22
lines changed

4 files changed

+99
-22
lines changed

‎compiler/src/dotty/tools/dotc/typer/Applications.scala‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,23 @@ trait Applications extends Compatibility {
913913

914914
fun1.tpe match {
915915
case err: ErrorType => cpy.Apply(tree)(fun1, proto.typedArgs()).withType(err)
916-
case TryDynamicCallType => typedDynamicApply(tree, pt)
916+
case TryDynamicCallType =>
917+
val isInsertedApply = fun1 match {
918+
case Select(_, nme.apply) => fun1.span.isSynthetic
919+
case TypeApply(sel @ Select(_, nme.apply), _) => sel.span.isSynthetic
920+
/* TODO Get rid of this case. It is still syntax-based, therefore unreliable.
921+
* It is necessary for things like `someDynamic[T](...)`, because in that case,
922+
* somehow typedFunPart returns a tree that was typed as `TryDynamicCallType`,
923+
* so clearly with the view that an apply insertion was necessary, but doesn't
924+
* actually insert the apply!
925+
* This is probably something wrong in apply insertion, but I (@sjrd) am out of
926+
* my depth there.
927+
* In the meantime, this makes tests pass.
928+
*/
929+
case TypeApply(fun, _) => !fun.isInstanceOf[Select]
930+
case _ => false
931+
}
932+
typedDynamicApply(tree, isInsertedApply, pt)
917933
case _ =>
918934
if (originalProto.isDropped) fun1
919935
else if (fun1.symbol == defn.Compiletime_summonFrom)

‎compiler/src/dotty/tools/dotc/typer/Dynamic.scala‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ trait Dynamic {
6161
* foo.bar(x = bazX, y = bazY, baz, ...) ~~> foo.applyDynamicNamed("bar")(("x", bazX), ("y", bazY), ("", baz), ...)
6262
* foo.bar[T0, ...](x = bazX, y = bazY, baz, ...) ~~> foo.applyDynamicNamed[T0, ...]("bar")(("x", bazX), ("y", bazY), ("", baz), ...)
6363
*/
64-
def typedDynamicApply(tree: untpd.Apply, pt: Type)(using Context): Tree = {
64+
def typedDynamicApply(tree: untpd.Apply, isInsertedApply: Boolean, pt: Type)(using Context): Tree = {
6565
def typedDynamicApply(qual: untpd.Tree, name: Name, selSpan: Span, targs: List[untpd.Tree]): Tree = {
6666
def isNamedArg(arg: untpd.Tree): Boolean = arg match { case NamedArg(_, _) => true; case _ => false }
6767
val args = tree.args
@@ -79,15 +79,22 @@ trait Dynamic {
7979
}
8080
}
8181

82-
tree.fun match {
83-
case sel @ Select(qual, name) if !isDynamicMethod(name) =>
84-
typedDynamicApply(qual, name, sel.span, Nil)
85-
case TypeApply(sel @ Select(qual, name), targs) if !isDynamicMethod(name) =>
86-
typedDynamicApply(qual, name, sel.span, targs)
87-
case TypeApply(fun, targs) =>
88-
typedDynamicApply(fun, nme.apply, fun.span, targs)
89-
case fun =>
90-
typedDynamicApply(fun, nme.apply, fun.span, Nil)
82+
if (isInsertedApply) {
83+
tree.fun match {
84+
case TypeApply(fun, targs) =>
85+
typedDynamicApply(fun, nme.apply, fun.span, targs)
86+
case fun =>
87+
typedDynamicApply(fun, nme.apply, fun.span, Nil)
88+
}
89+
} else {
90+
tree.fun match {
91+
case sel @ Select(qual, name) if !isDynamicMethod(name) =>
92+
typedDynamicApply(qual, name, sel.span, Nil)
93+
case TypeApply(sel @ Select(qual, name), targs) if !isDynamicMethod(name) =>
94+
typedDynamicApply(qual, name, sel.span, targs)
95+
case _ =>
96+
errorTree(tree, em"Dynamic insertion not applicable")
97+
}
9198
}
9299
}
93100

‎project/Build.scala‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,7 +1068,7 @@ object Build {
10681068
-- "InteroperabilityTest.scala" // various compile errors
10691069
-- "OptimizerTest.scala" // compile errors: false + string and () + string
10701070
-- "ReflectionTest.scala" // tests fail
1071-
-- "RegressionJSTest.scala" // compile error with js.Dynamic.literal
1071+
-- "RegressionJSTest.scala" // non-native JS classes
10721072
-- "RuntimeTypesTest.scala" // compile errors: no ClassTag for Null and Nothing
10731073
)).get
10741074

@@ -1081,23 +1081,22 @@ object Build {
10811081

10821082
++ (dir / "js/src/test/scala/org/scalajs/testsuite/jsinterop" ** (("*.scala": FileFilter)
10831083
-- "AsyncTest.scala" // needs PromiseMock.scala
1084-
-- "DynamicTest.scala" // compile error with js.Dynamic.literal
1084+
-- "DynamicTest.scala" // one test requires JS exports, all other tests pass
10851085
-- "ExportsTest.scala" // JS exports
10861086
-- "FunctionTest.scala" // IR checking errors
10871087
-- "IterableTest.scala" // non-native JS classes
10881088
-- "JSExportStaticTest.scala" // JS exports
1089-
-- "JSNameTest.scala" // compile error with js.Dynamic.literal
10901089
-- "JSNativeInPackage.scala" // IR checking errors
10911090
-- "JSOptionalTest.scala" // non-native JS classes
1092-
-- "JSSymbolTest.scala" // compile error with js.Dynamic.literal
1093-
-- "MiscInteropTest.scala" // compile error with js.Dynamic.literal
1091+
-- "JSSymbolTest.scala" // non-native JS classes
1092+
-- "MiscInteropTest.scala" // non-native JS classes
10941093
-- "ModulesWithGlobalFallbackTest.scala" // non-native JS classes
10951094
-- "NestedJSClassTest.scala" // non-native JS classes
10961095
-- "NonNativeJSTypeTest.scala" // non-native JS classes
10971096
-- "PromiseMock.scala" // non-native JS classes
1098-
-- "SpecialTest.scala" // compile error with js.Dynamic.literal
1097+
-- "SpecialTest.scala" // assertion error in ExpandSAMs
10991098
-- "SymbolTest.scala" // IR checking errors
1100-
-- "ThisFunctionTest.scala" // compile error with js.Dynamic.literal
1099+
-- "ThisFunctionTest.scala" // assertion error in ExpandSAMs
11011100
-- "UndefOrTest.scala" // StackOverflow in the compiler
11021101
)).get
11031102

@@ -1118,10 +1117,7 @@ object Build {
11181117
)).get
11191118

11201119
++ (dir / "js/src/test/scala/org/scalajs/testsuite/niobuffer" ** "*.scala").get
1121-
1122-
++ (dir / "js/src/test/scala/org/scalajs/testsuite/scalalib" ** (("*.scala": FileFilter)
1123-
-- "ScalaRunTimeJSTest.scala" // compile error with js.Dynamic.literal
1124-
)).get
1120+
++ (dir / "js/src/test/scala/org/scalajs/testsuite/scalalib" ** "*.scala").get
11251121

11261122
++ (dir / "js/src/test/scala/org/scalajs/testsuite/typedarray" ** (("*.scala": FileFilter)
11271123
-- "TypedArrayTest.scala" // assertion error in ExpandSAMs

‎tests/run/dynamicDynamicTests.scala‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ object Test {
4949
def main(args: Array[String]): Unit = {
5050
runFooTests1()
5151
runFooTests2()
52+
runFooTests3()
5253
runBarTests()
5354
runBazTests()
5455
runQuxTests()
@@ -102,6 +103,63 @@ object Test {
102103
assertEquals("selectDynamic(bazSelectUpdate).update(key, value)", foo.bazSelectUpdate("key") = "value")
103104
}
104105

106+
/** Test apply insertions kick in before dynamic calls. */
107+
def runFooTests3() = {
108+
val foo = new Foo
109+
110+
assertEquals("applyDynamic(apply)()", foo())
111+
assertEquals("applyDynamic(apply)(1)", foo(1))
112+
assertEquals("applyDynamic(apply)(1, 2, 3)", foo(1, 2, 3))
113+
assertEquals("applyDynamic(apply)(1, 2, a)", foo(1, 2, "a"))
114+
assertEquals("applyDynamic(apply)(1, 2, a)", foo(List(1, 2, "a"): _*))
115+
116+
assertEquals("applyDynamicNamed(apply)((a,1))", foo(a = 1))
117+
assertEquals("applyDynamicNamed(apply)((a,1), (b,2))", foo(a = 1, b = 2))
118+
assertEquals("applyDynamicNamed(apply)((a,1), (,0))", foo(a = 1, 0))
119+
assertEquals("applyDynamicNamed(apply)((a,1), (a,5))", foo(a = 1, a = 5))
120+
assertEquals("applyDynamicNamed(apply)((,d), (a,1), (,5), (a,c))", foo("d", a = 1, 5, a = 'c'))
121+
122+
assertEquals("applyDynamic(apply)()", foo.apply())
123+
assertEquals("applyDynamic(apply)(1)", foo.apply(1))
124+
assertEquals("applyDynamic(apply)(1, 2, 3)", foo.apply(1, 2, 3))
125+
assertEquals("applyDynamic(apply)(1, 2, a)", foo.apply(1, 2, "a"))
126+
assertEquals("applyDynamic(apply)(1, 2, a)", foo.apply(List(1, 2, "a"): _*))
127+
128+
assertEquals("applyDynamicNamed(apply)((a,1))", foo.apply(a = 1))
129+
assertEquals("applyDynamicNamed(apply)((a,1), (b,2))", foo.apply(a = 1, b = 2))
130+
assertEquals("applyDynamicNamed(apply)((a,1), (,0))", foo.apply(a = 1, 0))
131+
assertEquals("applyDynamicNamed(apply)((a,1), (a,5))", foo.apply(a = 1, a = 5))
132+
assertEquals("applyDynamicNamed(apply)((,d), (a,1), (,5), (a,c))", foo.apply("d", a = 1, 5, a = 'c'))
133+
134+
object bar {
135+
val foo: Foo = new Foo
136+
}
137+
138+
assertEquals("applyDynamic(apply)()", bar.foo())
139+
assertEquals("applyDynamic(apply)(1)", bar.foo(1))
140+
assertEquals("applyDynamic(apply)(1, 2, 3)", bar.foo(1, 2, 3))
141+
assertEquals("applyDynamic(apply)(1, 2, a)", bar.foo(1, 2, "a"))
142+
assertEquals("applyDynamic(apply)(1, 2, a)", bar.foo(List(1, 2, "a"): _*))
143+
144+
assertEquals("applyDynamicNamed(apply)((a,1))", bar.foo(a = 1))
145+
assertEquals("applyDynamicNamed(apply)((a,1), (b,2))", bar.foo(a = 1, b = 2))
146+
assertEquals("applyDynamicNamed(apply)((a,1), (,0))", bar.foo(a = 1, 0))
147+
assertEquals("applyDynamicNamed(apply)((a,1), (a,5))", bar.foo(a = 1, a = 5))
148+
assertEquals("applyDynamicNamed(apply)((,d), (a,1), (,5), (a,c))", bar.foo("d", a = 1, 5, a = 'c'))
149+
150+
assertEquals("applyDynamic(apply)()", bar.foo.apply())
151+
assertEquals("applyDynamic(apply)(1)", bar.foo.apply(1))
152+
assertEquals("applyDynamic(apply)(1, 2, 3)", bar.foo.apply(1, 2, 3))
153+
assertEquals("applyDynamic(apply)(1, 2, a)", bar.foo.apply(1, 2, "a"))
154+
assertEquals("applyDynamic(apply)(1, 2, a)", bar.foo.apply(List(1, 2, "a"): _*))
155+
156+
assertEquals("applyDynamicNamed(apply)((a,1))", bar.foo.apply(a = 1))
157+
assertEquals("applyDynamicNamed(apply)((a,1), (b,2))", bar.foo.apply(a = 1, b = 2))
158+
assertEquals("applyDynamicNamed(apply)((a,1), (,0))", bar.foo.apply(a = 1, 0))
159+
assertEquals("applyDynamicNamed(apply)((a,1), (a,5))", bar.foo.apply(a = 1, a = 5))
160+
assertEquals("applyDynamicNamed(apply)((,d), (a,1), (,5), (a,c))", bar.foo.apply("d", a = 1, 5, a = 'c'))
161+
}
162+
105163
/** Test cains of dynamic calls. */
106164
def runBarTests() = {
107165
val bar = new Bar("bar")

0 commit comments

Comments
 (0)