Skip to content

Don't ignore expected types of New. #15679

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 20, 2022
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jul 14, 2022

A problem arises if we typecheck an expression like new C() with expected type C[U] where C is defined like this

class C[X](using X)()

In this case, we'd like to propagate U as the type instance of X before we
resolve the using clause. To do this, we have to keep the expected result
type C[X] for typechecking the function part new C. Previously, that type was wrapped
in an IgnoredProto.

The problem was detected now since a class C with just the using clause and
no empty parameter clause was previously expanded to

class C[X]()(using X)

but is now expanded to

class C[X](using X)()

Under the previous expansion, we type checked and instantiated new C() before looking
for an argument of the using clause, so the problem did not arise.

Fixes #15664

@odersky
Copy link
Contributor Author

odersky commented Jul 14, 2022

It turns out an sjsJUnitTest fails without the second commit, since that one does insert an implicit conversion after a new.

This is all goes to show that what we do to deal with implicit conversions is completely crazy. It's a black art when to propagate expected types. There's often no better or worse. Almost any change we do can heal some code and break
some other code. I believe the only way to get out of this swamp is to get rid of unrestricted implicit conversions.

@prolativ prolativ self-requested a review July 20, 2022 11:19
@prolativ
Copy link
Contributor

@odersky one project in the community build is failing but this seems unrelated to your changes. I think rebasing might fix the problem

@prolativ prolativ assigned odersky and unassigned prolativ Jul 20, 2022
odersky added 2 commits July 20, 2022 13:22
A problem arises if we typecheck an expression like `new C()` with expected type
`C[U]` where `C` is
defined like this
```scala
class C[X](using X)()
```
In this case, we'd like to propagate `U` as the type instance of `X` before we
resolve the using clause. To do this, we have to keep the expected result
type `C[X]` for typechecking the function part `new C`. Previously, that type was wrapped
in an IgnoredProto.

The problem was detected now since a class C with just the using clause and
no empty parameter clause was previously expanded to
```scala
class C[X]()(using X)
```
but is now expanded to
```scala
class C[X](using X)()
```
Under the previous expansion, we type checked and `new C()` before looking
for an argument of the using clause, so the problem did not arise.
It turns out an sjsJUnitTest fails otherwise, since that one _does_ insert an implicit conversion after a `new`.

This is all goes to show that what we do to deal with implicit conversions
is completely crazy. It's a black art when to propagate exected types. There's
often no better or worse. Almost any change we do can heal some code and break
some other code. I believe the only way to get our of this swamp is to get
rid of unrestricted implicit conversions.
@odersky odersky merged commit b106de4 into scala:main Jul 20, 2022
@odersky odersky deleted the fix-15664 branch July 20, 2022 13:06
@Kordyjan Kordyjan added the backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" label Jul 26, 2022
@Kordyjan Kordyjan added this to the 3.2.0 backports milestone Jul 26, 2022
@Kordyjan Kordyjan added backport:done This PR was successfully backported. and removed backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" labels Jul 26, 2022
Kordyjan added a commit that referenced this pull request Jul 27, 2022
Backport #15679: Don't ignore expected types of `New`
@Kordyjan Kordyjan modified the milestones: 3.2.0 backports, 3.2.1 Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:done This PR was successfully backported.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression in inference of implicit arguments
3 participants