Skip to content

Conversation

@noti0na1
Copy link
Member

@noti0na1 noti0na1 commented Oct 22, 2025

This PR improves type inference for functions with unknown parameter types when the expected type contains type variables within union types. Previously, the compiler would only attempt to instantiate top-level TypeVars, but now it recursively searches through union types (OrType) and flexible types to find and instantiate nested type variables.

@noti0na1 noti0na1 changed the title Try to instantiate pt if this is possible by looking into parts Try to instantiate TypeVars inside pt when possible Oct 23, 2025
@noti0na1 noti0na1 requested a review from Copilot October 23, 2025 09:31

This comment was marked as resolved.

@noti0na1 noti0na1 marked this pull request as ready for review October 23, 2025 09:33
@hamzaremmal
Copy link
Member

hamzaremmal commented Oct 23, 2025

@noti0na1 I've been working on this a bit before seeing this work and I see that we reached the same conclusion for where the fix should be. In my opinion, this approach constraints too much inference when using union (could be extended to intersections). If we have a lambda as a first parameter, we will basically already instantiate now the type parameters even if we can refine them later. For example, in this PR, we cannot do the following:

class Box[T] val box: Box[Unit] = ??? def f[T, U](x: T | U, y: Box[U]): T = ??? val _: Any => Any = f(x => x, box)

I'm not sure yet how to find the best compromise to also make this work.

@hamzaremmal
Copy link
Member

I would argue we should probably use stripNull for now for the sake of better compatibility with explicit nulls and without the side effect of too much instantiation of type variables. We can improve later type inference to cover more scenarios. It is better than #24230 in my opinion.

@noti0na1
Copy link
Member Author

Even if we fix the inference issue, I don't we can keep Option.apply nullable, since it will still break the macro writer by users.

@hamzaremmal
Copy link
Member

hamzaremmal commented Oct 23, 2025

Even if we fix the inference issue, I don't we can keep Option.apply nullable, since it will still break the macro writer by users.

And they will have to migrate to it. That very specific thing should not be considered (again in my opinion) the blocker to have the right signature for Option.apply. Fixing inference is the most important part in that issue.

This comment was marked as outdated.

hamzaremmal added a commit that referenced this pull request Oct 24, 2025
…mNullable` (#24238) Yes, this is not the most ideal signature for `Option.apply` but it allows at least to drop `Option.fromNullable` and an easier migration to explicit nulls (no need to add `fromNullable` everywhere). Superseed #24231 in the sense where we focus here on the nullability issue rather than the more global issue. Reverts #24230 Relates to #24206
@noti0na1 noti0na1 force-pushed the fix-function-type-inference-in-union branch from 8840779 to 201c25a Compare October 29, 2025 13:27
@noti0na1 noti0na1 requested a review from odersky October 30, 2025 12:06
@odersky odersky assigned noti0na1 and unassigned odersky Nov 2, 2025
@noti0na1 noti0na1 force-pushed the fix-function-type-inference-in-union branch from 2d0db85 to d71d682 Compare November 3, 2025 13:08
@noti0na1 noti0na1 enabled auto-merge November 3, 2025 13:10
@noti0na1 noti0na1 merged commit fb8fddc into scala:main Nov 3, 2025
53 checks passed
@noti0na1 noti0na1 deleted the fix-function-type-inference-in-union branch November 3, 2025 17:25
noti0na1 added a commit that referenced this pull request Dec 12, 2025
This PR refines the `TypeVar` instantiation logic introduced by #24231. We only check the `calleeType` when there is actually a `TypeVar` in `pt` to instantiate now, prioritizing inference from the callee. Close #24686 #24689 #24694 #24696 Maybe #24695, since the behaviour is the same as 3.3 and 3.7 now. `calleeType` itself still has some issues, will fix in a separate PR: #24732
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants