Rework variances of higher-kinded types #8082

odersky · 2020-01-23T11:47:05Z

Decouple variances from type lambdas. Normally, a type lambda is just a function from types to types, it does not have a declared parameter variance. Of course, the variance of a type parameter can be determined by tracking occurrences of the parameter on the right hand side. That's a structural criterion, not
a user-defined one.

Instead of associating variances with all type lambdas, associate them only with those type lambdas

that are bounds for an abstract type, or
that are opaque aliases, or
that are match type aliases

So conceptually, the variance goes with the type alias or abstract type, not with the type lambda. In the current implementation, variances are still stored in type lambdas since it turned out to be simpler that way. But those lambdas are now classified as variant lambdas (i.e. those that are bounds or right hand sides of type declarations of the kinds listed above) and other lambdas. For those other lambdas, determine the parameter variances structurally, by looking at parameter occurrences on the right hand side.

Fixes #7993
Fixes #8049

Note: In the Type data structure, variances are stored in lambdas. But in the Tasty format they are associated with TYPEBOUNDS tags.

Note: This PR also drops the kludge of encoding lambda variances in parameter names (formerly of kind VariantName).

smarter · 2020-01-23T16:11:56Z

Some related discussions: #6369, #6320

odersky · 2020-01-27T21:44:33Z

Some related discussions: #6369, #6320

It turns out neither of these changes with the current PR.

odersky · 2020-01-27T22:16:31Z

todos:

drop TYPEALIAS tasty tag
simplify pickling of match aliases
systematically test all combinations of variances

odersky · 2020-01-29T10:22:54Z

I am done with it for now. I am sure this could be improved further spec-wise but I am out of energy and time to do more with it. The PR removes the major sources of ugliness of the previous implementation and solves type inferencing problems in existing code.

+ refactoring: account for Bivariance + refactpring: collect variance related ops in Variances, move Variances to core.

This is meant as a better alternative to encode variances in parameter names.

Encode them instead in the upper bound lambda of a TypeBounds type. For now, we also encode them in the alias of a Typealias type, but this will be dropped one we pass to structural lambda variance.

The old encoding using semantic parameter name is still in place. The new recording inside TypeBounds exists alongside the old one.

Needs to be passed as a parameter since Namer does not construct a bounds immediately.

These can hopefully be revived once the variance changes have completed

... unless they are on the right hand side of type bounds or match aliases or they are type aliases where some variance is given explicitly with a `+` or `-`.

This reverts commit 7d6e75e.

Type lambdas don't print with variances anymore

Since variances are associated conceptually with higher-kinded type variables, it makes no sense to write them on type lambdas. I believe it's better to disallow writing variances there because it will only need to variance-bike-shedding otherwise.

A refinement type would previously qualify as an `isRef` of Any, AnyKind, or AnyRef. Often that is not what was intended. Break out `isAny`, `isAnyKind`, and `isAnyRef` methods for tests that don't go through refinements.

smarter · 2020-01-29T17:02:50Z

compiler/src/dotty/tools/dotc/core/Types.scala

+          bounds.derivedAlias(expand(bounds.alias, true))
+        case bounds: TypeAlias =>
+          bounds.derivedAlias(expand(bounds.alias,
+            isOpaqueAlias | params.exists(!_.paramVariance.isEmpty)))


Suggested change

isOpaqueAlias | params.exists(!_.paramVariance.isEmpty)))

isOpaqueAlias || params.exists(!_.paramVariance.isEmpty)))

smarter · 2020-01-30T10:10:48Z

docs/docs/reference/new-types/type-lambdas-spec.md


 ## Relationship with Parameterized Type Definitions

+type F[X] <: List[F[X]]


Stray line ?

smarter · 2020-01-30T10:10:59Z

docs/docs/reference/new-types/type-lambdas-spec.md

+For instance,
+```scala
+type F2[A, +B] = A => B
+```scala


Suggested change

```scala

```

smarter · 2020-01-30T10:24:57Z

compiler/src/dotty/tools/dotc/core/Types.scala

@@ -3457,7 +3443,7 @@ object Types {
   *  @param  resultTypeExp   A function that, given the polytype itself, returns the
   *                          result type `T`.
   */
-  class HKTypeLambda(val paramNames: List[TypeName])(


The documentation above still says "Variances are encoded in parameter names"

smarter · 2020-01-30T13:12:56Z

compiler/src/dotty/tools/dotc/core/TypeComparer.scala

@@ -1951,7 +1933,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
      val t2 = distributeAnd(tp2, tp1)
      if (t2.exists) t2
      else if (isErased) erasedGlb(tp1, tp2, isJava = false)
-      else liftIfHK(tp1, tp2, op, original)
+      else liftIfHK(tp1, tp2, op, original, _ | _)


I don't get why we're doing something different with the variance depending on whether we have an AndType or an OrType, the variance position is the same. If I intersect [+X] => Cov[X] and [-X] => Contra[X] I get an invariant type parameter structurally: [X] => Cov[X] & Contra[X], but it looks like this code would make the type parameter bivariant instead.

It's because conceptually variances are attached to abstract types, not the lambdas. Say you have two abstract members

trait A { def F[-X] } trait B { def F[+X] } object O extends A, B { ... }

Here, the only legal implementation of F in O is indeed bivariant.

I have added a comment to the code.

smarter · 2020-01-30T13:36:03Z

tasty/src/dotty/tools/tasty/TastyFormat.scala

@@ -213,6 +211,10 @@ Standard-Section: "ASTs" TopLevelStat*
                  OPEN                                                             -- an open class
                  Annotation

+  Variance      = SEALED                                                           -- invariant


The implementation actually uses the STABLE flag:

Suggested change

Variance = SEALED -- invariant

Variance = STABLE -- invariant

smarter · 2020-01-30T13:36:47Z

tests/neg-custom-args/kind-projector.check

@@ -5,8 +5,8 @@
 -- Error: tests/neg-custom-args/kind-projector.scala:5:23 --------------------------------------------------------------
 5 |class Bar1 extends Foo[Either[*, *]] // error
  |                       ^^^^^^^^^^^^
-  |                       Type argument Either has not the same kind as its bound  <: [_$1] => Any
+  |                       Type argument Either has not the same kind as its bound [_$1]


This looks wrong to me, [_$1] is not a bound, it's a type parameter list.

It's the way we print bounds now. An abstract type

type A >: Nothing <: [X] => X

prints as

type A[X] <: X

Everything after the A is the bound. I think it's ultimately better this way since it is closer to the source but it might need some fine tuning.

I see, I think if the error message did not elide the Any and printed ...its bound [_$1] <: Any it'd be clearer

smarter · 2020-01-30T13:46:57Z

compiler/src/dotty/tools/dotc/core/Types.scala

+        tparams.head.givenVariance = vs.head
+        setVariances(tparams.tail, vs.tail)
+
+    override val isVariantLambda = variances.nonEmpty


I would use a more specific name highlighting that this is about lambdas with given variances.
(less important, but it would also be nicer to not overload the term "given", we could say "fixed variance" instead maybe)

Suggested change

override val isVariantLambda = variances.nonEmpty

override val isGivenVarianceLambda = variances.nonEmpty

I changed it to isDeclaredVarianceLambda

smarter · 2020-01-30T13:49:15Z

compiler/src/dotty/tools/dotc/core/Types.scala

+      case that: HKTypeLambda =>
+        paramNames.eqElements(that.paramNames)
+        && isVariantLambda == that.isVariantLambda
+        && (!isVariantLambda


I think this line and the next two are equivalent to:

Suggested change

&& (!isVariantLambda

&& givenVariances == that.givenVariances

yes, but it's faster the way it is written. iso is performance critical!

smarter · 2020-01-30T13:50:32Z

compiler/src/dotty/tools/dotc/core/Types.scala

-        case rt =>
-          expand(rt)
+    def boundsFromParams[PI <: ParamInfo.Of[TypeName]](params: List[PI], bounds: TypeBounds)(implicit ctx: Context): TypeBounds = {
+      def expand(tp: Type, useVariances: Boolean) =


Could be called useGivenVariances for extra clarity.

panacekcz · 2020-01-31T09:49:14Z

docs/docs/reference/new-types/type-lambdas-spec.md

@@ -94,6 +87,20 @@ is treated as a shorthand for
 ```scala
 [F >: Nothing <: [X] =>> Coll[X]]
 ```
+Abstract types and opaque type aliases remember the variances they were created with. So the type
+```scala
+def F2[-A, +B]


I think this should be typeF2[-A, +B] is known to be contravariant in A and covariant in B

Thanks! Fixed.

smarter

Otherwise LGTM!

smarter · 2020-01-31T10:24:45Z

compiler/src/dotty/tools/dotc/core/Types.scala

-   *  Variances are encoded in parameter names. A name starting with `+`
-   *  designates a covariant parameter, a name starting with `-` designates
-   *  a contravariant parameter, and every other name designates a non-variant parameter.
+   *  Variances are encoded in parameter names. A


Looks like a stray line that wasn't deleted

odersky force-pushed the change-lambda-variance branch from f194a86 to bd5c3b9 Compare January 23, 2020 12:59

odersky force-pushed the change-lambda-variance branch 3 times, most recently from d54b1b0 to 7d6e75e Compare January 27, 2020 11:58

odersky force-pushed the change-lambda-variance branch from caa4d32 to 2e166b7 Compare January 28, 2020 20:30

odersky marked this pull request as ready for review January 28, 2020 22:18

odersky requested a review from smarter January 29, 2020 10:22

odersky assigned smarter Jan 29, 2020

odersky added 18 commits January 29, 2020 17:26

Implement base variance ops

0d692ea

+ refactoring: account for Bivariance + refactpring: collect variance related ops in Variances, move Variances to core.

rename paramVariance -> paramVarianceSign

573c454

Add infrastructure for setting paramVariances in HKTypeLambdas

8bfdc0b

This is meant as a better alternative to encode variances in parameter names.

Abstract over paramVariance instead of paramVarianceSign

8a7abc7

Don't encode variances in names

dcff28c

Encode them instead in the upper bound lambda of a TypeBounds type. For now, we also encode them in the alias of a Typealias type, but this will be dropped one we pass to structural lambda variance.

Parse and print variances according to new scheme

d557ae1

The old encoding using semantic parameter name is still in place. The new recording inside TypeBounds exists alongside the old one.

Update kind projector check file

0eb65e5

Update repltest

ba42a23

Update signature help test

0847719

Tasty pickling of variances in TypeBounds

9fb77fd

Make useVariances tunable

50ae719

Needs to be passed as a parameter since Namer does not construct a bounds immediately.

Fix unpickling of type aliases

3bd90cd

Blacklist 3 pickling tests

91e167b

These can hopefully be revived once the variance changes have completed

Switch to structural variances for type lambdas

39657ec

Drop VariantNames

d983f4f

Treat type lambdas structurally ...

ef7bb36

... unless they are on the right hand side of type bounds or match aliases or they are type aliases where some variance is given explicitly with a `+` or `-`.

Don't treat opaque aliases structurally

f358710

Revert "Blacklist 3 pickling tests"

7c7bc58

This reverts commit 7d6e75e.

odersky added 10 commits January 29, 2020 17:26

Add fixed issue tests

ec293d9

Update check files

eef36c0

Type lambdas don't print with variances anymore

Drop adaptHkVariance

4ecf246

Simplify EtaExpand

5c9d041

Disallow variances in type lambdas

d2a2fdd

Since variances are associated conceptually with higher-kinded type variables, it makes no sense to write them on type lambdas. I believe it's better to disallow writing variances there because it will only need to variance-bike-shedding otherwise.

Further simplifications and test fixes

86cb086

Improve isRef queries against top types

da5ab63

A refinement type would previously qualify as an `isRef` of Any, AnyKind, or AnyRef. Often that is not what was intended. Break out `isAny`, `isAnyKind`, and `isAnyRef` methods for tests that don't go through refinements.

Simplify pickling of type aliases

363a05c

Don't stop at LazyRefs when computing structural variances

bfa8579

Improve variance computation and printing

eb04b8f

odersky mentioned this pull request Jan 29, 2020

Fix #8067: Don't constrain with types of wrong kinds #8130

Merged

smarter mentioned this pull request Jan 30, 2020

Make Id covariant typelevel/cats#3264

Closed

odersky added this to the 0.22 Tech Preview milestone Jan 30, 2020

smarter requested changes Jan 30, 2020

View reviewed changes

smarter assigned odersky and unassigned smarter Jan 30, 2020

Address review comments

8667689

odersky force-pushed the change-lambda-variance branch from ecfeaa7 to 8667689 Compare January 30, 2020 17:44

Add comment link to test

80c91ee

panacekcz reviewed Jan 31, 2020

View reviewed changes

Fix typos

22a7a81

smarter approved these changes Jan 31, 2020

View reviewed changes

Remove stray comment line

2396fe9

odersky merged commit f833baa into scala:master Jan 31, 2020

odersky deleted the change-lambda-variance branch January 31, 2020 14:52

bishabosha mentioned this pull request Mar 31, 2020

Support TASTy version 20.0 [Dotty 0.23] scalacenter/scala#41

Closed

smarter mentioned this pull request Apr 14, 2020

Quote pattern matching runtime spec #8687

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework variances of higher-kinded types #8082

Rework variances of higher-kinded types #8082

odersky commented Jan 23, 2020 •

edited

Loading

smarter commented Jan 23, 2020

odersky commented Jan 27, 2020

odersky commented Jan 27, 2020 •

edited

Loading

odersky commented Jan 29, 2020

smarter Jan 29, 2020

smarter Jan 30, 2020

smarter Jan 30, 2020

smarter Jan 30, 2020

smarter Jan 30, 2020

odersky Jan 30, 2020

smarter Jan 30, 2020

smarter Jan 30, 2020

odersky Jan 30, 2020

smarter Jan 30, 2020

smarter Jan 30, 2020

odersky Jan 30, 2020

smarter Jan 30, 2020

odersky Jan 30, 2020

smarter Jan 30, 2020

panacekcz Jan 31, 2020

odersky Jan 31, 2020

smarter left a comment

smarter Jan 31, 2020

	isOpaqueAlias \| params.exists(!_.paramVariance.isEmpty)))
	isOpaqueAlias \|\| params.exists(!_.paramVariance.isEmpty)))


		## Relationship with Parameterized Type Definitions

		type F[X] <: List[F[X]]

	Variance = SEALED -- invariant
	Variance = STABLE -- invariant

	override val isVariantLambda = variances.nonEmpty
	override val isGivenVarianceLambda = variances.nonEmpty

	&& (!isVariantLambda
	&& givenVariances == that.givenVariances

Rework variances of higher-kinded types #8082

Rework variances of higher-kinded types #8082

Conversation

odersky commented Jan 23, 2020 • edited Loading

smarter commented Jan 23, 2020

odersky commented Jan 27, 2020

odersky commented Jan 27, 2020 • edited Loading

odersky commented Jan 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smarter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

odersky commented Jan 23, 2020 •

edited

Loading

odersky commented Jan 27, 2020 •

edited

Loading