Fix #7139: Implement kind-projector compatibility #7775

travisbrown · 2019-12-16T08:45:23Z

This change adds support for a subset of kind-projector's syntax behind the existing -Ykind-projector flag (closing #7139).

It supports the following kind-projector features:

* as a placeholder (Functor[Either[L, *]] is equivalent to Functor[[x] => Either[L, x]]).
* as a placeholder in tuple types (Functor[(A, *)] is equivalent to Functor[[x] => (A, x)]).
* as a placeholder in function types (both Functor[S => *] and Functor[* => T] work).
λ syntax for naming parameters ( Functor[λ[x => (x, x)]] for Functor[[x] => (x, x)]).

There are a few things kind-projector provides that this change doesn't support:

? as a placeholder (since it collides with wildcards).
* as a placeholder in infix types.
Lambda as an alternative for λ.
λ arguments of a kind other than * (e.g. λ[f[_] => Functor[f]] isn't supported).
Variance annotations on either * or λ arguments.
Polymorphic lambda values (λ[Vector ~> List](_.toList)).

I've prioritized supporting the kind-projector features needed in Cats, and while we do use some of these features there, it's pretty easy to make small changes to avoid them without hurting readability (in most cases thse changes improve readability it, in my view—e.g. getting rid of λ[`-α` => ...]).

The changes here have no effect on parsing if -Ykind-projector isn't enabled. ~~I enabled it generally in the test configuration since I didn't see a way to enable it for specific test files.~~

dottybot

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

smarter · 2019-12-16T11:41:07Z

I enabled it generally in the test configuration since I didn't see a way to enable it for specific test files.

You'll need to add your test files in tests/pos-special and tests/neg-custom-args, then specify which flags to pass like this: https://github.com/lampepfl/dotty/blob/b8449ae956fbfcefaa50283291f7ba3b233303f4/compiler/test/dotty/tools/dotc/CompilationTests.scala#L58 https://github.com/lampepfl/dotty/blob/b8449ae956fbfcefaa50283291f7ba3b233303f4/compiler/test/dotty/tools/dotc/CompilationTests.scala#L127

travisbrown · 2019-12-16T11:45:45Z

@smarter Thanks! Would it be better for me to do that? It seems like there's also value in confirming that the changes behind the flag don't break any existing tests.

smarter · 2019-12-16T11:48:32Z

It seems like there's also value in confirming that the changes behind the flag don't break any existing tests.

Yeah it's a trade-off, but I think it's better to not enable special language features by default in the tests, since that potentially decreases the coverage of the code path that most people will run.

travisbrown · 2019-12-16T11:56:26Z

@smarter Sounds reasonable, thanks. I've just pushed the change.

smarter · 2019-12-16T12:02:10Z

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

+
+      val newParams = params.map {
+        case Ident(tpnme.raw.STAR) =>
+          val name = tpnme.syntheticTypeParamName(tparams.length)


To make sure there's no weird shadowing issues, it's probably better to use fresh names like we do for term parameters:

Suggested change

val name = tpnme.syntheticTypeParamName(tparams.length)

val name = WildcardParamName.fresh().toTypeName

smarter · 2019-12-16T12:03:14Z

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

+    private def replaceKindProjectorPlaceholders(params: List[Tree]): (List[Tree], List[TypeDef]) = {
+      val tparams = new ListBuffer[TypeDef]
+
+      val newParams = params.map {


To reuse the same list if possible:

Suggested change

val newParams = params.map {

val newParams = params.mapConserve {

smarter · 2019-12-16T12:04:53Z

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

+            if (tparams.isEmpty) {
+              Function(params, t)
+            } else {
+              LambdaTypeTree(tparams, Function(newParams, newT))
+            }


I think this is equivalent to:

lambdaAbstract(tparams, Function(newParams, newT))

(lambdaAbstract could also be used in a couple of other places)

I was using LambdaTypeTree directly to avoid using the new params, etc. when they weren't needed, and in the tuple case it lets us avoid instantiating a new Tuple value:

if (tparams.isEmpty) { t } else { LambdaTypeTree(tparams, Tuple(newParams)) }

I'm changing the other two now, and am happy to change the tuple one as well if you'd prefer.

smarter · 2019-12-16T12:12:26Z

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

+      val tparams = new ListBuffer[TypeDef]
+
+      val newParams = params.map {
+        case Ident(tpnme.raw.STAR) =>


Name the parameter to be used in another suggestion below:

Suggested change

case Ident(tpnme.raw.STAR) =>

case param @ Ident(tpnme.raw.STAR) =>

smarter · 2019-12-16T12:13:38Z

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

+      val newParams = params.map {
+        case Ident(tpnme.raw.STAR) =>
+          val name = tpnme.syntheticTypeParamName(tparams.length)
+          tparams += TypeDef(name, TypeBoundsTree(EmptyTree, EmptyTree))


set the Param flag to make sure this is handled as a type parameter

set the position ("span") of the constructed tree to match the position of the user-written *, for better error messages and IDE support.

Suggested change

tparams += TypeDef(name, TypeBoundsTree(EmptyTree, EmptyTree))

tparams += TypeDef(name, TypeBoundsTree(EmptyTree, EmptyTree))

.withFlags(Param).withSpan(param.span)

There's another use of TypeDef below which should do something similar (or even better, be refactored so there's only one point where TypeDef is used)

I don't see a way to use the * span without ending up with a "child positions in wrong order" error:

java.lang.AssertionError: assertion failed: position error, child positions overlap or in wrong order parent = [_$5] =>> Either[Int, _$5] 1st child = _$5 1st child span = [117..118] 2nd child = Either[Int, _$5] 2nd child span = <105..115> while compiling tests/pos-special/kind-projector.scala Fatal compiler crash when compiling: tests/pos-special/kind-projector.scala: assertion failed: position error, child positions overlap or in wrong order parent = [_$5] =>> Either[Int, _$5] 1st child = _$5 1st child span = [117..118] 2nd child = Either[Int, _$5] 2nd child span = <105..115> dotty.DottyPredef$.assertFail(DottyPredef.scala:17) dotty.tools.dotc.ast.Positioned.check$1(Positioned.scala:183)

It seems like in this case it just might not be possible to point to the user-written *?

travisbrown · 2019-12-16T12:50:52Z

@smarter Thanks for the comments! I've made the changes you suggest except for two things (also noted above):

I'm using LambdaTypeTree directly (instead of lambdaAbstract) for the tuple case where that lets us avoid instantiating a new Tuple.
I'm only setting the span on the TypeDef in the λ-argument case, since for * it ends up getting positions in the wrong order.

travisbrown · 2019-12-17T11:55:11Z

@smarter Let me know if there's anything else I can do here, and thanks again!

Katrix · 2019-12-18T12:20:41Z

Isn't Functor[Either[L, *]] being equivalent to [x] => Functor[Either[L, x]] the wrong behavior? Shouldn't it instead be equivalent to Functor[[x] => Either[L, x]]?

It's spelled out pretty explicitly as a gotcha on the docs too. https://github.com/typelevel/kind-projector/blob/master/README.md#type-lambda-gotchas

travisbrown · 2019-12-18T12:30:39Z

@Katrix Sorry, that was me mistyping in the description. The actual behavior is as you describe. For example, if you compile this with -Ykind-projector -Xprint:parser:

trait Functor[F[_]]

object Functor {
  def eitherFunctor[L]: Functor[[x] =>> Either[L, x]] = ???
  def otherEitherFunctor[L]: Functor[Either[L, *]] = ???
}

You get this:

parsed:
package <empty> {
  trait Functor[F[_$1]] {}
  module object Functor {
    def eitherFunctor[L]: Functor[[x] =>> Either[L, x]] = ???
    def otherEitherFunctor[L]: Functor[[_$2] =>> Either[L, _$2]] = ???
  }
}

I've fixed the description. Thanks for catching that.

travisbrown · 2020-01-16T11:26:13Z

@smarter I don't mean to sound impatient, but please let me know if it'd be useful for me to merge master or if there's anything else I can do here. Thanks!

smarter · 2020-01-16T13:42:25Z

Don't worry about merging with master, it's fine as is. Sorry for the wait, I keep getting distracted trying to fix the issues you're reporting :).

smarter

LGTM with one minor nit, sorry for the delay in review!

Longer term, I think we should try handling type placeholders in the same way we handle term placeholders currently (see the var placeholderParams that is used as a stack). This would avoid having to special case applied types, tuples, infix types, etc. This isn't a big deal for now since placeholder params are just a compatibility thing, but in the future we'd like to have them by default with one syntax or another: #5379

smarter · 2020-01-22T21:20:04Z

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

@@ -1413,6 +1420,29 @@ object Parsers {
      }
    }

+    private def makeKindProjectorTypeDef(name: TypeName, span: Option[Span]): TypeDef = {


I think we should drop the span parameter and just have call-sites that need to set a span do it themselves (makeKindProjectorTypeDef(...).withSpan(...)), it's not much longer and slightly clearer.

Agreed—I've just pushed the change.

smarter · 2020-01-22T22:29:38Z

Sorry, you'll need to rebase to pick up some recent CI changes fixing dependency resolution (while you're at it, please squash these commits together since they represent a single change, and maybe reuse the PR description as a commit message since it's more detailed).

This change adds support for a subset of kind-projector's syntax behind the existing -Ykind-projector flag. It supports the following kind-projector features: - * placeholder (Functor[F[L, *]] instead of Functor[[x] => F[L, x]]). - * in tuple types (Functor[(A, *)] instead of Functor[[x] => (A, x)]). - * in function types (both Functor[S => *] and Functor[* => T] work). - λ syntax (Functor[λ[x => (x, x)]] for Functor[[x] => (x, x)]). There are a few things kind-projector provides that the flag doesn't: - ? as a placeholder (since it collides with wildcards). - * as a placeholder in infix types. - Lambda as an alternative for λ. - λ arguments of a kind other than * (no λ[f[_] => Functor[f]]). - Variance annotations on either * or λ arguments. - Polymorphic lambda values (λ[Vector ~> List](_.toList)). The changes have no effect on parsing if -Ykind-projector isn't enabled.

travisbrown · 2020-01-23T10:15:03Z

@smarter Done!

prepare dotty. dotty "-Ykind-projector" does not support higher kind scala/scala3#7775 > λ arguments of a kind other than * (e.g. λ[f[_] => Functor[f]] isn't supported).

dotty `-Ykind-projector` does not support this patterns scala/scala3#7775

djspiewak · 2020-05-25T03:18:14Z

For posterity…

There are a few cases which are remarkably annoying to port from kind-projector syntax to -Ykind-projector due to the omissions in this implementation. Specifically, something like this:

trait Bar[R[_[_], _], F[_], E]
trait Baz[F[_], E]

def foo[R[_[_], _], F[_], E]: Bar[λ[(f[_], a) => Baz[R[f, ?], a]], F, E] = ???

The only way I could find to meaningfully port this was to use an OG type lambda:

type BazLam[R[_[_], _]] = { type L[F[_], A] = Baz[R[F, *], A] }
def foo[R[_[_], _], F[_], E]: Bar[BazLam[R]#L, F, E] = ???

This could would be a lot easier to write if it were Scala 2 only, or Scala 3 only, but since it's cross-compiling it becomes a bit of a headache.

smarter · 2020-05-25T13:35:19Z

@djspiewak We welcome changes to -Ykind-projector to make it support more complicated cases of kind-projector type lambda syntax if it's useful for cross-compilation. though if these usages of kind-pjector are rare enough in your codebase, maybe defining some type aliases would be good enough?

djspiewak · 2020-05-25T17:00:42Z

though if these usages of kind-pjector are rare enough in your codebase, maybe defining some type aliases would be good enough?

I'm currently tracking the pain meter. :-) If it reaches a certain point, I'll bite the bullet and PR improvements to make it easier. Honestly the more painful thing is losing the value-level syntax, since that's basically everywhere in all of my codebases, but I'm under the impression that is somewhat more complicated to achieve.

djspiewak · 2020-05-25T23:58:10Z

(as an aside, https://gist.github.com/02019a4f1c83f7b7b3978964b9ed06ab almost works for shimming the value level syntax; the only real problem is that the call site inference for polyfunctions is rather limited)

travisbrown · 2020-05-26T04:57:47Z

For what it’s worth I have a Scalafix rule that desugars the value-level syntax here.

smarter · 2020-05-26T11:58:15Z

call site inference for polyfunctions is rather limited

Yeah that's on my todo list.

dottybot reviewed Dec 16, 2019

View reviewed changes

odersky requested a review from smarter December 16, 2019 09:24

odersky assigned smarter Dec 16, 2019

travisbrown mentioned this pull request Dec 16, 2019

Replace kind-projector placeholders in infix types typelevel/cats#3215

Merged

smarter requested changes Dec 16, 2019

View reviewed changes

travisbrown mentioned this pull request Jan 22, 2020

Some small changes for Dotty compatibility typelevel/cats-effect#757

Merged

smarter approved these changes Jan 22, 2020

View reviewed changes

travisbrown force-pushed the topic/kind-projector-compat branch from 880f5f4 to b5adfcb Compare January 23, 2020 10:14

odersky merged commit 709a3c7 into scala:master Jan 23, 2020

travisbrown mentioned this pull request Jan 24, 2020

Dotty cross-building [WIP] typelevel/cats#3269

Closed

14 tasks

travisbrown mentioned this pull request Mar 10, 2020

Dotty preparation http4s/http4s#3242

Closed

xuwei-k added a commit to scalaz/scalaz that referenced this pull request May 16, 2020

avoid kind-projector with variance and {upper, lower} type bounds

662e704

dotty `-Ykind-projector` does not support this patterns scala/scala3#7775

neko-kai mentioned this pull request May 5, 2021

Support variance annotations in -Ykind-projector mode #12341

Merged

Daenyth mentioned this pull request Jan 25, 2022

Extend kind-projector support lampepfl/dotty-feature-requests#285

Closed

smarter mentioned this pull request Nov 19, 2022

Missing Ykind-projector usecase that prevent cross-compilation with Scala 2 : higher-kind-parameterised type-lambdas #17672

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #7139: Implement kind-projector compatibility #7775

Fix #7139: Implement kind-projector compatibility #7775

travisbrown commented Dec 16, 2019 •

edited

Loading

dottybot left a comment

smarter commented Dec 16, 2019

travisbrown commented Dec 16, 2019

smarter commented Dec 16, 2019

travisbrown commented Dec 16, 2019

smarter Dec 16, 2019

smarter Dec 16, 2019

smarter Dec 16, 2019

travisbrown Dec 16, 2019

smarter Dec 16, 2019

smarter Dec 16, 2019

travisbrown Dec 16, 2019

travisbrown commented Dec 16, 2019

travisbrown commented Dec 17, 2019

Katrix commented Dec 18, 2019

travisbrown commented Dec 18, 2019

travisbrown commented Jan 16, 2020

smarter commented Jan 16, 2020

smarter left a comment

smarter Jan 22, 2020

travisbrown Jan 22, 2020

smarter commented Jan 22, 2020

travisbrown commented Jan 23, 2020

djspiewak commented May 25, 2020

smarter commented May 25, 2020

djspiewak commented May 25, 2020

djspiewak commented May 25, 2020

travisbrown commented May 26, 2020

smarter commented May 26, 2020

	val name = tpnme.syntheticTypeParamName(tparams.length)
	val name = WildcardParamName.fresh().toTypeName

	val newParams = params.map {
	val newParams = params.mapConserve {

	case Ident(tpnme.raw.STAR) =>
	case param @ Ident(tpnme.raw.STAR) =>

	tparams += TypeDef(name, TypeBoundsTree(EmptyTree, EmptyTree))
	tparams += TypeDef(name, TypeBoundsTree(EmptyTree, EmptyTree))
	.withFlags(Param).withSpan(param.span)

Fix #7139: Implement kind-projector compatibility #7775

Fix #7139: Implement kind-projector compatibility #7775

Conversation

travisbrown commented Dec 16, 2019 • edited Loading

dottybot left a comment

Choose a reason for hiding this comment

smarter commented Dec 16, 2019

travisbrown commented Dec 16, 2019

smarter commented Dec 16, 2019

travisbrown commented Dec 16, 2019

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

travisbrown commented Dec 16, 2019

travisbrown commented Dec 17, 2019

Katrix commented Dec 18, 2019

travisbrown commented Dec 18, 2019

travisbrown commented Jan 16, 2020

smarter commented Jan 16, 2020

smarter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smarter commented Jan 22, 2020

travisbrown commented Jan 23, 2020

djspiewak commented May 25, 2020

smarter commented May 25, 2020

djspiewak commented May 25, 2020

djspiewak commented May 25, 2020

travisbrown commented May 26, 2020

smarter commented May 26, 2020

travisbrown commented Dec 16, 2019 •

edited

Loading