Skip to content
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

infinite loop when evaluating a recursive given definition #10947

Closed
Swoorup opened this issue Dec 29, 2020 · 13 comments · Fixed by #15481
Closed

infinite loop when evaluating a recursive given definition #10947

Swoorup opened this issue Dec 29, 2020 · 13 comments · Fixed by #15481
Milestone

Comments

@Swoorup
Copy link

Swoorup commented Dec 29, 2020

When running a program which consumes opaque types as follows, it appears to be stuck on an infinite loop. Not sure what is going on here.

Minimized code

object Prices {
  opaque type Price = BigDecimal

  object Price{
    given Ordering[Price] = summon[Ordering[BigDecimal]]
  }

}

extension[K,V] (m1: Map[K,V]) def computeDiff(m2: Map[K,V])(using Ordering[K])(using orderAsc: Boolean = true): Unit = println("Ok....")

import Prices._
@main def mainz = {
    println("begin")
    val result = Map.empty[Price,Int].computeDiff(Map.empty[Price,Int])
    println(s"end $result")
}

Scastie link: https://scastie.scala-lang.org/ajEagvzvTbitmzbqFt0Oxw

Output

begin

Expectation

Should successfully print the following:

begin
Ok....
end

This works if the Price was swapped with BigDecimal in the line

  val result = Map.empty[Price,Int].computeDiff(Map.empty[Price,Int])

This is happening on 3.0.0-M3

@som-snytt
Copy link
Contributor

som-snytt commented Dec 29, 2020

WFM

➜  snips scalac -version
Scala compiler version 3.0.0-M3 -- Copyright 2002-2020, LAMP/EPFL
➜  snips scalac -d /tmp i10947.scala
➜  snips scala -classpath /tmp mainz
begin
Ok....
end ()

Sorry I didn't look closely. Corrected...

➜  snips cat i10947.scala

object Prices:
  opaque type Price = BigDecimal

  object Price:
    given Ordering[Price] = summon[Ordering[BigDecimal]]

  extension[K,V] (m1: Map[K,V]) def computeDiff(m2: Map[K,V])(using Ordering[K])(using orderAsc: Boolean = true): Unit =
    println("Ok....")
end Prices

import Prices._

@main def mainz =
  println("begin")
  val result = Map.empty[Price,Int].computeDiff(Map.empty[Price,Int])
  println(s"end $result")

@griggt
Copy link
Contributor

griggt commented Dec 29, 2020

Seems like the issue occurs on Scastie but not locally (works for me too)

@Swoorup
Copy link
Author

Swoorup commented Dec 29, 2020

@som-snytt @griggt please remove the indentation on the import and mainz block, i.e It appears it should not be on the same block as where the opaque type is defined for the issue to occur.

It should occur then. I'll update the example to use curlies for clarity.

import Prices._

@main def mainz =
  println("begin")
  val result = Map.empty[Price,Int].computeDiff(Map.empty[Price,Int])
  println(s"end $result")

@griggt
Copy link
Contributor

griggt commented Dec 29, 2020

Still works for me locally, formatted with the curly braces (I had it indented as you intended without them also).

I am able to reproduce it on Scastie, however.

EDIT: nvm, it hangs at runtime, not compile time

@Swoorup Swoorup changed the title Stuck on infinite loop when consuming opaque types. Generated program stuck on infinite loop when consuming opaque types. Dec 29, 2020
@julienrf
Copy link
Contributor

object Prices {
  opaque type Price = BigDecimal

  object Price{
    given Ordering[Price] = summon[Ordering[BigDecimal]]
  }

}

What happens here is that you have a recursive given definition. summon[Ordering[BigDecimal]] returns Price.given_Ordering_Price since the opaque type Price is equal to BigDecimal at that place.

What you need to do is to summon the Ordering[BigDecimal] instance at a place where the type Price is opaque (not transparent), or at a place where Price.given_Ordering_Price is not in scope.

It seems that the following works fine:

object Prices {
  opaque type Price = BigDecimal

  private val orderingBigDecimal = summon[Ordering[BigDecimal]]
  object Price{
    given Ordering[Price] = orderingBigDecimal
  }

}

extension[K,V] (m1: Map[K,V]) def computeDiff(m2: Map[K,V])(using Ordering[K])(using orderAsc: Boolean = true): Unit = println("Ok....")

import Prices._
@main def mainz =
    println("begin")
    val result = Map.empty[Price,Int].computeDiff(Map.empty[Price,Int])
    println(s"end $result")

https://scastie.scala-lang.org/6gIRy2nwSLGcsIAvlvTVKw

@julienrf julienrf changed the title Generated program stuck on infinite loop when consuming opaque types. infinite loop when evaluating a recursive given definition Dec 29, 2020
@julienrf
Copy link
Contributor

I am not sure what we should do about this problem. I think the current behavior is expected. I don’t think we can reliably implement a detection mechanism for such loops at compilation time so I’m closing the issue. Feel free to re-open it if I miss something.

@Swoorup
Copy link
Author

Swoorup commented Dec 29, 2020

It seems like an easy pitfall to fall into, but then again I am not sure how to best prevent these kind of issue myself.

I wonder there is any chance to eagarly evaluate givens for opaque type at compile time? I don't know if thats possible.

@julienrf
Copy link
Contributor

julienrf commented Dec 29, 2020

This is not specific to opaque types, the same issue can happen with regular type aliases, abstract type members, etc. It is easy to construct a recursive instance, but it is known to be a hard problem to detect such constructions at compile-time. Also, not all the recursive given definitions are wrong. Some of them are correct (this is how we define instances for recursive data types).

@Swoorup
Copy link
Author

Swoorup commented Dec 29, 2020

😦 That is a bit worrying, hope we can think of something in the future.

@liufengyun
Copy link
Contributor

We could enhance the initialization check to check for possible non-termination of lazy fields. A related issue #9668.

@tel
Copy link

tel commented Jan 15, 2021

What's interesting about this one is that one common use case is derivation (a la Haskell's generalize newtype deriving). Would it be possible to hook something into the derivation mechanism?

@som-snytt
Copy link
Contributor

It is a lint in Scala 2

  ~ scala -Xlint
Welcome to Scala 2.13.4 (OpenJDK 64-Bit Server VM, Java 15).
Type in expressions for evaluation. Or try :help.

scala> type Price = BigDecimal
type Price

scala> implicit val `order price`: Ordering[Price] = implicitly[Ordering[BigDecimal]]
                                                               ^
       warning: Implicit resolves to enclosing value order price
val order price: Ordering[Price] = null

scala>

@kubukoz
Copy link
Contributor

kubukoz commented Feb 13, 2021

What's interesting about this one is that one common use case is derivation (a la Haskell's generalize newtype deriving). Would it be possible to hook something into the derivation mechanism?

@tel I really wish this was possible. I don't see people switching from @newtype to opaque types anytime soon if this isn't made easier - I just bumped on the issue at question myself and the boilerplate needed to do it "properly" is very discouraging.

@odersky odersky reopened this Jun 19, 2022
odersky added a commit to dotty-staging/dotty that referenced this issue Jun 19, 2022
 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 scala#15474
Fixes scala#10947
@Kordyjan Kordyjan added this to the 3.2.1 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants