Skip to content

Refine implicit search fallbacks for better ClassTag handling #23532

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

Merged
merged 2 commits into from
Jul 18, 2025

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jul 15, 2025

  1. Don't use FullyDefinedType when synthesizing ClassTags

It's not necessary to instantiate all type variables, deeply, since we are only interested in the outermost shape. Also, that way we do not instantiate to Nothing, which is something difficult to recover from. This step is necessary to enable (2)

  1. Constrain by deepening expected result types in more implicit searches

We used to look at the deep expected type when the result of an implicit was ambiguous.
This could add more constraints which could resolve the ambiguity.

We now do the same also if the search type has uninstantiated type variables. In that
case, consulting more context might further constrain type variables, which might in
turn enlarge the implicit scope so that a solution can be found.

Fixes #23526

odersky added 2 commits July 15, 2025 17:44
It's not necessary to instantiate all type variables, deeply, since
we are only interested in the outermost shape. Also, that way we do
not instantiate to Nothing, which is something difficult to recover from.
We used to look at deep expected type when the result of an implicit was ambiguous.
This could add more constraints which could resolve the ambiguity.

We now do the same also if the search type has uninstantiated type variables. In that
case, consulting more context might further constrain type variables, which might in
turn enlarge the implicit scope so that a solution can be found.

Fixes scala#23526
@odersky odersky changed the title Don't use FullyDefinedType when synthesizing ClassTags Refine implicit search fallbacks for better ClassTag handling Jul 15, 2025
@odersky odersky requested a review from lrytz July 16, 2025 21:00
Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

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

Thanks, nice explanation in the PR description!

@odersky odersky merged commit 73b935c into scala:main Jul 18, 2025
76 of 89 checks passed
@odersky odersky deleted the fix-23526 branch July 18, 2025 10:33
@WojciechMazur WojciechMazur added this to the 3.7.3 milestone Jul 29, 2025
odersky added a commit to dotty-staging/dotty that referenced this pull request Aug 5, 2025
scala#23609 triggers an assertion error in TyperState. The relevant explanation seems to be
in ProtoTypes.scala:
```scala
            // To respect the pre-condition of `mergeConstraintWith` and keep
            // `protoTyperState` committable we must ensure that it does not
            // contain any type variable which don't already exist in the passed
            // TyperState. This is achieved by instantiating any such type
            // variable. NOTE: this does not suffice to discard type variables
            // in ancestors of `protoTyperState`, if this situation ever
            // comes up, an assertion in TyperState will trigger and this code
            // will need to be generalized.
```
We should go to the bottom of it and fix the assertion. But before that's done this PR
offers a temporary hack to catch the exception when it is triggered from a new code path
created by PR scala#23532. This should fix the regression reported in scala#23609. We should leave
the issue open as a reminder that we still need a better fix.

Also: handle crash to missing span in a migration helper.
WojciechMazur pushed a commit that referenced this pull request Aug 5, 2025
#23609 triggers an assertion error in TyperState. The relevant
explanation seems to be in ProtoTypes.scala:
```scala
            // To respect the pre-condition of `mergeConstraintWith` and keep
            // `protoTyperState` committable we must ensure that it does not
            // contain any type variable which don't already exist in the passed
            // TyperState. This is achieved by instantiating any such type
            // variable. NOTE: this does not suffice to discard type variables
            // in ancestors of `protoTyperState`, if this situation ever
            // comes up, an assertion in TyperState will trigger and this code
            // will need to be generalized.
```
We should go to the bottom of it and fix the assertion. But before
that's done this PR offers a temporary hack to catch the exception when
it is triggered from a new code path created by PR #23532. This should
fix the regression reported in #23609. We should leave the issue open as
a reminder that we still need a better fix.

Also: handle crash due to missing span in a migration helper.
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.

ClassTag inference fails with method call chain
3 participants