From 88ab4efd244c118b40a244bb91c445730fb4f2f5 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 21 Dec 2019 12:24:11 +0100 Subject: [PATCH 1/2] Avoid nonsensical error message in processByNameArgs Transforming the argument can already cause errors coming from TypeAssigne rthat need to be caught instead of being issued. --- compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala | 1 - compiler/src/dotty/tools/dotc/typer/Nullables.scala | 7 ++++--- compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala | 1 + .../explicit-nulls/byname-nullables1.check | 8 ++++++++ .../explicit-nulls/byname-nullables1.scala | 9 +++++++++ 5 files changed, 22 insertions(+), 4 deletions(-) create mode 100644 tests/neg-custom-args/explicit-nulls/byname-nullables1.check create mode 100644 tests/neg-custom-args/explicit-nulls/byname-nullables1.scala diff --git a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala index b03ace7f8d50..b74ad17a1311 100644 --- a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala +++ b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala @@ -89,7 +89,6 @@ object ErrorReporting { if (tree.tpe.widen.exists) i"${exprStr(tree)} does not take ${kind}parameters" else { - if (ctx.settings.Ydebug.value) new FatalError("").printStackTrace() i"undefined: $tree # ${tree.uniqueId}: ${tree.tpe.toString} at ${ctx.phase}" } diff --git a/compiler/src/dotty/tools/dotc/typer/Nullables.scala b/compiler/src/dotty/tools/dotc/typer/Nullables.scala index f66f5e996b90..4410660af1dc 100644 --- a/compiler/src/dotty/tools/dotc/typer/Nullables.scala +++ b/compiler/src/dotty/tools/dotc/typer/Nullables.scala @@ -464,7 +464,8 @@ object Nullables with */ def postProcessByNameArgs(fn: TermRef, app: Tree)(given ctx: Context): Tree = fn.widen match - case mt: MethodType if mt.paramInfos.exists(_.isInstanceOf[ExprType]) => + case mt: MethodType + if mt.paramInfos.exists(_.isInstanceOf[ExprType]) && !fn.symbol.is(Inline) => app match case Apply(fn, args) => val dropNotNull = new TreeMap with @@ -487,10 +488,10 @@ object Nullables with case _ => super.typedUnadapted(t, pt, locked) def postProcess(formal: Type, arg: Tree): Tree = - val arg1 = dropNotNull.transform(arg) + val nestedCtx = ctx.fresh.setNewTyperState() + val arg1 = dropNotNull.transform(arg)(given nestedCtx) if arg1 eq arg then arg else - val nestedCtx = ctx.fresh.setNewTyperState() val arg2 = retyper.typed(arg1, formal)(given nestedCtx) if nestedCtx.reporter.hasErrors || !(arg2.tpe <:< formal) then ctx.error(em"""This argument was typed using flow assumptions about mutable variables diff --git a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala index 09cf453e483e..d31e2b6c1bfd 100644 --- a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -411,6 +411,7 @@ trait TypeAssigner { else errorType(i"wrong number of arguments at ${ctx.phase.prev} for $fntpe: ${fn.tpe}, expected: ${fntpe.paramInfos.length}, found: ${args.length}", tree.sourcePos) case t => + if (ctx.settings.Ydebug.value) new FatalError("").printStackTrace() errorType(err.takesNoParamsStr(fn, ""), tree.sourcePos) } ConstFold(tree.withType(ownType)) diff --git a/tests/neg-custom-args/explicit-nulls/byname-nullables1.check b/tests/neg-custom-args/explicit-nulls/byname-nullables1.check new file mode 100644 index 000000000000..a43c141baf05 --- /dev/null +++ b/tests/neg-custom-args/explicit-nulls/byname-nullables1.check @@ -0,0 +1,8 @@ +-- Error: tests/neg-custom-args/explicit-nulls/byname-nullables1.scala:9:22 -------------------------------------------- +9 | if x != null then f(x.fld != null) // error + | ^^^^^^^^^^^^^ + | This argument was typed using flow assumptions about mutable variables + | but it is passed to a by-name parameter where such flow assumptions are unsound. + | Wrapping the argument in `byName(...)` fixes the problem by disabling the flow assumptions. + | + | `byName` needs to be imported from the `scala.compiletime` package. diff --git a/tests/neg-custom-args/explicit-nulls/byname-nullables1.scala b/tests/neg-custom-args/explicit-nulls/byname-nullables1.scala new file mode 100644 index 000000000000..cee09ecffb71 --- /dev/null +++ b/tests/neg-custom-args/explicit-nulls/byname-nullables1.scala @@ -0,0 +1,9 @@ +def f(op: => Boolean): Unit = () +def f(op: Int): Unit = () + +class C with + var fld: String | Null = null + +def test() = + var x: C | Null = C() + if x != null then f(x.fld != null) // error \ No newline at end of file From a5f51019cae2f897f5d09380d0d81b0f14e2d841 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sat, 21 Dec 2019 12:32:01 +0100 Subject: [PATCH 2/2] Only postprocess by-name args The previous code was buggy in that *all* arguments of a method containing a single by-name argument were post processed. --- .../src/dotty/tools/dotc/typer/Nullables.scala | 5 ++++- .../explicit-nulls/byname-nullables1.check | 16 ++++++++-------- .../explicit-nulls/byname-nullables1.scala | 4 +++- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Nullables.scala b/compiler/src/dotty/tools/dotc/typer/Nullables.scala index 4410660af1dc..544c8b49ec98 100644 --- a/compiler/src/dotty/tools/dotc/typer/Nullables.scala +++ b/compiler/src/dotty/tools/dotc/typer/Nullables.scala @@ -507,7 +507,10 @@ object Nullables with def recur(formals: List[Type], args: List[Tree]): List[Tree] = (formals, args) match case (formal :: formalsRest, arg :: argsRest) => - val arg1 = postProcess(formal.widenExpr.repeatedToSingle, arg) + val arg1 = + if formal.isInstanceOf[ExprType] + then postProcess(formal.widenExpr.repeatedToSingle, arg) + else arg val argsRest1 = recur( if formal.isRepeatedParam then formals else formalsRest, argsRest) diff --git a/tests/neg-custom-args/explicit-nulls/byname-nullables1.check b/tests/neg-custom-args/explicit-nulls/byname-nullables1.check index a43c141baf05..e4a46bf8322f 100644 --- a/tests/neg-custom-args/explicit-nulls/byname-nullables1.check +++ b/tests/neg-custom-args/explicit-nulls/byname-nullables1.check @@ -1,8 +1,8 @@ --- Error: tests/neg-custom-args/explicit-nulls/byname-nullables1.scala:9:22 -------------------------------------------- -9 | if x != null then f(x.fld != null) // error - | ^^^^^^^^^^^^^ - | This argument was typed using flow assumptions about mutable variables - | but it is passed to a by-name parameter where such flow assumptions are unsound. - | Wrapping the argument in `byName(...)` fixes the problem by disabling the flow assumptions. - | - | `byName` needs to be imported from the `scala.compiletime` package. +-- Error: tests/neg-custom-args/explicit-nulls/byname-nullables1.scala:10:6 -------------------------------------------- +10 | f(x.fld != null) // error + | ^^^^^^^^^^^^^ + | This argument was typed using flow assumptions about mutable variables + | but it is passed to a by-name parameter where such flow assumptions are unsound. + | Wrapping the argument in `byName(...)` fixes the problem by disabling the flow assumptions. + | + | `byName` needs to be imported from the `scala.compiletime` package. diff --git a/tests/neg-custom-args/explicit-nulls/byname-nullables1.scala b/tests/neg-custom-args/explicit-nulls/byname-nullables1.scala index cee09ecffb71..eb28a5af96c0 100644 --- a/tests/neg-custom-args/explicit-nulls/byname-nullables1.scala +++ b/tests/neg-custom-args/explicit-nulls/byname-nullables1.scala @@ -6,4 +6,6 @@ class C with def test() = var x: C | Null = C() - if x != null then f(x.fld != null) // error \ No newline at end of file + if x != null then + f(x.fld != null) // error + require(x.fld != null, "error") // ok \ No newline at end of file