-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Warning for infinite recursive lazy vals #13749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: Tom Grigg <[email protected]> Co-authored-by: Nicolas Stucki <[email protected]> Co-authored-by: Timothée Andres <[email protected]>
The scripted test change seems correct. I'm not sure what the original intent of |
|
||
def firstDigitIsEven(n: Int): Boolean = if n % 10 == n then n % 2 == 0 else firstDigitIsEven(n / 10) | ||
|
||
lazy val simple4: String = if firstDigitIsEven(22) then this.simple4 else "a" // error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deja-vu, then: this shouldn't error, because it's only conditionally recursive, which we bias to not warning (IIRC). I don't understand why this is passing CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only check the condition:
case If(cond, _, _) =>
checkNotLooping(cond)
Is this erroring as a consequence of being a lazy val?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no there's no error, it's my bad, I forgot to put ok
there, testCompilation
was indeed failing
I've pushed in the fix now.
Fixes #13011
Part of the issue spree
Co-authored-by: Tom Grigg [email protected]
Co-authored-by: Nicolas Stucki [email protected]
Co-authored-by: Timothée Andres [email protected]