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

Automatic rewrite for <function> _ can produce non-compiling code for curried functions #21394

Closed
WojciechMazur opened this issue Aug 16, 2024 · 1 comment · Fixed by #21715
Closed

Comments

@WojciechMazur
Copy link
Contributor

Issue found when applying automatic rewrites for sagifogel/proptics in Open CB

Compiler version

All Scala 3.4+ versions

Minimized code

//> using options -Ykind-projector
import scala.collection.SortedMap

trait NonEmptyMap[K, +A]:
  def toSortedMap: SortedMap[K, A]
  
trait FoldableWithIndex[F[_], I]:
  def test[K] = new FoldableWithIndex[NonEmptyMap[K, *], K] {
    def foldLeftWithIndex[A, B](f: (B, (A, K)) => B)(fa: NonEmptyMap[K, A], b: B): B =
      mapLikeFoldLeftWithIndex((fa.toSortedMap.foldLeft[B] _))(f)(b)
  }

  def mapLikeFoldLeftWithIndex[A, B, K](foldLeft: B => ((B, (K, A)) => B) => B)(f: (B, (A, K)) => B)(b: B): B = ???

Output

//> using options -Ykind-projector
import scala.collection.SortedMap

trait NonEmptyMap[K, +A]:
  def toSortedMap: SortedMap[K, A]
  
trait FoldableWithIndex[F[_], I]:
  def test[K] = new FoldableWithIndex[NonEmptyMap[K, *], K] {
    def foldLeftWithIndex[A, B](f: (B, (A, K)) => B)(fa: NonEmptyMap[K, A], b: B): B =
      mapLikeFoldLeftWithIndex(((() => fa.toSortedMap.foldLeft[B])))(f)(b)
  }

  def mapLikeFoldLeftWithIndex[A, B, K](foldLeft: B => ((B, (K, A)) => B) => B)(f: (B, (A, K)) => B)(b: B): B = ???
-- [E007] Type Mismatch Error: /Users/wmazur/projects/sandbox/src/main/scala/schema.scala:10:39 
10 |      mapLikeFoldLeftWithIndex(((() => fa.toSortedMap.foldLeft[B])))(f)(b)
   |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^
   |                             Found:    B => Any
   |                             Required: ((Any, (Any, Any)) => Any) => Any
   |
   | longer explanation available when compiling with `-explain`

Expectation

The compiler should not suggest an automatic rewrite when the function is curried. Other cases for rewrite should be also considered

@WojciechMazur WojciechMazur changed the title Automatic rewrite for <function> _ can produce non-compiling code Automatic rewrite for <function> _ can produce non-compiling code for curried functions Aug 16, 2024
@bracevac
Copy link
Contributor

bracevac commented Oct 3, 2024

De-noised example:

//> using options -rewrite -source 3.4-migration 
trait Indirect:
  def fun(x: Int)(y: Int): Int

trait Container:
  def indirectDef: Indirect
  val indirectVal: Indirect
  def fun(x: Int)(y: Int): Int
  
def test(c: Container): Int =
    use(c.indirectDef.fun _) // becomes (() => c.indirectDef.fun), WRONG! Should be c.indirectDef.fun
  + use(c.indirectVal.fun _) // becomes c.indirectVal.fun, OK!
  + use(c.fun _)             // becomes c.fun, OK!

def use(f: Int => Int => Int): Int = ???

It appears the rewrite logic of #18926
does not correctly handle eta-expanding a method accessed through at least one other intermediate method.

bracevac added a commit that referenced this issue Oct 7, 2024
A rewrite would previously produce uncompilable
code if the access path to the eta-expanded
function goes through at least one `def`.

Fixes #21394
@WojciechMazur WojciechMazur added this to the 3.6.0 milestone Oct 8, 2024
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.

2 participants