Find more looping implicits

[odersky]

1. Also check apply methods of companions of implicit or given classes.
   Specifically apply methods of implicit Conversions.

2. Look inside Inlined nodes to detect loops.

Fixes #15474
Fixes #10947

[julienrf]

Could you please add a comment to explain the intent behind the condition on the right side of the ||? I wonder if this condition is too strong, but I am not sure. Do we have existing “pos” tests (the PR only contains two “neg” tests)?

[som-snytt]

This will help folks who need help and also save time on tickets!

[wangzaixiang]

I tried the main branch for the following code (compiler build from commit: de3a82c, Find more looping implicits):

// scalaVersion := "3.2.1-RC1-bin-SNAPSHOT"

object Demo2: 

  given Conversion[String, Int] with
    def apply(s: String): Int = s.toInt

  def main(args: Array[String]): Unit = 
    val s = "123"
    val i: Int = s
    println(s"s=$s i=$i }")

It report "Infinite loop in function body" now.

[som-snytt]

@wangzaixiang Your example is now a test. Note that s.toInt requires a conversion, and it uses the conversion defined here (not the "usual" one), so that it loops. (Your ticket called it wrong, but it is expected; the linked tickets have more examples.) I'm not sure whether you approve this resolution.

[wangzaixiang]

@wangzaixiang Your example is now a test. Note that s.toInt requires a conversion, and it uses the conversion defined here (not the "usual" one), so that it loops. (Your ticket called it wrong, but it is expected; the linked tickets have more examples.) I'm not sure whether you approve this resolution.

Sure. I got it. It's not a BUG. thanks.

[som-snytt]

My eyes went briefly out of focus when I looked at the example. It's like an optical illusion.

To clarify further, the conversion to Int is chosen because Int does have a toInt method.

Rubbing my eyes.

[WojciechMazur]

@odersky Isn't needed change to check file of test i13044 a sign of regression? Since introducing this change Caliban is not able to build in the community build (it's using -Xfatal-warnings)
Test i13044 was a reproduction of issues Caliban found when upgrading 3.0.0 -> 3.0.1
3.2.1-RC2-nightly logs for failing Caliban build https://scala3.westeurope.cloudapp.azure.com/blue/rest/organizations/jenkins/pipelines/buildCommunityProject/runs/890/nodes/12/log/?start=0

The mentioned test file is used 2 times:

• in neg tests where it fails and output is being checked
• in pos tests with "-Xmax-inlines:33", which it still passes, but its warnings are being ignored

PS.
On the other hand, it might be actually an improvement in that case, as it might directly point to unsafe parts that might fail in some cases. @ghostdogpr You might be interested in that 3.2.x change

[ghostdogpr]

The code that fails to compile is deriving an instance for a recursive ADT (simplified):

sealed trait InputValue
object InputValue {
  case class ListValue(values: List[InputValue])          extends InputValue 
  case class ObjectValue(fields: Map[String, InputValue]) extends InputValue 
  case class SimpleValue(name: String)                    extends InputValue
}

And it was called like this:

implicit lazy val inputValueSchema: Schema[Any, InputValue] = Schema.derived

During the derivation, it will need an instance of itself because it's recursive, but it was finding itself thanks to lazy.

[odersky]

@ghostdogpr Can we minimize this to a self-contained example? That would be important to see whether the code is actually safe and, if that's the case, to refine the checking analysis.

[ghostdogpr]

@ghostdogpr Can we minimize this to a self-contained example? That would be important to see whether the code is actually safe and, if that's the case, to refine the checking analysis.

Here's a minimized example: https://gist.github.com/ghostdogpr/b525ee4062427fa06861b96e3544fae2
It works well with 3.1.1 and the warning shows up with 3.2.1-RC1-bin-20220705-9bb3108-NIGHTLY.
Hope it helps!

[odersky]

@ghostdogpr Thanks for the minimization! It seems the change in this PR uncovered another bug in the loop checking. Fix forthcoming.