Skip to content

Make translucentSuperType handle match types #12153

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 3 commits into from
Apr 20, 2021

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Apr 20, 2021

This fixes several erasure related things, such as signature, or erasure itself.

Fixes #8666
Fixes #7894

@odersky
Copy link
Contributor Author

odersky commented Apr 20, 2021

Note: This is technically a binary incompatible change since it causes bridge methods to be created where none were created before. On the other hand, I have a hard time thinking of an example that compiled before and now compiles with different binary signatures.

@odersky odersky changed the title Make transluctentSuperType handle match types Make translucentSuperType handle match types Apr 20, 2021
@odersky odersky requested a review from smarter April 20, 2021 09:25
@sjrd
Copy link
Member

sjrd commented Apr 20, 2021

This is technically a binary incompatible change since it causes bridge methods to be created where none were created before.

If it's only bridges, and only added, then it's only forward binary incompatible; it's backward compatible.

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

I played a bit with it and couldn't find a binary- or tasty-incompatible difference either (but I have to say that I don't really understand in what situations we do or don't normalize match types).

@@ -4011,7 +4011,7 @@ object Types {
case tycon: TypeRef if tycon.symbol.isOpaqueAlias =>
tycon.translucentSuperType.applyIfParameterized(args)
case _ =>
superType
tryNormalize.orElse(superType)
Copy link
Member

Choose a reason for hiding this comment

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

Why do this in translucentSuperType and not superType? At the very least it means the documentation of the base translucentSuperType in https://github.com/lampepfl/dotty/blob/f618dd66a93a962c99c22eb586dbc93d7d410988/compiler/src/dotty/tools/dotc/core/Types.scala#L1891-L1892 is now incorrect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed the comment, which now also explains why we can't do it in superType.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I can see we did not normalize match types when computing the signatures and erasures of inherited members. That means we did not detect logical overrides, and that means we did not generate bridges where they would have been necessary.

Match types that can be reduced directly in signatures were reduced. For instance in

  type M[X] = X match
    case Int => String
    case _ => X

  def test(x: M[Int]): Int = ???

the signature of test is String => Int.

odersky added 3 commits April 20, 2021 14:33
This fixes several erasure related things, such as signature, or erasure itself.

Fixes scala#8666
@odersky odersky merged commit 6bafd95 into scala:master Apr 20, 2021
@odersky odersky deleted the fix-#8666 branch April 20, 2021 18:54
@liufengyun
Copy link
Contributor

This PR has a big impact on performance when compiling stdlib:

http://dotty-bench2.epfl.ch/

@odersky
Copy link
Contributor Author

odersky commented Apr 28, 2021

I tracked it down to match type simplification for tuples, and it seems in particular provablyDisjoint.

@Kordyjan Kordyjan added this to the 3.0.1 milestone Aug 2, 2023
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.

Match types used in methods cause issues with inheritance Override check does not account for match type.
5 participants