Missing warning for invalid recursive val.

[Swoorup]

Compiler version

3.1.1

Minimized code

val a: Int = a

https://scastie.scala-lang.org/9Naw4C8JRuKQtPd0VXBXLw

No warnings or compilation errors

Expectation

Compiler error or warning.
Scala 2 gives value a in object Playground does nothing other than call itself recursively

Without warning, this is easy to create bugs like this. For eg:

 def create[F[_], S](using enc: Encoder[S])(state: S, fsm: FSM[F, S, Cmd, Option[Stat]]) =
    new Stat[F]:
      val state = state
      val fsm   = fsm

[som-snytt]

It seems to me this came up again but I can't find a ticket or discussion.

That was an early retronym contribution. Someone always says why would anyone do that, so it's worth noticing why retronym used to do that in 2012:

trait A { def a: Int }
def foo(a: Int) {
  new A {
    def a = a
  }
}

I guess that is the same use case.

Note the similarity to the last tweak to name binding, illustrated at this ticket and changed in the PR linked there (which I will try not to pollute with a reference). The problem there was that the shadowing member was inherited.

object C {
  def m(x: Int) = 1
  object T extends K {
    val x = m(1)  // ambiguous
  }
}
class K {
  def m(i: Int) = 2
}

Perhaps the tweak could be tweaked: the shadowing member matches an inherited member, which is grounds for making the name on the RHS ambiguous. However, you're supposed to be able to introduce an override specifically to disambiguate in this case, so maybe this only justifies a warning when the shadowing member is the enclosing definition.

trait T {
  def f: Int
}
def g(f: Int): Int = {
  val t = new T {
    def x = f      // ambiguous if f defined in T
    //val f = f     // no warn
    def f = f     // warn recursion
  }
  t.f
}

There was another recent request for help with shadowing.

Maybe this is the ticket I was thinking of, to warn on certain recursions.

This is your ticket for givens: #10947

Here is tpolecat's almost a duplicate but lazy vals. #12943 which is what I was remembering.

[Swoorup]

This is your ticket for givens: #10947

I will keep running into this issue 😄

[nicolasstucki]

I remember adding something for recursive defs def f: Int = f at some point. Maybe that can be generalized or reused.

[som-snytt]

I linked the recursive warning at "Maybe #7821 I was thinking of, to warn on certain recursions." #7821 That was clever.

[mbovel]

Indeed dotty-staging@ddf73dd introduced the check for recursive definitions:

-- Warning: tests/neg/recursive-def.scala:2:6 ----------------------------------
2 |  def f: Int = f
  |  ^^^^^^^^^^^^^^
  |  Infinite recursive call

It is done during the TailRec mini-phase:

https://github.com/lampepfl/dotty/blob/74c29542f9008e0b8cd7848d9c9383a986200f39/compiler/src/dotty/tools/dotc/transform/TailRec.scala#L204

The check for vals should probably be done somewhere else. Should we add a mini-phase for this, or can you think of an appropriate existing phase to do this?

[liufengyun]

This should already be covered by -Ysafe-init.

Edit: As the option -Ysafe-init is opt-in, it's still a good idea to have an additional check for such cases that are enabled by default.

[mbovel]

Excellent, indeed:

28 |  val y: Int = y
   |      ^
   |      Access non-initialized value y. Calling trace:
   |       -> val y: Int = y    [ <...>.scala:28 ]

it's still a good idea to have an additional check for such cases that are enabled by default.

Is it? Shouldn't we encourage users interested in these checks to use -Ysafe-init instead, as it is a way more robust and powerful alternative to the shallow checks we could add by default?

@Swoorup could you use this flag for your use-cases?

[Swoorup]

Asking the obvious, why isn't -Ysafe-init a default? Or what is preventing from it being the default? I recall turning it on at one point, and then subsequently leaving it off due to munit and IDE issue. I will turn it on again.

Regardless I do expect this particular case to at least give me a warning. Its not immediately obvious to figure what the issue is and let alone where the issue lies, in a simple 50-100 line code, when you use the default vanilla setting.

[liufengyun]

Asking the obvious, why isn't -Ysafe-init a default?

There is still something we want to improve, in particular, better error messages for handling inner classes, and improve the modularity of the check (warn for subtle initialization behavior of open class/traits).

And we also want to get more community feedback before turning it on by default.