Skip to content

Regression in Scala 3.5.0-RC1 related to implicit search and match types that return type lambdas #20482

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

Closed
mrdziuban opened this issue May 27, 2024 · 6 comments · Fixed by #20268
Assignees
Labels
area:implicits related to implicits itype:bug regression This worked in a previous version but doesn't anymore

Comments

@mrdziuban
Copy link

Compiler version

3.5.0-RC1, the issue is not present in 3.4.2

Minimized code

https://scastie.scala-lang.org/mrdziuban/lnO61Y1VSSy2xu5NuyAg8w

trait WrapperType[A]

case class Foo[A]()

case class Bar[A]()

type FooToBar[D[_]] = [A] =>> D[Unit] match {
  case Foo[Unit] => Bar[A]
}

case class Test()
object Test {
  implicit val wrapperType: WrapperType[Bar[Test]] = new WrapperType[Bar[Test]] {}
}

val test = summon[WrapperType[FooToBar[Foo][Test]]]

Output

16 |val test = summon[WrapperType[FooToBar[Foo][Test]]]
   |                                                   ^
   |No given instance of type WrapperType[FooToBar[Foo][Test]] was found for parameter x of method summon in object Predef
   |
   |The following import might fix the problem:
   |
   |  import Test.wrapperType
   |

Expectation

Test.wrapperType should satisfy the implicit search of the summon call.

Interestingly, if I change FooToBar to not return a type lambda, then everything compiles correctly, i.e.

type FooToBar[D[_], A] = D[Unit] match {
  case Foo[Unit] => Bar[A]
}
// ...
val test = summon[WrapperType[FooToBar[Foo, Test]]]
@mrdziuban mrdziuban added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels May 27, 2024
@mrdziuban
Copy link
Author

A git bisect suggests that d421f88 (from #20147) is the culprit. /cc @EugeneFlesselle and @smarter in case you have any thoughts

@Gedochao Gedochao added area:implicits related to implicits regression This worked in a previous version but doesn't anymore and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Jun 3, 2024
@EugeneFlesselle EugeneFlesselle self-assigned this Jun 3, 2024
@EugeneFlesselle
Copy link
Contributor

The issue is fixed on #20268

Unfortunately, that PR now has issues with the named tuples, which happen to trigger the following problem with caching of super types:

* FIXME: The semantics of this flag are broken by the existence of `TypeVar#resetInst`,
* a non-provisional type could go back to being provisional after
* a call to `resetInst`. This means all caches that rely on `isProvisional`
* can likely end up returning stale results.
*/
def isProvisional(using Context): Boolean = mightBeProvisional && testProvisional

Really not sure what to do with that \cc @smarter

@smarter
Copy link
Member

smarter commented Jun 3, 2024

Hmm yeah I forgot about that, it seems we have to get rid of resetInst (which is only used in resetTo after a snapshot()). First we should check if using a nested TyperState instead of snapshot/resetTo fixes the issue you encounter, then we can see if that's good enough performance-wise or we need something more complicated.

@smarter
Copy link
Member

smarter commented Jun 3, 2024

Note that in theory at least resetInst shouldn't be needed during subtyping because of the check for subtypeCheckInProgress in

if ((ctx.typerState eq owningState.nn.get.uncheckedNN) && !TypeComparer.subtypeCheckInProgress)
setPermanentInst(tp)
, this flag could perhaps be generalized to always be set when calling snapshot() in a specific scope.

@EugeneFlesselle
Copy link
Contributor

I may have blamed resetInst too hastily after seeing it was resetting some things post match type reduction. Either way there seems to be something wrong with the caching of super types of applied types as the problems disappear when disabling it.

@EugeneFlesselle
Copy link
Contributor

this flag could perhaps be generalized to always be set when calling snapshot() in a specific scope

Or also during match type reduction specifically I suppose.

@Gedochao Gedochao linked a pull request Jun 10, 2024 that will close this issue
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Jun 11, 2024
tryNormalize used to not recursively check if tycon of applied type was normalizable,
this may be necessary in the case of an applied type dealiasing to a type lambda.

Fixes scala#20482
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Jun 28, 2024
tryNormalize used to not recursively check if tycon of applied type was normalizable,
this may be necessary in the case of an applied type dealiasing to a type lambda.

Fixes scala#20482
WojciechMazur pushed a commit that referenced this issue Aug 27, 2024
tryNormalize used to not recursively check if tycon of applied type was normalizable,
this may be necessary in the case of an applied type dealiasing to a type lambda.

Fixes #20482

[Cherry-picked 9465d65]
prolativ pushed a commit that referenced this issue Nov 20, 2024
tryNormalize used to not recursively check if tycon of applied type was normalizable,
this may be necessary in the case of an applied type dealiasing to a type lambda.

Fixes #20482

[Cherry-picked 9465d65]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:implicits related to implicits itype:bug regression This worked in a previous version but doesn't anymore
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants