Improve forward reference handling?

#1

Hello

Currently, in Scala 2 and Dotty, forward references are handled in such a way that the code can compile yet break at runtime, cf this example of wrong vals init in an object.

Being bitten by this behavior is often a big disappointment: a typesafe language compiling should run properly, not return unexpected nulls and NullPointerException. It can also appear when someone reformat the code (and I’m not speaking of big refactoring, just changing the fields declaration order), and thus suddenly introduce some regressions seen only at runtime. This is really annoying and a trust killer.

I’ve read about the rational behind the current behavior, due to trait instantiation.

The current solution, -Xcheckinit, comes with so much performance warnings that I guess not one is willing to really use it:

It is inadvisable to use this flag outside of testing. It adds significantly to the code size by putting a wrapper around all potentially uninitialized field accesses: the wrapper will throw an exception rather than allow a null (or 0/false in the case of primitive types) to silently appear. Note also that this adds a runtime check: it can only tell you anything about code paths which you exercise with it in place.

However i hope it could be improved, essentially at compilation by disallowing illegal forward references in objects (just like Java does with the “Illegal forward reference” compiler error) and maybe even in classes (or maybe provide a warning there).

I guess it could also help when implicit resolution compiles but breaks at runtime due, again, to ordering issue, which also shows up at runtime, often rather lately, through NPE/null during Json conversions…

Tackling it a better would really enhance onboarding new developers (scala compiling but breaking at runtime isn’t exactly convincing to new comers…) as well as reduce wariness when fiddling with fields’ ordering, which should come for free in a typesafe language.

What’s your pick on the matter?

Thanks in advance

5 Likes
Better Implicit Search Errors: problematic cases wanted!
#2

Unfortunately, this is a hard problem. I remember some work in this direction. Maybe @liufengyun or @biboudis can give us more details?

1 Like
#3

Thanks for raising the issue @cluelessjoe .

The short answer: we have an attack for exactly the problem you mentioned, and we are working on a compiler plugin implementation to make it available.

With our solution, the compiler will report an error when bar is used:

object ForwardReferenceGotcha {
  val foo = Foo(bar)      // error
  val bar = Bar()

  case class Bar(i: Int = 0)
  case class Foo(bar: Bar)

  def main(args: Array[String]): Unit = {
    println(foo)
  }
}

It handles inheritance:

abstract class AbstractFile {
   def name: String
   val extension: String = Path.extension(name)     // error
}

class RemoteFile(url: String) extends AbstractFile {
   val localFile: String = url.hashCode + ".tmp"
   def name: String = localFile
}

and linearization:

trait TA {
  val x = "world"
}
trait TB {
 def x: String
 val m = "hello" + x
}
class Foo extends TA with TB  // OK
class Bar extends TB with TA  // error

and inner classes:

object Trees {
  class ValDef { counter += 1 } // error
  class EmptyValDef extends ValDef
  val theEmptyValDef = new EmptyValDef // error
  private var counter = 0
}

and functions:

abstract class Parent {
  val f: () => String = () => this.message
  def message: String
}
class Child extends Parent {
  val a = f()                          // error
  val b = "hello"
  def message: String = b
}

class Foo {
  val even: Int => Boolean = (n: Int) => n == 0 || odd(n - 1)
  val flag1: Int = even(3)                            // error
  val odd: Int => Boolean = (n: Int) => n == 1 || even(n - 1)
  val flag2: Boolean = odd(6)
}

For all the features above, no annotation is required. However, to support leaking of this , an annotation like @cold is required:

class Parent { val child = new Child(this) }
class Child(parent: Parent @cold) {
  println(parent.child)      // error
}

The analysis will reject some anti-patterns of initializaiton, e.g., assigning this to an already initialized object. However, it will only report warnings in such cases (it reports errors only if it’s certain that an uninitialized field is used), so we hope it will not create big usability or migration problems.

We are happy to hear more your thoughts on the problem.

8 Likes
Future transformation methods should null-check their parameters
#4

Will the error be emitted on the definition of val extension: String = Path.extension(name)? Or on def name: String = localFile?

#5

The error reporting part is still under consideration. In principle, we can report a path that leads to initialization problems.

#6

thanks a lot for the answer.

The solution looks really neat and exhaustive (i don’t know enough to assert it is really but still it cares for more than i thought of).

Are you targeting Dotty and/or Scala2?

The sooner it can be released the better :slight_smile:

#7

We are targeting Dotty initially. However, nothing prevents the solution from being implemented for Scala2. We are more familiar with Dotty, so it’s an easy starting point, and some of the lessons learned & problems encountered in implementation may be re-used in Scala2 version.

Thanks for letting us know, we are working to make it happen soon to solicit more feedback.

2 Likes
#8

btw another issue we spoke of recently in Better implicit search errors:

import org.scalatest.FunSuite
import play.api.libs.json.{Format, Json}

class ImplicitOrderingTest extends FunSuite {

  implicit val BarFormat: Format[Bar] = Json.format
  implicit val FooFormat: Format[Foo] = Json.format
  test("Writing Bar - right order: compile and crash at runtime") {

    assert(BarFormat.writes(Bar(Foo(1))).toString() === """{"f":{"i":1}}""")  //throws!!
  }

}

case class Foo(i: Int)

case class Bar(f: Foo)

This compiles but throws NPE at runtime.
Ideally it shouldn’t compile, just like this doesn’t compile :

import org.scalatest.FunSuite
import play.api.libs.json.{Format, Json}

class ImplicitOrderingTest extends FunSuite {

  test("Writing Bar - right order: compile and crash at runtime") {
    implicit val BarFormat: Format[Bar] = Json.format
    implicit val FooFormat: Format[Foo] = Json.format

    assert(BarFormat.writes(Bar(Foo(1))).toString() === """{"f":{"i":1}}""")  
  }

}

case class Foo(i: Int)

case class Bar(f: Foo)

The compiler error being “forward reference extends over definition of value BarFormat
[error] implicit val BarFormat: Format[Bar] = Json.format”

Hopefully your work could also improve this :slight_smile:

#9

Thanks for letting us know the case. Yes, the checker will be able to catch the bug and issue an error.

1 Like