Merge pull request #5751 from dotty-staging/fix-5750

Fix #5750: Type then part of If with no else part as Unit

Author: odersky

compiler/src/dotty/tools/dotc/ast/Positioned.scala

@@ -53,9 +53,6 @@ abstract class Positioned(implicit @constructorOnly src: SourceFile) extends Pro
         if (mySpan.isSynthetic) {
           if (!mySpan.exists && span.exists) {
             envelope(source, span.startPos) // fill in children spans
-            () // Note: the `()` is there to prevent some inefficient code from being generated.
-               // Without it we get an allocation of a span here since the result type of the `if`
-               // is `Any`, the lub of `Span` and `Unit`.
           }
           this
         }

compiler/src/dotty/tools/dotc/transform/PostTyper.scala

@@ -261,7 +261,6 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase
               ctx.compilationUnit.source.exists &&
               sym != defn.SourceFileAnnot)
               sym.addAnnotation(Annotation.makeSourceFile(ctx.compilationUnit.source.file.path))
-            tree
           }
           processMemberDef(super.transform(tree))
         case tree: New if isCheckable(tree) =>

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

@@ -728,12 +728,17 @@ class Typer extends Namer
   def typedIf(tree: untpd.If, pt: Type)(implicit ctx: Context): Tree = track("typedIf") {
     if (tree.isInline) checkInInlineContext("inline if", tree.posd)
     val cond1 = typed(tree.cond, defn.BooleanType)
-    val thenp2 :: elsep2 :: Nil = harmonic(harmonize, pt) {
-      val thenp1 = typed(tree.thenp, pt.notApplied)
-      val elsep1 = typed(tree.elsep.orElse(untpd.unitLiteral.withSpan(tree.span)), pt.notApplied)
-      thenp1 :: elsep1 :: Nil
+
+    if (tree.elsep.isEmpty) {
+      val thenp1 = typed(tree.thenp, defn.UnitType)
+      val elsep1 = tpd.unitLiteral.withSpan(tree.span.endPos)
+      cpy.If(tree)(cond1, thenp1, elsep1).withType(defn.UnitType)
+    }
+    else {
+      val thenp1 :: elsep1 :: Nil = harmonic(harmonize, pt)(
+        (tree.thenp :: tree.elsep :: Nil).map(typed(_, pt.notApplied)))
+      assignType(cpy.If(tree)(cond1, thenp1, elsep1), thenp1, elsep1)
     }
-    assignType(cpy.If(tree)(cond1, thenp2, elsep2), thenp2, elsep2)
   }
 
   /** Decompose function prototype into a list of parameter prototypes and a result prototype

compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala

@@ -568,7 +568,7 @@ class TestBCode extends DottyBytecodeTest {
 
   /* Test that objects compile to *final* classes. */
 
-  def checkFinalClass(outputClassName: String, source: String) = {
+  private def checkFinalClass(outputClassName: String, source: String) = {
     checkBCode(source) {
       dir =>
         val moduleIn   = dir.lookupName(outputClassName, directory = false)
@@ -623,4 +623,30 @@ class TestBCode extends DottyBytecodeTest {
         |  }
         |}
       """.stripMargin)
+
+  @Test def i5750 = {
+    val source =
+      """class Test {
+        |  def foo: String = ""
+        |  def test(cond: Boolean): Int = {
+        |    if (cond) foo
+        |    1
+        |  }
+        |}
+      """.stripMargin
+
+    checkBCode(source) { dir =>
+      val clsIn   = dir.lookupName("Test.class", directory = false).input
+      val clsNode = loadClassNode(clsIn)
+      val method  = getMethod(clsNode, "test")
+
+      val boxUnit = instructionsFromMethod(method).exists {
+        case Field(Opcodes.GETSTATIC, "scala/runtime/BoxedUnit", _, _) =>
+          true
+        case _ =>
+          false
+      }
+      assertFalse(boxUnit)
+    }
+  }
 }

library/src/scala/tasty/reflect/Printers.scala

@@ -1226,7 +1226,6 @@ trait Printers
                   printedPrefix  |= printProtectedOrPrivate(vdef)
                   if (vdef.symbol.flags.is(Flags.Mutable)) this += highlightValDef("var ", color)
                   else if (printedPrefix || !vdef.symbol.flags.is(Flags.CaseAcessor)) this += highlightValDef("val ", color)
-                  else this // val not explicitly needed
                 }
             }
           case _ =>

tests/pos/i5750.scala

@@ -0,0 +1,15 @@
+class Test {
+  def foo: String = ""
+  def test(cond: Boolean): Int = {
+    if (cond) foo
+    1
+  }
+
+  def test2(cond: Boolean): Unit = {
+    val x = if (cond) foo
+  }
+
+  def test3(cond: Boolean): Unit = {
+    val x: Unit = if (cond) foo
+  }
+}