Refine by-name handling and add test

Author: odersky

compiler/src/dotty/tools/dotc/config/Printers.scala

@@ -30,6 +30,7 @@ object Printers {
   val lexical: Printer = noPrinter
   val inlining: Printer = noPrinter
   val interactiv: Printer = noPrinter
+  val nullables: Printer = noPrinter
   val overload: Printer = noPrinter
   val patmatch: Printer = noPrinter
   val pickling: Printer = noPrinter

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

@@ -25,7 +25,7 @@ import transform.TypeUtils._
 import Nullables.given
 
 import collection.mutable
-import config.Printers.{overload, typr, unapp}
+import config.Printers.{overload, typr, unapp, nullables}
 import TypeApplications._
 
 import reporting.diagnostic.Message
@@ -1059,28 +1059,45 @@ trait Applications extends Compatibility {
    */
   private 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]) =>
         app match
           case Apply(fn, args) =>
             val dropNotNull = new TreeMap with
               override def transform(t: Tree)(given Context) = t match
-                case AssertNotNull(t0) if t0.symbol.is(Mutable) => transform(t0)
+                case AssertNotNull(t0) if t0.symbol.is(Mutable) =>
+                  nullables.println(i"dropping $t")
+                  transform(t0)
                 case t: ValDef if !t.symbol.is(Lazy) => super.transform(t)
-                case t: MemberDef => t // stop here since embedded references are out of order anyway
-                case t => super.transform(t)
+                case t: MemberDef =>
+                  // stop here since embedded references to mutable variables would be
+                  // out of order, so they would not asserted ot be not-null anyway.
+                  // @see Nullables.usedOutOfOrder
+                  t
+                case _ => super.transform(t)
+
+            object retyper extends ReTyper with
+              override def typedUnadapted(t: untpd.Tree, pt: Type, locked: TypeVars)(implicit ctx: Context): Tree = t match
+                case t: ValDef if !t.symbol.is(Lazy) => super.typedUnadapted(t, pt, locked)
+                case t: MemberDef => promote(t)
+                case _ => super.typedUnadapted(t, pt, locked)
 
             def postProcess(formal: Type, arg: Tree): Tree =
               val arg1 = dropNotNull.transform(arg)
-              if (arg1 ne arg) && !(arg1.tpe <:< formal) then
-                ctx.error(em"""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.""",
-                          arg.sourcePos)
-                arg
+              if arg1 eq arg then arg
               else
-                arg1
+                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
+                                |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.""",
+                            arg.sourcePos)
+                  arg
+                else
+                  nestedCtx.typerState.commit()
+                  arg2
 
             def recur(formals: List[Type], args: List[Tree]): List[Tree] = (formals, args) match
               case (formal :: formalsRest, arg :: argsRest) =>

compiler/test/dotty/tools/dotc/CompilationTests.scala

@@ -115,6 +115,7 @@ class CompilationTests extends ParallelTesting {
       compileFilesInDir("tests/neg-custom-args/deprecation", defaultOptions.and("-Xfatal-warnings", "-deprecation")),
       compileFilesInDir("tests/neg-custom-args/fatal-warnings", defaultOptions.and("-Xfatal-warnings")),
       compileFilesInDir("tests/neg-custom-args/allow-double-bindings", allowDoubleBindings),
+      compileFilesInDir("tests/neg-custom-args/explicit-nulls", defaultOptions.and("-Yexplicit-nulls")),
       compileDir("tests/neg-custom-args/impl-conv", defaultOptions.and("-Xfatal-warnings", "-feature")),
       compileFile("tests/neg-custom-args/implicit-conversions.scala", defaultOptions.and("-Xfatal-warnings", "-feature")),
       compileFile("tests/neg-custom-args/implicit-conversions-old.scala", defaultOptions.and("-Xfatal-warnings", "-feature")),

tests/neg-custom-args/explicit-nulls/byname-nullables.check

@@ -0,0 +1,28 @@
+-- [E007] Type Mismatch Error: tests/neg-custom-args/explicit-nulls/byname-nullables.scala:19:24 -----------------------
+19 |    if x != null then f(x)   // error: f is call-by-name
+   |                        ^
+   |                        Found:    (x : String | Null)
+   |                        Required: String
+-- Error: tests/neg-custom-args/explicit-nulls/byname-nullables.scala:43:32 --------------------------------------------
+43 |    if x != null then f(identity(x), 1)   // error: dropping not null check fails typing
+   |                        ^^^^^^^^^^^
+   |                     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-nullables.scala:68:24 --------------------------------------------
+68 |    if x != null then f(x, 1)   // error: dropping not null check typechecks OK, but gives incompatible result type
+   |                        ^
+   |                     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.
+-- [E134] Type Mismatch Error: tests/neg-custom-args/explicit-nulls/byname-nullables.scala:81:22 -----------------------
+81 |    if x != null then f(byName(x), 1)   // error: none of the overloaded methods match argument types
+   |                      ^
+   |                      None of the overloaded alternatives of method f in object Test7 with types
+   |                       (x: => String, y: Int): String
+   |                       (x: String, y: String): String
+   |                      match arguments (String | Null, (1 : Int))

tests/neg-custom-args/explicit-nulls/byname-nullables.scala

@@ -0,0 +1,82 @@
+object Test1 with
+
+  def f(x: String) =
+    x ++ x
+
+  def g() =
+    var x: String | Null = "abc"
+    if x != null then f(x)   // OK: f is call-by-value
+    else x
+
+
+object Test2 with
+
+  def f(x: => String) =
+    x ++ x
+
+  def g() =
+    var x: String | Null = "abc"
+    if x != null then f(x)   // error: f is call-by-name
+    else x
+
+object Test3 with
+
+  def f(x: String, y: String) = x
+
+  def f(x: => String | Null, y: Int) =
+    x
+
+  def g() =
+    var x: String | Null = "abc"
+    if x != null then f(x, 1)   // OK: not-null check successfully dropped
+    else x
+
+object Test4 with
+
+  def f(x: String, y: String) = x
+
+  def f(x: => String | Null, y: Int) =
+    x
+
+  def g() =
+    var x: String | Null = "abc"
+    if x != null then f(identity(x), 1)   // error: dropping not null check fails typing
+    else x
+
+object Test5 with
+  import compiletime.byName
+
+  def f(x: String, y: String) = x
+
+  def f(x: => String | Null, y: Int) =
+    x
+
+  def g() =
+    var x: String | Null = "abc"
+    if x != null then f(byName(identity(x)), 1)   // OK, byName avoids the flow typing
+    else x
+
+object Test6 with
+
+  def f(x: String, y: String) = x
+
+  def f(x: => String, y: Int) =
+    x
+
+  def g() =
+    var x: String | Null = "abc"
+    if x != null then f(x, 1)   // error: dropping not null check typechecks OK, but gives incompatible result type
+    else x
+
+object Test7 with
+  import compiletime.byName
+
+  def f(x: String, y: String) = x
+
+  def f(x: => String, y: Int) =
+    x
+
+  def g() =
+    var x: String | Null = "abc"
+    if x != null then f(byName(x), 1)   // error: none of the overloaded methods match argument types
+    else x