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
Copilot

This comment was marked as resolved.

@noti0na1 noti0na1 marked this pull request as ready for review October 23, 2025 09:33
// when we try to infer the parameter type.
isFullyDefined(pt, ForceDegree.flipBottom)
def tryToInstantiateInUnion(tp: Type): Unit = tp match
case tp: OrType =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least, we should put AndOrType here to cover such cases:

def f[T, U](x: T & U): T = ???
val _: AnyRef => AnyRef = f(x => x)

@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.

Copilot

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
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.

2 participants