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

Scala 2.13 regression: type param affecting overload #12663

Closed
japgolly opened this issue Jun 1, 2021 · 5 comments · Fixed by #17501
Closed

Scala 2.13 regression: type param affecting overload #12663

japgolly opened this issue Jun 1, 2021 · 5 comments · Fixed by #17501
Labels
backlog No work planned on this by the core team for the time being. itype:bug

Comments

@japgolly
Copy link
Contributor

japgolly commented Jun 1, 2021

Compiler version

3.0.0

Minimized code (ver 1)

final class HookComponentBuilder[Ctx, CtxFn[_]] {
  def asd[A](f: Ctx => A): A = ???
  def asd[A](f: CtxFn[A]): A = ???
}

object HookCtx {
  case class P1[P, H1](props: P, hook1: H1)
}

object HookCtxFn {
  sealed trait P1[P, H1] { type Fn[A] = (P, H1) => A }
}

object Test {
  val b: HookComponentBuilder[HookCtx.P1[String, Int], HookCtxFn.P1[String, Int]#Fn] = ???

  b.asd($ => $.props.length + $.hook1)
  b.asd((props, hook1) => props.length + hook1)
}

Minimized code (ver 2)

Changing HookCtxFn.P1 to a type lambda...

final class HookComponentBuilder[Ctx, CtxFn[_]] {
  def asd[A](f: Ctx => A): A = ???
  def asd[A](f: CtxFn[A]): A = ???
}

object HookCtx {
  case class P1[P, H1](props: P, hook1: H1)
}

object HookCtxFn {
  type P1[P, H1] = [A] =>> (P, H1) => A
}

object Test {
  val b: HookComponentBuilder[HookCtx.P1[String, Int], HookCtxFn.P1[String, Int]] = ???

  b.asd($ => $.props.length + $.hook1)
  b.asd((props, hook1) => props.length + hook1)
}

Output

[error] 18 |  b.asd((props, hook1) => props.length + hook1)
[error]    |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]    |        Wrong number of parameters, expected: 1

Expectation

It should compile. It does with Scala 2.13

Workaround

Inlining HookCtxFn.P1 makes it work.

final class HookComponentBuilder[Ctx, CtxFn[_]] {
  def asd[A](f: Ctx => A): A = ???
  def asd[A](f: CtxFn[A]): A = ???
}

object HookCtx {
  case class P1[P, H1](props: P, hook1: H1)
}

object Test {
  val b: HookComponentBuilder[HookCtx.P1[String, Int], [A] =>> (String, Int) => A] = ???

  b.asd($ => $.props.length + $.hook1)
  b.asd((props, hook1) => props.length + hook1)
}
@odersky
Copy link
Contributor

odersky commented Jun 1, 2021

This could well fall into the grey zone where Scala 3 and Scala 2 are just different. I was staring at the code for a while and trying to figure out things by fiddling with it. My impression is that this code is so complex that nobody will be able to tell who is right here. If we should try to fix this we need a better minimization and a cogent argument why the overloading resolution should succeed in this case. But in general: please don't write code like this! you are tempting the compiler's curse.

@japgolly
Copy link
Contributor Author

japgolly commented Jun 1, 2021

Ok I can see how this would look complex without context. Let me share the context and I think it will become simple to understand.

This is basically arity abstraction where users can specify both (allArgs...) => Z and HelperClassWithAllArgs => Z

Imagine this, it's a single API method (asd) repeated over and over again to cater for different sized inputs/contexts:

object HookComponentBuilder {
  final case class Arity2NamedArgs[A, B](arg1: A, arg2: B)
  final class Arity2[A, B] {
    def asd[Z](f: Arity2NamedArgs[A, B] => Z): Z = ???
    def asd[Z](f:                (A, B) => Z): Z = ???
  }

  final case class Arity3NamedArgs[A, B, C](arg1: A, arg2: B, arg3: C)
  final class Arity3[A, B, C] {
    def asd[Z](f: Arity3NamedArgs[A, B, C] => Z): Z = ???
    def asd[Z](f:                (A, B, C) => Z): Z = ???
  }

  final case class Arity4NamedArgs[A, B, C, D](arg1: A, arg2: B, arg3: C, arg4: D)
  final class Arity4[A, B, C, D] {
    def asd[Z](f: Arity4NamedArgs[A, B, C, D] => Z): Z = ???
    def asd[Z](f:                (A, B, C, D) => Z): Z = ???
  }

  // etc for 5 up to 22
}

And now imagine there are 20 methods instead of just asd.
Code generation is an option but this exists in Scala.JS land so it would greatly increase the output JS size, not to mention that maintenance becomes a nightmare. We can nicely separate the API and arity abstraction like this:

object HookComponentBuilder {
  // Can't abstract these away
  final case class Arity2NamedArgs[A, B      ](arg1: A, arg2: B)
  final case class Arity3NamedArgs[A, B, C   ](arg1: A, arg2: B, arg3: C)
  final case class Arity4NamedArgs[A, B, C, D](arg1: A, arg2: B, arg3: C, arg4: D)

  // This API can now be made abstract and written just once
  final class Api[NamedArgs, ArgFn[_]] {
    def asd[Z](f: NamedArgs => Z): Z = ???
    def asd[Z](f: ArgFn[Z]      ): Z = ???
  }

  // Now we can recreate the previous types if we want to
  type Arity2[A, B]       = Api[NamedArgs[A, B      ], [Z] =>> (A, B      ) => Z]
  type Arity3[A, B, C]    = Api[NamedArgs[A, B, C   ], [Z] =>> (A, B, C   ) => Z]
  type Arity4[A, B, C, D] = Api[NamedArgs[A, B, C, D], [Z] =>> (A, B, C, D) => Z]
}

The problem I'm running into with Scala 3 is that the above code works but the following doesn't, even though it's functionally identical.

object HookComponentBuilder {
  // Can't abstract these away
  final case class Arity2NamedArgs[A, B      ](arg1: A, arg2: B)
  final case class Arity3NamedArgs[A, B, C   ](arg1: A, arg2: B, arg3: C)
  final case class Arity4NamedArgs[A, B, C, D](arg1: A, arg2: B, arg3: C, arg4: D)

  // This API can now be made abstract and written just once
  final class Api[NamedArgs, ArgFn[_]] {
    def asd[Z](f: NamedArgs => Z): Z = ???
    def asd[Z](f: ArgFn[Z]      ): Z = ???
  }

  // Scala 2 doesn't have type-lambdas so let's create some helper equivalents
  sealed trait Arity2FnTypeLambda[A, B      ] { type F[Z] = (A, B      ) => Z }
  sealed trait Arity3FnTypeLambda[A, B, C   ] { type F[Z] = (A, B, C   ) => Z }
  sealed trait Arity4FnTypeLambda[A, B, C, D] { type F[Z] = (A, B, C, D) => Z }

  // Now we replace the type-lambdas with something that will cross-compile with Scala 2
  type Arity2[A, B]       = Api[NamedArgs[A, B      ], Arity2FnTypeLambda[A, B      ]#F]
  type Arity3[A, B, C]    = Api[NamedArgs[A, B, C   ], Arity3FnTypeLambda[A, B, C   ]#F]
  type Arity4[A, B, C, D] = Api[NamedArgs[A, B, C, D], Arity4FnTypeLambda[A, B, C, D]#F]
}

And that leads to the minimised examples I first reported, where

  • HookComponentBuilder[Ctx, CtxFn[_]] = Api[NamedArgs, ArgFn[_]]
  • HookCtx.P1 = Arity2NamedArgs
  • HookCtxFn.P1 = Arity2FnTypeLambda
  • b.type <: Arity2[String, Int]

The problem isn't that this falls into a grey area because when I provide an inlined type-lambda Scala 3 does the correct and expected thing. The difference between this working and this not working has nothing do to with types lining up or not; the difference between this working or not is entirely determined by whether Arity2FnTypeLambda[A, B]#F or [Z] =>> (A, B) => Z is specified. We already know that Arity2FnTypeLambda[A, B]#F can be widened into [Z] =>> (A, B) => Z without contention. I suspect this is simply a case of some part of the compiler doing f =:= g instead of f <:< g, or maybe a .dealias or a .widenBlah is needed somewhere or something. IMO it should be a very small fix but the difficulty will be in finding where exactly the fix is needed :)

@japgolly
Copy link
Contributor Author

japgolly commented Jun 2, 2021

Here's a simpler and hopefully much-clearer minimisation:

final class Builder[CtxFn[_]] {
  def asd[A](f: Int => A): A = ???
  def asd[A](f: CtxFn[A]): A = ???
}

object Test {
  val b1: Builder[[Z] =>> (String, Int) => Z] = ???
  b1.asd(identity)
  b1.asd(_.length + _) // works

  sealed trait Scala2TL { type F[Z] = (String, Int) => Z }
  val b2: Builder[Scala2TL#F] = b1
  b2.asd(identity)
  b2.asd(_.length + _) // error

  type Scala3TL = [Z] =>> (String, Int) => Z
  val b3: Builder[Scala3TL] = b1
  b3.asd(identity)
  b3.asd(_.length + _) // error

  val b4: Builder[({ type F[Z] = (String, Int) => Z })#F] = b1
  b4.asd(identity)
  b4.asd(_.length + _) // works
}

@japgolly
Copy link
Contributor Author

japgolly commented Jun 2, 2021

Also did a quick test to see if all 4 representations are identical. They are:

object Test2 {
  sealed trait Scala2TL { type F[Z] = (String, Int) => Z }
  type Scala3TL = [Z] =>> (String, Int) => Z

  var b1: Builder[[Z] =>> (String, Int) => Z            ] = ???
  var b2: Builder[Scala2TL#F                            ] = ???
  var b3: Builder[Scala3TL                              ] = ???
  var b4: Builder[({ type F[Z] = (String, Int) => Z })#F] = ???

  b1 = b1
  b1 = b2
  b1 = b3
  b1 = b4

  b2 = b1
  b2 = b2
  b2 = b3
  b2 = b4

  b3 = b1
  b3 = b2
  b3 = b3
  b3 = b4

  b4 = b1
  b4 = b2
  b4 = b3
  b4 = b4

  // no errors. good.
}

So this must be just a .dealias needed somewhere in overload resolution?

@odersky odersky self-assigned this Jun 5, 2021
@odersky
Copy link
Contributor

odersky commented Jun 5, 2021

Thanks for the minimization! I will take another look.

@odersky odersky removed their assignment Dec 5, 2021
@odersky odersky added the backlog No work planned on this by the core team for the time being. label Apr 7, 2022
nicolasstucki added a commit that referenced this issue May 15, 2023
This adds in all 3 examples from the issue reported.

[skip community_build]

closes #12663
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog No work planned on this by the core team for the time being. itype:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants