scala3 icon indicating copy to clipboard operation
scala3 copied to clipboard

Fixed Missing warning for invalid recursive val

Open gagandeepkalra opened this issue 2 years ago • 6 comments

resolves #14429

cc. @anatoliykmetyuk

gagandeepkalra avatar Mar 01 '22 17:03 gagandeepkalra

Also #12943 which already seems to be fixed.

griggt avatar Mar 01 '22 21:03 griggt

About the error message:

  • Infinite loop in function body does not seem appropriate for vals.
  • Why don't we refer to the precise place where the recursion happens instead of embedding the function/val body in the error message?

Currently:

-- Warning: tests/neg-custom-args/fatal-warnings/i13011.scala:15:29 ------------
15 |  lazy val simple6: String = {  // error
   |                             ^
   |                             Infinite loop in function body
   |                             {
   |                               this.simple6
   |                               "aa"
   |                             }
16 |    this.simple6
17 |    "aa"
18 |  }

Instead, I would suggest:

-- Warning: tests/neg-custom-args/fatal-warnings/i13011.scala:16:9 -------------
16 |    this.simple6
   |    ^^^^^^^^^^^^
   |    Infinite loop

That could be achieved by simplifying checkNotSelfRef to:

def checkNotSelfRef(t: RefTree) =
  if t.symbol eq sym then
    report.warning("Infinite loop", t.srcPos)

Seems to work for for extension methods and givens as well:

-- Warning: tests/neg-custom-args/fatal-warnings/i13011.scala:22:41 ------------
22 |  extension (n: Int) def foo(): Unit = n.foo()
   |                                         ^^^
   |                                         Infinite loop
-- Warning: tests/neg-custom-args/fatal-warnings/i13011.scala:24:17 ------------
24 |  given x: Int = x
   |                 ^
   |                 Infinite loop

mbovel avatar Mar 14 '22 11:03 mbovel

Minor: should we rename this phase to CheckInfiniteLoops as it's not just about implicits?

mbovel avatar Mar 14 '22 12:03 mbovel

@gagandeepkalra do you plan to further work on this PR? It would be great to have it merged! 😃

mbovel avatar Sep 06 '22 18:09 mbovel

Hi @mbovel, I'll get back to working this.

gagandeepkalra avatar Sep 11 '22 14:09 gagandeepkalra

Hi @mbovel, I'll get back to working this.

Hey @gagandeepkalra do you still plan on returning to this?

ckipp01 avatar May 10 '23 07:05 ckipp01