Skip to content

Fix #18763: Run type inference before implicit search#23020

Draft
Alex1005a wants to merge 70 commits intoscala:mainfrom
Alex1005a:fix-18763
Draft

Fix #18763: Run type inference before implicit search#23020
Alex1005a wants to merge 70 commits intoscala:mainfrom
Alex1005a:fix-18763

Conversation

@Alex1005a
Copy link
Copy Markdown
Contributor

Fixes #18763
It seems that the problem is that searching for an instance of Functor[F] for the implicit conversion toFunctorOps tries all possible variants, because F is not constrained in any way. After failure in the case of AmbiguousImplicits, type inference occurs (F is constrained to IO) and the required instance is found. Inferring types before searching for an instance will help to avoid such cases.

@Alex1005a
Copy link
Copy Markdown
Contributor Author

The fact that type inference only happens when an "ambiguous implicit" error occurs can lead to weird things.

import cats.effect.IO
import cats.implicits.*
import cats.effect.kernel.Async
import cats.Monad

case class SomeF[F[_], A]()

object SomeF {
  implicit def asyncForSomeF[F[_]](implicit F: Async[F]): Async[[A] =>> SomeF[F, A]] = ???

  implicit def monadForSomeF1[F[_]]: Monad[[A] =>> SomeF[F, A]] = ???

  implicit def monadForSomeF2[F[_]]: Monad[[A] =>> SomeF[F, A]] = ???

  def unit[F[_]]: SomeF[F, Unit] = SomeF()
}

val test: SomeF[IO, Unit] = toFunctorOps(SomeF.unit).as(???) // toFunctorOps can be omitted

In this code, for toFunctorOps the argument asyncForSomeF will be inferred. If you remove one of the Monad implicit, the remaining one will be inferred (for example, if you remove monadForSomeF2, then monadForSomeF1 will be inferred). If you remove both Monad implicit, then an error will be output that implicit not found.
If type inference was before implicit search, there would not be such strange behavior.

@Alex1005a
Copy link
Copy Markdown
Contributor Author

Tracing profiles of the program without and with optimization.
Without optimization:
without optimization
With optimization:
with optimization

@Alex1005a Alex1005a marked this pull request as ready for review April 29, 2025 17:34
@Gedochao Gedochao requested a review from noti0na1 April 29, 2025 19:55
@odersky
Copy link
Copy Markdown
Contributor

odersky commented May 26, 2025

@Alex1005a, thanks for the contribution! The interaction of type inference and implicit search is tricky, since implicit search might be intended to instantiate some variables and inferring these variables beforehand might lose solutions. So a solution for one set of programs often causes failures in other programs. The problem is to find a heuristic that works uniformly better. @EugeneFlesselle, you worked on similar issues before, can you take a look at this?

@EugeneFlesselle
Copy link
Copy Markdown
Contributor

@EugeneFlesselle, you worked on similar issues before, can you take a look at this?

Yes, of course, I'll check it out

# Conflicts:
#	compiler/src/dotty/tools/dotc/typer/Typer.scala
@Gedochao Gedochao marked this pull request as draft March 3, 2026 14:06
@Gedochao
Copy link
Copy Markdown
Contributor

Gedochao commented Mar 3, 2026

@Alex1005a so 70 commits looks a bit crazy 😅
What's the status of this PR? Are you still working on this?

@Alex1005a
Copy link
Copy Markdown
Contributor Author

I think this can be closed after merge #25109 or some other fix for problematic diverging implicits. This will fix the original issue.
There is a potential fix #24824 in this PR, but it can be done in a separate PR.

@Gedochao
Copy link
Copy Markdown
Contributor

Gedochao commented Mar 4, 2026

I think this can be closed after merge #25109 or some other fix for problematic diverging implicits. This will fix the original issue. There is a potential fix #24824 in this PR, but it can be done in a separate PR.

sorry, I got confused here.
Should we close this and track it elsewhere or not?

@Alex1005a
Copy link
Copy Markdown
Contributor Author

Alex1005a commented Mar 4, 2026

sorry, I got confused here.
Should we close this and track it elsewhere or not?

This PR will not be relevant and may be closed when the issue with diverging implicit search in another PR is resolved.

dancewithheart added a commit to dancewithheart/scala3 that referenced this pull request Mar 31, 2026
Based on work by @Alex1005a in scala#23020.

The compiler sometimes builds a FunProtoTyped (a function prototype where
arguments are already typed), but later transformations (e.g. in
wildApprox and FunProto.map) can turn it back into a FunProto.

This is a problem because:
- FunProto assumes arguments are not typed yet
- FunProtoTyped assumes they already are

Switching between these representations breaks internal assumptions and
can lead to crashes, e.g. in `collectParts`:
  case t: TypeParamRef =>
    assert(!ctx.typerState.constraint.contains(t),
      i"`wildApprox` failed to remove uninstantiated $t")

This change preserve the FunProtoTyped representation:
- wildApprox(FunProto) now always returns FunProtoTyped
- FunProtoTyped.map now preserves the subtype via derivedFunProtoTyped

Background:
- scala#17305 (discussion around wildApprox)
- scala#17471 by @andrzejressel (with review comments by @odersky)
- scala#23020 by @Alex1005a (handling of FunProtoTyped)

Signed-off-by: Piotr Paradzinski <dancingwithopenheart@gmail.com>
@dancewithheart
Copy link
Copy Markdown
Contributor

dancewithheart commented Mar 31, 2026

Hi @Alex1005a, thanks for your work on this

There is a potential fix #24824 in this PR, but it can be done in a separate PR.

While investigating #24824, I ended up using your approach - I copied the way you handle FunProtoTyped in wildApprox and map. It seems to be enough to fix the reported crash.

I’m currently using #25662 as a small playground to validate that this is indeed the correct fix.

If you’d like to extract a minimal fix for 24824 from your PR,
I’m totally happy to drop mine and help instead 🙂

@Alex1005a
Copy link
Copy Markdown
Contributor Author

Hi @dancewithheart. Glad to see a PR with the proposed fix! I wasn’t planning to work on this issue, so I’d be happy if you take care of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Slow performance in a combination of features

6 participants