Skip to content
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

Fix variance handling for parameterized type aliases #8651

Merged
merged 1 commit into from Feb 10, 2020

Conversation

smarter
Copy link
Member

@smarter smarter commented Jan 21, 2020

The variance of a type alias was taken to be the variance of its
right-hand side, but that doesn't make sense for a parameterized type
alias which is free to appear in any position: variance checking should
only kick in when it is applied to something.

It was possible to work around this by using a type lambda instead of a
type alias, cats just had to do that: typelevel/cats#3264

@scala-jenkins scala-jenkins added this to the 2.13.2 milestone Jan 21, 2020
The variance of a type alias was taken to be the variance of its
right-hand side, but that doesn't make sense for a parameterized type
alias which is free to appear in any position: variance checking should
only kick in when it is applied to something.

It was possible to work around this by using a type lambda instead of a
type alias, cats just had to do that: typelevel/cats#3264
@SethTisue

This comment has been minimized.

@lrytz lrytz requested a review from adriaanm January 22, 2020 13:58
Copy link
Contributor

@adriaanm adriaanm left a comment

Choose a reason for hiding this comment

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

Thanks!

override def variance =
// A non-applied parameterized type alias can appear in any variance position
if (typeParams.nonEmpty)
Invariant
Copy link
Contributor

Choose a reason for hiding this comment

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

looks sound to me :-)

else if (isLocalToThis)
Bivariant
else
info.typeSymbol.variance
Copy link
Contributor

Choose a reason for hiding this comment

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

I can no longer construct the soundness argument for this; but, then again, I also don't have a counter-example. (And it's been like this forever 🤷‍♂ )

@smarter

This comment has been minimized.

@SethTisue

This comment has been minimized.

@SethTisue

This comment has been minimized.

@lrytz

This comment has been minimized.

@SethTisue

This comment has been minimized.

@SethTisue
Copy link
Member

SethTisue commented Feb 6, 2020

green community build run: https://scala-ci.typesafe.com/job/scala-2.13.x-integrate-community-build/3148/

@lrytz
Copy link
Member

lrytz commented Feb 10, 2020

Yay

@lrytz lrytz merged commit 1bde691 into scala:2.13.x Feb 10, 2020
@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Feb 10, 2020
@SethTisue SethTisue changed the title Fix variance computation for parameterized type aliases Fix variance handling for parameterized type aliases Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
6 participants