Added a :kind command to the REPL #2340

Merged
merged 1 commit into from May 17, 2013

Projects

None yet

8 participants

@folone
Contributor
folone commented Mar 31, 2013

Description

Because Scala supports working with higher kinded types, we might want to be able to inspects kinds as well as types. This pull request adds a simple :kind command, implemented by @eed3si9n in his "Introduction to Scalaz: day 4" post, to the Scala REPL. Here is an example output:

scala> :kind scala.Option
Option's kind is * -> *

scala> :k scalaz.Unapply
Unapply's kind is ((* -> *) -> *) -> * -> *

scala> import scalaz._
import scalaz._

scala> :k Monad // Finds locally imported types.
Monad's kind is (* -> *) -> *
This is a type constructor that takes type constructor(s): a higher-kinded type.

scala> :k Nonexisting
<console>:14: error: not found: value Nonexisting
              Nonexisting
              ^

scala> class Foo
defined class Foo

scala> new Foo { def empty = true }
res0: Foo{def empty: Boolean} = $anon$1@786aceba

scala> :k res0
Foo{def empty: Boolean}'s kind is *

Future improvements

As pointed out by @retronym here, variance and type bounds are also part of the kind and we might want to include them in command's output.

Acknowledgements

The function for :kind feature is originally implemented by @eed3si9n
I got a lot of help from @xeno-by and @retronym while working on this feature, thank you for that.

Happy Easter everyone!

@folone
Contributor
folone commented Mar 31, 2013

I probably have to somehow assign this pull request to @paulp, but it looks like I don't have permissions to do that.

@retronym
Member

Two things to address before we can consider this:

  1. Please add test cases. See test/files/run/constant-type.scala as an example. Use ./test/partest --update-check --show-log to create the .check file with expected output.
  2. I'd like to find a Scala syntax for expressing the kinds. What's wrong with: :k Option \n Option[A] ?

Oh, and happy Easter, too! 🐰

@eed3si9n
Member

@retronym

What's wrong with: :k Option \n Option[A] ?

Using Scala syntax does add the ability to show variance and bounds and we might even get it for free from simply displaying the signature from Type. On the other hand, this would return kinds for Array as Array[A], which is different kind from Option[A], but they are both * -> *.

Anonymizing them both as F[A] I think is better. Not sure if there are conventions on the letters beyond the first two, but we could do something like: A, F, X, Y, Z, O, P, Q, ...; and number them as F[A1, A2].

@folone
Contributor
folone commented Mar 31, 2013

@retronym @eed3si9n I'm also not sure if things like Unapply[TC[_[_]], MA] are more readable/understandable than ((* -> *) -> *) -> * -> *

@eed3si9n
Member

@folone For the majority of Scala users (assuming without any Haskell background) F[A1, A2] and X[F[A]] are probably easier to understand than the curried notation of * -> * -> * and (* -> *) -> *.

Unapply[TC[_[_]], MA] in my proposed alphabet soup would be Y[X[F[A1]], A2], which is not bad once you mentally map X to be a functor typeclass.

@som-snytt
Contributor

For the test, I was going to suggest overriding toString; but I don't know if that was an extraneous part of the test.

@seanparsons

I agree it would be preferable to have a form that is something that we can use in Scala code.

@folone
Contributor
folone commented Mar 31, 2013

PLS REBUILD ALL

@scala-jenkins

(kitty-note-to-self: ignore 15697719)
🐱 Roger! Rebuilding pr-rangepos-per-commit, pr-checkin-per-commit for 7855b08f, c9314fb8, c4245ae6. 🚨

@eed3si9n
Member
eed3si9n commented Apr 1, 2013

Created topic/kind2 branch under folone/scala: folone/scala@ca9edc1
This implements Scala syntax and variance. Not sure if I can get the bounds staying generic at API level.

scala> :k Int
Int's kind is A

scala> :k -v Either
Either's kind is F[+A1, +A2]
This is a type constructor: a 1st-order-kinded type.

scala> :k -v scalaz.Monad
Monad's kind is X[F[A]]
This is a type constructor that takes type constructor(s): a higher-kinded type.

scala> :k scalaz.Unapply
Unapply's kind is Y[X[F[A1]], A2]
@retronym
Member
retronym commented Apr 1, 2013

I'm not really sure what makes the most sense. Just to add a bit more confusion:

scala> typeOf[Either[Any, Any]].typeConstructor.etaExpand
res10: $r.intp.global.Type = [+A, +B]Either[A,B]

scala> trait Monad[F[_]]
warning: there were 1 feature warning(s); re-run with -feature for details
defined trait Monad

scala> typeOf[Monad[Any]].typeConstructor.etaExpand
res11: $r.intp.global.Type = [F[_]]Monad[F]

scala> trait Unapply[TC[_[_]], MA]
warning: there were 2 feature warning(s); re-run with -feature for details
defined trait Unapply

scala> typeOf[Unapply[Any, Any]].typeConstructor.etaExpand
res12: $r.intp.global.Type = [TC[_[_]], MA]Unapply[TC,MA]
@paulp
Contributor
paulp commented Apr 1, 2013

For any which go deeper than a single type constructor, how about something like this - I probably blew the logic since I'm jumping on a plane, but wave your hands and make it right, you get the gist.

scala> :k Unapply
Unapply[TC[_2], MA]
  where _1 =    [A1] => F[A1]   // (* -> *)
  where _2 = [_2[F]] => X[_2]   // (* -> *) -> *)

I like this less than what I pictured when I started it, but I think some sort of decomposition is unavoidable.

@eed3si9n
Member
eed3si9n commented Apr 1, 2013

Unapply show down.

standard notation

((* -> *) -> *) -> * -> *

Eugene's Scala notation

Y[X[F[A1]], A2]

REPL's current notation for eta-expanded type constructor

[TC[_[_]], MA]Unapply[TC,MA]

Paul's notation

I'm interpreting Paul's suggestion to be "Use function-like syntax to express type constructor":

[_1, MA] => Unapply[_1, MA]
  where _1 = [_2] => X[_2]
  where _2 = [A1] => F[A1]

why mine is better

If you put trait in front, it compiles. Or better way of looking at it is, if the given expression appeared in a larger type, the entire type expression parses correctly as type variables. In other words,

scala> trait Foo[Y[X[F[A1]], A2]]
warning: there were 3 feature warning(s); re-run with -feature for details
defined trait Foo

Using _ means that you're not interested in using the type variable later. I don't think TC[_[_]] makes it any better than X[F[A1]].

The naming issue. Let's step back and ask the type of 1:

scala> :t 1
Int

Yea, REPL gives Int. Now what's the kind of Int?

scala> :k Int
Int's kind is A

A or in standard notation * can be the only answer here, not Int, because that's like answering 1's type is 1. Seemingly-intuitive, but not right, because it doesn't match 2's type. Similarly, Int's kind should match up that of Double's; Option's with List's, etc.

Keeping the original type variable names coud be confusing too. Some people use M instead of F. Others are using F to mean *. For example, Scalaz 7 codegens F for the type argument of both Functor and Monoid (to avoid confusion with A used for method type params?).

scala> :k scalaz.Monoid
Monoid's kind is F[A]
@folone
Contributor
folone commented Apr 2, 2013

👍 I like @eed3si9n's notation. We could also consider adding other notation(s) to the verbose output. Something like:

scala> :k Unapply
Unapply's kind is Y[X[F[A1]], A2]

scala> :k -v Unapply
Unapply's kind is Y[X[F[A1]], A2]
In standard kind notation: ((* -> *) -> *) -> * -> *
In decomposed notation:
[_1, MA] => Unapply[_1, MA]
  where _1 = [_2] => X[_2]
  where _2 = [A1] => F[A1]
This is a type constructor that takes type constructor(s): a higher-kinded type.
@adriaanm
Member
adriaanm commented Apr 2, 2013

For completeness, I should point out that kinds also track bounds and variance, as discussed in http://adriaanm.github.com/files/higher.pdf

@paulp
Contributor
paulp commented Apr 2, 2013

Adriaan's comment is part of why I was devising my plane-aborted expanded notation. If we're seeking a "scala syntax" it must be capable of expressing type constructor variance and bounds, which means a bunch of stars won't suffice, and underscores will only take you so far.

@eed3si9n
Member
eed3si9n commented Apr 2, 2013

@paulp Isn't "Scala syntax" essentially the same as the type parameter expression we write between the braces?

trait Foo[F[A <: Ordered[A]] <: Iterable[A]]

trait Foo[F[+A]]
@paulp
Contributor
paulp commented Apr 2, 2013

The problems arise at the boundaries. I assume we're all familiar with the problem in this one:

trait Foo[CC[_] <: Traversable[_]]

The collision between an existential and a "don't care" placeholder type argument reaches tragic proportions if we'd like to come anywhere near optimal syntax for expressing kinds.

Even this can mean two different things

Foo[F[_]]

You have to know whether Foo's type argument is * -> * or * in order to know what F is doing there. But if the point of it is to communicate Foo's kind... I think we tolerate an unacceptable level of syntactic ambiguity.

@paulp paulp commented on an outdated diff Apr 3, 2013
src/repl/scala/tools/nsc/interpreter/Kind.scala
@@ -0,0 +1,66 @@
+/* NSC -- new Scala compiler
+ * Copyright 2013 LAMP/EPFL
+ * @author Eugene Yokota
+ */
+
+package scala.tools.nsc
+package interpreter
+
+trait Kind {
+ type T <: scala.reflect.api.Universe
@paulp
paulp Apr 3, 2013 Contributor

There are conventions to which we should attempt to adhere. This T should be called either U (for Universe) or G (for Global.)

@paulp paulp and 1 other commented on an outdated diff Apr 3, 2013
src/repl/scala/tools/nsc/interpreter/Kind.scala
+ */
+
+package scala.tools.nsc
+package interpreter
+
+trait Kind {
+ type T <: scala.reflect.api.Universe
+ val u: T
+ def tpe: u.Type
+ def sig: u.Type
+ def typeKind: String
+ def description: String
+}
+
+object Kind {
+ def apply[A <: scala.reflect.api.Universe](au: A)
@paulp
paulp Apr 3, 2013 Contributor

This one should be U1, the incoming val u, and I really don't like letter strings like atpe, asig, etc. I suggest tpe0, sig0, ... instead.

@paulp
paulp Apr 3, 2013 Contributor

Although life is easier yet if you abandon the pretense of generality and declare Kind more like

class KindU[U <: api.Universe](val u: U) { case class Kind(tpe: u.Type, sig: u.Type, typeKind: String, description: String) { } }

because then you don't have to transplant each parameter from an unselectable method into a trait.

@eed3si9n
eed3si9n Apr 4, 2013 Member

I tried KindU, but got the following error trying to use it:

[quick.repl] /Users/eed3si9n/work/folone_scala/src/repl/scala/tools/nsc/interpreter/package.scala:143: error: type mismatch;
[quick.repl]  found   : reflect.runtime.universe.Type
[quick.repl]  required: _1.u.Type where val _1: scala.tools.nsc.interpreter.KindU[scala.reflect.api.JavaUniverse]
[quick.repl]         case (Some(typ), _) => KindU(ru).kind(typ)
[quick.repl]    

For now I'm going to keep the Kind trait with improved param names.

@paulp paulp commented on an outdated diff Apr 3, 2013
src/repl/scala/tools/nsc/interpreter/Kind.scala
+ val u = au
+ def tpe = atpe.asInstanceOf[u.Type]
+ def sig = asig.asInstanceOf[u.Type]
+ val typeKind = atypeKind
+ val description = adescription
+ }
+ def kind[T <: scala.reflect.api.Universe](u: T)(a: u.Type): Option[Kind] = {
+ import u._
+ def typeKind(sig: Type): String = sig match {
+ case PolyType(params, resultType) =>
+ (params map { p =>
+ typeKind(p.typeSignature) match {
+ case "*" => "*"
+ case s => "(" + s + ")"
+ }
+ }).mkString(" -> ") + " -> *"
@paulp
paulp Apr 3, 2013 Contributor

This should be something more like

(paramTypes :+ resultType) map typeKind mkString " -> "

even if you can't nest polytypes today, maybe tomorrow.

@paulp paulp commented on an outdated diff Apr 3, 2013
src/repl/scala/tools/nsc/interpreter/Kind.scala
+ case PolyType(params, resultType) =>
+ (params map { p =>
+ typeKind(p.typeSignature) match {
+ case "*" => "*"
+ case s => "(" + s + ")"
+ }
+ }).mkString(" -> ") + " -> *"
+ case _ => "*"
+ }
+ def typeSig(tpe: Type): Type = tpe match {
+ case SingleType(pre, sym) => sym.companionSymbol.typeSignature
+ case ExistentialType(q, TypeRef(pre, sym, args)) => sym.typeSignature
+ case TypeRef(pre, sym, args) => sym.typeSignature
+ case ClassInfoType(ext, scope, sym) => sym.typeSignature
+ case ThisType(sym) => sym.typeSignature
+ case _ => tpe
@paulp
paulp Apr 3, 2013 Contributor

Habitual non-elimination of unused variable patterns is very noisy. This could be written

def typeSig(tpe: Type): Type = tpe match {
  case SingleType(_, sym)                     => sym.companionSymbol.typeSignature
  case ExistentialType(_, TypeRef(_, sym, _)) => sym.typeSignature
  case TypeRef(_, sym, _)                     => sym.typeSignature
  case ClassInfoType(_, _, sym)               => sym.typeSignature
  case ThisType(sym)                          => sym.typeSignature
  case _                                      => tpe
}

Much of the logic in that method could use some explanation.

@paulp paulp and 1 other commented on an outdated diff Apr 3, 2013
src/repl/scala/tools/nsc/interpreter/Kind.scala
+ case _ => tpe
+ }
+ a match {
+ case NoType => None
+ case _ =>
+ val sig = typeSig(a)
+ var s = typeKind(sig)
+ val desc =
+ s match {
+ case "*" =>
+ "This is a proper type."
+ case x if !(x contains "(") =>
+ "This is a type constructor: a 1st-order-kinded type."
+ case x =>
+ "This is a type constructor that takes type constructor(s): a higher-kinded type."
+ }
@paulp
paulp Apr 3, 2013 Contributor

I strive to avoid logic of the form:

a) Calculate meaningful value
b) Derive string from meaningful value
c) Derive second string from first string instead of from meaningful value

Use primary sources.

@eed3si9n
eed3si9n Apr 3, 2013 Member

Yea I agree. That was from the stupid hack I wrote originally, and it's now gone. Both the notation and description will be built from a case class structure converted from Type.

@paulp paulp and 1 other commented on an outdated diff Apr 3, 2013
test/files/run/kind-repl-command.check
@@ -0,0 +1,20 @@
+Type in expressions to have them evaluated.
+Type :help for more information.
+
+scala>
+
+scala> :kind scala.Option
+Option's kind is * -> *
+
+scala> :k -v Either
+Either's kind is * -> * -> *
+This is a type constructor: a 1st-order-kinded type.
@paulp
paulp Apr 3, 2013 Contributor

Maybe this was discussed, but is there some reason we would call Either's kind * -> * -> * rather than (*, *) -> * ? Maybe they're the "same thing" right now in the sense that there's only one formulation in the language, but if they're ever not the same thing, then this will be on the losing end. It's conceivable someday you can declare Either like this:

class Either[+A][+B]

And that would be the * -> * -> * one.

@eed3si9n
eed3si9n Apr 3, 2013 Member

I think you have some point there, but I don't know "it's conceivable someday ..." is good enough reason to mess with the Standard curried notation. If we accept @adriaanm's notation for bounds, that already overloads the standard meaning of parens. Plus Scala already lets you write type Function1Int[A] = Function1[Int, A].

@paulp paulp commented on an outdated diff Apr 3, 2013
src/repl/scala/tools/nsc/interpreter/package.scala
@@ -127,6 +127,35 @@ package object interpreter extends ReplConfig with ReplStrings {
""
}
+ def kindCommandInternal(expr: String, verbose: Boolean): Unit = {
+ val m = ru.runtimeMirror(intp.classLoader)
+ val catcher = scala.util.control.Exception.catching(classOf[MissingRequirementError],
+ classOf[ScalaReflectionException])
@paulp
paulp Apr 3, 2013 Contributor

I admit I am the original perpetrator of catching, but I don't like it and I discourage its use. Also, I don't like long package paths inlined directly. Import either Exception or catching up top so the code is not obscured by irrelevant packaging details.

@paulp paulp commented on an outdated diff Apr 3, 2013
src/repl/scala/tools/nsc/interpreter/package.scala
+ val typ1 = catcher opt {
+ val typeSymbol = exprTyper.typeOfExpression(expr.trim).typeSymbol
+ m.staticClass(typeSymbol.fullName).toType
+ }
+ val typ2 = catcher opt {
+ m.staticClass(expr.trim).toType
+ }
+
+ ((typ1, typ2) match {
+ case (Some(typ), _) => Kind.kind(ru)(typ)
+ case (_, Some(typ)) => Kind.kind(ru)(typ)
+ case (None, None) =>
+ val info = replInfo(symbolOfLine(expr))
+ Kind.kind(intp.global)(info)
+ }) map { echoKind(_, verbose) }
+ }
@paulp
paulp Apr 3, 2013 Contributor
def fallback = Kind.kind(intp.global)(replInfo(symbolOfLine(expr)))
(typ1 orElse typ2).fold(fallback)(Kind.kind(ru)) foreach (k => echoKind(k, verbose))
@paulp paulp commented on an outdated diff Apr 3, 2013
src/repl/scala/tools/nsc/interpreter/ILoop.scala
@@ -292,6 +293,13 @@ class ILoop(in0: Option[BufferedReader], protected val out: JPrintWriter)
}
}
+ private def kindCommand(expr: String): Result =
+ expr.trim match {
+ case "" => ":kind [-v] <expression>"
+ case s if s startsWith "-v" => intp.kindCommandInternal(s stripPrefix "-v " trim, verbose = true)
+ case s => intp.kindCommandInternal(s, verbose = false)
@paulp
paulp Apr 3, 2013 Contributor

These two cases are really one case:

intp.kindCommandInternal(s stripPrefix "-v " trim, verbose = s startsWith "-v ")
@paulp paulp and 1 other commented on an outdated diff Apr 3, 2013
src/repl/scala/tools/nsc/interpreter/package.scala
+ m.staticClass(expr.trim).toType
+ }
+
+ ((typ1, typ2) match {
+ case (Some(typ), _) => Kind.kind(ru)(typ)
+ case (_, Some(typ)) => Kind.kind(ru)(typ)
+ case (None, None) =>
+ val info = replInfo(symbolOfLine(expr))
+ Kind.kind(intp.global)(info)
+ }) map { echoKind(_, verbose) }
+ }
+
+ def echoKind(kind: Kind, verbose: Boolean) {
+ def typeString: String =
+ if (kind.u == intp.global) "" + kind.tpe
+ else kind.sig.typeSymbol.name.toString
@paulp
paulp Apr 3, 2013 Contributor

What is the significance of the comparison of Universes and different logics? If you have evidence it is accomplishing something of interest, I would like to see that in the form of a test case.

@eed3si9n
eed3si9n Apr 4, 2013 Member

That was working around refined types printing out <refinement>. I moved the typeString method to Kind, changed it to check for refined types, and added a test case.

@eed3si9n
Member
eed3si9n commented Apr 3, 2013

Since we're discussing implementation, we have a loose consensus on attempting Scala syntax as kind notation? If so could we roll back the last commit "Addressed some of issues, raised by @paulp" and merge folone/scala: folone/scala@ca9edc1 from topic/kind2 branch first? That's where Scala syntax notation is and it does address some of the points raised by Paul.

@folone I'd also like to get a chance to review the review before incorporating it.

@folone
Contributor
folone commented Apr 3, 2013

@eed3si9n sure, go ahead. I actually should have done this in another branch.

@folone
Contributor
folone commented Apr 3, 2013

PLS REBUILD ALL

@scala-jenkins

(kitty-note-to-self: ignore 15839597)
🐱 Roger! Rebuilding pr-rangepos-per-commit, pr-checkin-per-commit for 7855b08f, c9314fb8, c4245ae6, ca9edc14. 🚨

@adriaanm
Member
adriaanm commented Apr 3, 2013

It would be great if the outcome of this work would be a Kind data structure we can use in the type checker to represent kinds internally. I found some work I did on this 4 years ago and published it for the world to see/ridicule:

https://github.com/adriaanm/scala/blob/explicitkinds/src/compiler/scala/tools/nsc/symtab/Kinds.scala

@eed3si9n
Member
eed3si9n commented Apr 5, 2013

@adriaanm If you don't mind I'm going to try to replace my hack logic with your inferKind this weekend.

In the mean time, can I bring up @folone's question to scala-internal again? Given a String scala.Option, how can we get Type (or TypeSymbol) out of it?

Normally one would have no choice but to go for the runtime universe like:

      val m = ru.runtimeMirror(intp.classLoader)
      val catcher = catching(classOf[MissingRequirementError],
                             classOf[ScalaReflectionException])
      val typ1 = catcher opt {
        val typeSymbol = exprTyper.typeOfExpression(expr.trim).typeSymbol
        m.staticClass(typeSymbol.fullName).toType
      }
      val typ2 = catcher opt {
        m.staticClass(expr.trim).toType
      }

But we have access to global, which is a universe on it's own. @retronym's answer to Getting type information inside scala repl via IMain seems to indicate it's doable. But it turns out that lookupSymbol is good for "Option", but won't work for "scala.Option":

scala> val sym = context.lookupSymbol("Option": TypeName, _ => true).symbol
sym: g.analyzer.global.Symbol = class Option

scala> val sym = context.lookupSymbol("scala.Option": TypeName, _ => true).symbol
sym: g.analyzer.global.Symbol = <none>

Ideally, I would like to have IMain.symbolOfLine equivalent that accepts a legal type symbol. I don't think I should be parsing it myself by separating the dots and hash. Note it needs to accept scala.Option as well as scala.Option[Int].

The benefit of not having to deal with runtime universe is 1) it simplifies kind.scala since I can code to a single universe. 2) it gives me richer Type info including bounds, which I don't think is available now for API-level Type.

@paulp
Contributor
paulp commented Apr 5, 2013

There are too many ways to do this sort of thing, but long ago (before reflection) I put this in global, which still seems to work:

scala> global.findMemberFromRoot("scala.collection.TraversableLike": TypeName)
res0: $r.global.Symbol = trait TraversableLike

It isn't going to work with "Option[Int]", you're on your own there. There's no general purpose String => Type facility. I'd be thrilled if there were one. I'd be even more thrilled if even the "easy" direction worked all the time, i.e. that the toString of a Type was always valid scala source representing that type, except when impossible to express.

@adriaanm
Member
adriaanm commented Apr 5, 2013

If you don't mind I'm going to try to replace my hack logic with your inferKind this weekend.

That would be the ideal outcome of my manipulative reference to that old branch.
There's already a Kinds.scala in the compiler, by the way.

@eed3si9n
Member
eed3si9n commented Apr 5, 2013

@paulp I guess if I had the full name I could ask g.rootMirror too:

scala> g.rootMirror.staticClass("scalaz.Monad")
res3: g.ClassSymbol = class Monad

But now it won't work for imported names:

scala> import scalaz._
import scalaz._

scala> g.rootMirror.staticClass("Monad")
scala.reflect.internal.MissingRequirementError: class Monad not found.
@folone
Contributor
folone commented Apr 5, 2013

@eed3si9n this problem is exactly why I added two cases as a workaround here. This is how I get the fully qualified name for your case.

@eed3si9n
Member
eed3si9n commented Apr 5, 2013

@folone By using exprTyper.typeOfExpression(expr.trim) it's treating the name as a term, so it currently requires companion object for imported names to work?:

scala> class Foo[A]
defined class Foo

scala> :k Foo
<console>:40: error: not found: value Foo
              Foo
              ^

scala> case class Bar[A]()
defined class Bar

scala> :k Bar
Bar's kind is F[A]
@eed3si9n
Member
eed3si9n commented Apr 8, 2013

PLS REBUILD ALL

inferKind is ported. This solves kind with an elegant if statement:

if(!tpe.isHigherKinded)
  ProperTypeKind(tpe.asSeenFrom(pre, owner).bounds, tpe)
else { ... }

Thanks @retronym for solving the ClassInfoType mystery.

The verbose output now includes enhanced standard notation with bounds and variance:

scala> :k -v Either
scala.util.Either's kind is F[+A, +B]
* -(+)-> * -(+)-> *
This is a type constructor: a 1st-order-kinded type.

scala> :k -v scala.collection.generic.Sorted
scala.collection.generic.Sorted's kind is F[K, +This <: Sorted]
* -> *(Sorted) -(+)-> *
This is a type constructor: a 1st-order-kinded type.

I also added a limited String => Type facility. This allows me to write several types that was not supported previously:

scala> class Foo
defined class Foo

scala> :k Foo
Foo's kind is A

scala> :k Int => Int
scala.Function1's kind is F[-T1, +R]
@scala-jenkins

(kitty-note-to-self: ignore 16032252)
🐱 Roger! Rebuilding pr-rangepos-per-commit, pr-checkin-per-commit for 7855b08f, c9314fb8, c4245ae6, ca9edc14, 4cc2222a, 3245d85e, 8999a60b, 5d7e1431. 🚨

@adriaanm adriaanm was assigned Apr 10, 2013
@eed3si9n
Member

@adriaanm I took the type out of the kind. I was ignoring bounds that's the same as tpe, so now it comes out like this for proper types:

scala> :k Int
scala.Int's kind is A >: Int <: Int

scala> class Foo
defined class Foo

scala> :k Foo
Foo's kind is A >: Foo <: Foo

I don't know if this is correct/helpful.

@adriaanm
Member

sorry, i got swamped and lost track of this

I'd expect Int and Foo's kind to simply be *
I don't think bounds are relevant for concrete types. For abstract types, they are, of course.

type X <: String would have kind *(Nothing, String), or *(String) since we usually only use upper bounds

@paulp
Contributor
paulp commented Apr 19, 2013

I also expect Int and Foo's kind to be *, if anyone was waiting with bated breath for me to chime in.

@paulp paulp commented on an outdated diff Apr 19, 2013
src/reflect/scala/reflect/internal/Kinds.scala
+ * Type constructors are reprented using TypeConKind.
+ */
+ abstract class Kind {
+ def description: String
+ def order: Int
+
+ /** Scala syntax notation of this kind.
+ * Proper types are expresses as A.
+ * Type constructors are expressed as F[k1, k2, ...] where k1, k2, ... are parameter kinds.
+ */
+ def scalaNotation: String
+
+ def starNotation: String
+
+ private[internal] def buildString(sym: Symbol, variance: Variance, env: scala.collection.mutable.Map[Int, Int]): String
+ protected def typeAlias(x: Int): String =
@paulp
paulp Apr 19, 2013 Contributor

Can you import mutable at the top of the file, and refer to mutable.Map? I am not into the super long paths being inlined.

@paulp
paulp Apr 19, 2013 Contributor

Mutable-map-as-method-parameter is almost invariably the wrong thing. Efficiency is no issue here, which removes the only imaginable defense. Please limit mutant wandering as much as possible.

@paulp paulp commented on an outdated diff Apr 19, 2013
src/reflect/scala/reflect/internal/Kinds.scala
+ private[internal] def buildString(sym: Symbol, variance: Variance, env: scala.collection.mutable.Map[Int, Int]): String
+ protected def typeAlias(x: Int): String =
+ "$" + (x match {
+ case 0 => "A"
+ case 1 => "F"
+ case 2 => "X"
+ case 3 => "Y"
+ case 4 => "Z"
+ case n if n < 12 => ('O'.toInt - 5 + n).toChar.toString
+ case _ => "V"
+ })
+ protected def headString(env: scala.collection.mutable.Map[Int, Int]): String = {
+ env(order) += 1
+ typeAlias(order) + env(order).toString
+ }
+ protected def varianceString(variance: Variance): String = {
@paulp
paulp Apr 19, 2013 Contributor

Surely you shouldn't have to write this method as a one-off. Let's put it on Variance please. I'm sure I could be persuaded that there's no point to having "+/-" printed.

@paulp paulp commented on an outdated diff Apr 19, 2013
src/reflect/scala/reflect/internal/Kinds.scala
+ def lowerBound = bounds.lo
+ def upperBound = bounds.hi
+ private def emptyLowerBound: Boolean =
+ typeIsNothing(lowerBound) || lowerBound.isWildcard
+ private def emptyUpperBound: Boolean =
+ typeIsAny(upperBound) || upperBound.isWildcard
+ val description: String = "This is a proper type."
+ val order = 0
+ def buildString(sym: Symbol, variance: Variance, env: scala.collection.mutable.Map[Int, Int]): String = {
+ headString(env)
+ varianceString(variance) + sym.nameString + scalaBoundsString
+ }
+ def scalaNotation: String = {
+ typeAlias(order).replaceAllLiterally("$", "") + scalaBoundsString
+ }
+ private def scalaBoundsString: String = {
@paulp
paulp Apr 19, 2013 Contributor

This is another method which exists a bunch of places, and I just don't think I can abide any more type/string sprawl.

@paulp paulp commented on an outdated diff Apr 19, 2013
src/reflect/scala/reflect/internal/Kinds.scala
+ headString(env)
+ varianceString(variance) + sym.nameString + scalaBoundsString
+ }
+ def scalaNotation: String = {
+ typeAlias(order).replaceAllLiterally("$", "") + scalaBoundsString
+ }
+ private def scalaBoundsString: String = {
+ (if (emptyLowerBound) ""
+ else " >: %s" format typeString(lowerBound)) +
+ (if (emptyUpperBound) ""
+ else " <: %s" format typeString(upperBound))
+ }
+ def starNotation: String = {
+ "*" + starBoundsString
+ }
+ private def starBoundsString: String = {
@paulp
paulp Apr 19, 2013 Contributor

Heh, you may be interested: #2405

@paulp paulp and 1 other commented on an outdated diff Apr 19, 2013
src/reflect/scala/reflect/internal/Kinds.scala
+ case Invariant => " -> "
+ case Covariant => " -(+)-> "
+ case Contravariant => " -(-)-> "
+ })
+ }).mkString + "*"
+ }
+ }
+ object TypeConKind {
+ def apply(args: Seq[TypeConKind.Argument]): TypeConKind = new TypeConKind(args)
+ def unapply(tck: TypeConKind): Some[Seq[TypeConKind.Argument]] = Some(tck.args)
+ case class Argument(variance: Variance, kind: Kind)(val sym: Symbol) {}
+ }
+
+ /**
+ * Starting from a Symbol (sym) or a Type (tpe), infer the kind that classifies it (sym.tpeHK/tpe).
+ * It must be run in typer phase.
@paulp
paulp Apr 19, 2013 Contributor

It must be run in typer phase? That's not going to be true. You can't really say anything useful about phases. I would just strike that from the record.

@eed3si9n
eed3si9n Apr 19, 2013 Member

Maybe I'm not phrasing it correctly, but I wanted to set the expectation that the stated behavior is guaranteed only if sym knows about the type since sym.tpeHK can also return ClassInfoType. See REPL: intp.global vs "global" available in :power mode (ClassInfoType problem)

@paulp
paulp Apr 19, 2013 Contributor

But that sort of caveat could be placed on every method in the compiler. There is a bunch of stuff you have to know, there's no getting around it.

Things to confuse you further: you don't get a ClassInfoType for tpeHK. You might get one for "info". tpeHK will give you a typeref with no type args. tpe_* will give you one with dummy type args. "typeSignature" in the reflection api corresponds to "info", not tpeHK. It will give you a polytype before erasure and a classinfotype after erasure. If you normalize the typeref you got via tpeHK you can find another polytype.

Also, there are a lot of phases after typer where it would still work the same as does during typer, so it's not accurate even if we forget the phase details.

@paulp paulp commented on an outdated diff Apr 19, 2013
src/reflect/scala/reflect/internal/Kinds.scala
+ }
+
+ /**
+ * Starting from a Symbol (sym) or a Type (tpe), infer the kind that classifies it (sym.tpeHK/tpe).
+ * It must be run in typer phase.
+ */
+ object inferKind {
+ import TypeConKind.Argument
+ def apply(pre: Type): InferKind = new InferKind {
+ def apply(tpe: Type, owner: Symbol): Kind = {
+ if(!tpe.isHigherKinded)
+ ProperTypeKind(tpe.asSeenFrom(pre, owner).bounds)
+ else {
+ val params = tpe.typeParams
+ val args = params map { p =>
+ val v = p.asType match {
@paulp
paulp Apr 19, 2013 Contributor

This seems like a lot of obfuscation to avoid declaring a val or calling asType twice, neither of which will hurt that badly. In fact, is there something taking place in lines 365-375 which is not captured by this line:

TypeConKind(tpe.typeParams map (p => Argument(p.variance, apply(p))(p)))

...maybe something involving bivariance? Perhaps you would like a method on Variance which maps bivariant to invariant and leaves the others alone. Although I'm curious what situation arises where one encounters bivariance on types one wishes to print.

@eed3si9n eed3si9n Add :kind command to REPL
:kind command diplays the kind of types and type constructors in Scala
syntax notation.

    scala> :kind (Int, Int) => Int
    scala.Function2's kind is F[-A1,-A2,+A3]
1d1492f
@eed3si9n
Member

Added a new commit reflecting @paulp's code review. I've accepted all points that were raised.

Mainly, Kind now tries to avoid getting into the business of converting other objects (Variance, Type, and TypeBounds) into String, and instead call the appropriate method on them to do the conversion. For that purpose, scalaNotation and starNotation methods were added to TypeBounds. Also, the logic that was passing a mutable map around to build Scala notation is replaced with series of immutable state transformation.

Other changes:

  • Proper types no longer contains bounds (Int's kind is A)
  • Type constructors now contains bounds too (scala.collection.generic.ImmutableSortedMapFactory's kind is X[CC[A,B] <: scala.collection.immutable.SortedMap[A,B] with scala.collection.SortedMapLike[A,B,CC[A,B]]])
  • Type variable names are used if bounds exists, otherwise they are normalized to prescribed alphabets: A, F, X, Y, Z...

The commits were growing, so I squashed them and rebased it off the current master.

@eed3si9n
Member

Any chance :kind gets into 2.11.0-M3?

@adriaanm
Member

LGTM

Thanks for patiently slogging through the PR maze. There are some refinements to be made before using the Kinds data structure more generally, but let's get this in already!

(Here's my -- very late -- review:

  • only proper kinds track bounds
  • tycon kinds track them indirectly by the kinds they compose into function kinds
  • also, I prefer "parameter" to "argument" when referring to the abstraction and not the application
    )
@adriaanm adriaanm merged commit bf3c44c into scala:master May 17, 2013

1 check passed

default pr-checkin-per-commit Took 56 min.
Details
@eed3si9n
Member

Nice! Thanks all for the feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment