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

New proposal for extension methods #5114

Merged
merged 56 commits into from Dec 7, 2018

Conversation

@odersky
Contributor

odersky commented Sep 17, 2018

Motivated by @LukaJCB's proposal and the discussion on Contributors, here's a new proposal to add extension methods to Scala. It replaces #4114. Compared to #4114, this one is simpler and integrates out of the box with implicits.

A parallel discussion about coherence is in issue #4234

@odersky

This comment has been minimized.

Contributor

odersky commented Sep 17, 2018

It's cute that this is exactly 1000 PR's after the last outstanding attempt to do extension methods 😉

@odersky odersky force-pushed the dotty-staging:add-extensions branch from 1259aaf to d79d658 Sep 17, 2018

```scala
implicit object StringOps {
def (xs: Seq[String).longestStrings = {

This comment has been minimized.

@buzden

buzden Sep 17, 2018

Contributor

Typo, Seq[String] instead

@odersky

This comment has been minimized.

Contributor

odersky commented Sep 17, 2018

@buzden Yes, I left the this accidentally, it should be x. The meaning of this in an extension method is exactly the same as in a normal method, i.e. the enclosing object itself.

def result[T](implicit er: WrappedResult[T]): T = WrappedResult.unwrap(er)
implicit object Ensuring {
def (x: T).ensuring(condition: implicit WrappedResult[T] => Boolean): T = {

This comment has been minimized.

@jducoeur

jducoeur Sep 17, 2018

Contributor

This is the only bit of this proposal that I'm not getting. How, in this declaration, does the compiler know that T (in def (x: T).ensuring(...)) is generic? I'm not seeing any obvious distinction between T as concrete type and T as type parameter here...

This comment has been minimized.

@odersky

odersky Sep 17, 2018

Contributor

Oops, no wonder you were not getting that! There's a type parameter missing. That's the problem of posting example code without having a compiler to check it...

@soronpo

This comment has been minimized.

Contributor

soronpo commented Sep 17, 2018

Say I wish to manually call an extension method (for testing purposes or code sharing), is the syntax to do so looks like as follows?:

implicit object CircleOps {
  def (c: Circle).circumference: Double = c.radius * math.Pi * 2
}

CircleOps.circumference(myCircle)  // Will this work?

Additionally, is it possible to pass CircleOps.circumference as a byname value of Circle => Double ?

@odersky

This comment has been minimized.

Contributor

odersky commented Sep 17, 2018

@soronpo CircleOps.circumference(myCircle) should work, and CircleOps.circumference should be passable as a Circle => Double.

@nafg

This comment has been minimized.

nafg commented Sep 17, 2018

Seems very elegant. Could the type parameters go before? It feels weird to use something that is defined later.

@odersky

This comment has been minimized.

Contributor

odersky commented Sep 17, 2018

@nafg I played with type parameters first, but in the end decided against it because it needs more syntax and looks sometimes not very natural. Note there's precedence for type parameters being used before they are defined, e.g.

def f[X <: Y, Y]

Admittedly, these are in the same parameter clause, but still...

@olafurpg

This comment has been minimized.

Contributor

olafurpg commented Sep 17, 2018

It feels weird to use something that is defined later.

@nafg If it's any consolation, it's already possible to forward reference parameter names inside context bounds

def convert[T: c.WeakTypeTag](c: blackbox.Context)(in: c.Tree): Converted[c.type]
@LukaJCB

This comment has been minimized.

LukaJCB commented Sep 17, 2018

Looks pretty cool, thanks a lot for working on this! :)

I have a question w.r.t. type class usage.
If I have a semigroup type class and instance:

trait Semigroup[A] {
  def (x: A).combine(y: A): A
}

implicit val IntSemigroup = new Semigroup[Int] {
  def (x: Int).combine(y: Int): Int = x + y
}

Will I be able to use 1.combine(2)? I'm guessing only if it's in scope somehow, but how can we import it into scope? It's not a 100% clear to me.

@odersky

This comment has been minimized.

Contributor

odersky commented Sep 17, 2018

@LukaJCB For 1.combine(2) to work you need an implicit SemiGroup[Int] instance in scope. This could be either IntSemiGroup itself, or an implicit parameter of SemiGroup type.

By contrast, you do not need to "open" the SemiGroup with an import to make the extension methods available. The fact that you have an implicit instance is enough.

@LukaJCB

This comment has been minimized.

LukaJCB commented Sep 17, 2018

@odersky Thanks, that makes sense and is pretty cool!

I have a follow up question.
If I put a type class instance in the companion object of the type class itself like this:

trait Semigroup[A] {
  def (x: A).combine(y: A): A
}

object Semigroup {
  implicit val intSemigroup = new Semigroup[Int] {
    def (x: Int).combine(y: Int): Int = x + y
  }
}

(which is fairly common in the typelevel ecosystem), is there a way to import that single operation?
I could probably just directly import the instance: import Semigroup.intSemigroup, correct?

Right now in Cats we use an import that allows us to import specific syntax, so e.g. import cats.syntax.functor._ allows me to write fa.map(f) for any data type that has a functor. Of course it'd be great if we were somehow able to replicate that :)

@odersky

This comment has been minimized.

Contributor

odersky commented Sep 17, 2018

(which is fairly common in the typelevel ecosystem), is there a way to import that single operation?
I could probably just directly import the instance: import Semigroup.intSemigroup, correct?

Correct.

@kaishh

This comment has been minimized.

kaishh commented Sep 17, 2018

This + SAM can implement scope injection, if SAM was extended to make SAM's this and members visible, for example:

class Scope(value: Int) {
  def (scope: ScopeFn).it: Int = value
}

trait ScopeFn {
  def apply(): Int
}

def foo(f: implicit Scope => ScopeFn): Int = {
  implicit val scope = new Scope(5)

  f()
}

foo(() => it + this.it)
// 10

Would be even better if SAM was extended to accept blocks without parameters:

foo { it + it }
// An instance declaration:
implicit object StringMonoid extends Monoid[String] {
def (x: String).combine(y: String): String
def unit: String

This comment has been minimized.

@tabdulradi

tabdulradi Sep 17, 2018

Implementation is missing here, right?

This comment has been minimized.

@odersky

odersky Sep 18, 2018

Contributor

Right. Will fix in next push.

def (x: F[A]).flatMap[A, B](f: A => F[B]): F[B]
def (x: F[A]).map[A, B](f: A => B) = x.flatMap(f `andThen` pure)
def pure[A]: F[A]

This comment has been minimized.

@joshlemer

joshlemer Sep 17, 2018

should this be

def pure[A](a:A): F[A]

?

This comment has been minimized.

@odersky

odersky Sep 18, 2018

Contributor

oops, yes.

@soronpo

This comment has been minimized.

Contributor

soronpo commented Sep 17, 2018

What about prefix unary extension definitions? unary_~, unary_!, unary_-, unary_+

implicit object CircleOps {
  def unary_!(c: Circle): InvertOfCircle = ???
  //OR?
  def (c: Circle).unary_!: InvertOfCircle = ???
}
@lihaoyi

This comment has been minimized.

Contributor

lihaoyi commented Sep 18, 2018

Is the new syntax really necessary? It seems to me that the "novelty" here is in searching the members of in-scope implicit objects for conversions, which is orthogonal to the new def (foo: Foo).bar syntax the proposal also introduces.

For example, we could define the semantics such that instead of the new proposed syntax:

implicit object StringOps {
  def (xs: Seq[String]).longestStrings = {
    val maxLength = xs.map(_.length).max
    xs.filter(_.length == maxLength)
  }
}

You could use the existing syntax with the new semantics:

implicit object StringOps {
  implicit class Wrapper(xs: Seq[String]) extends AnyVal {
    def longestStrings = {
      val maxLength = xs.map(_.length).max
      xs.filter(_.length == maxLength)
    }
  }
}

Which we could define as semantically identical (i.e. if StringOps is in scope, we consider StringOps.Dummy when searching for extension methods. The somewhat awkward generic typeclass here:

  implicit class ListOrd[T: Ord] {
    def (xs: List[T]).compareTo(ys: List[T]): Int = (xs, ys) match
      case (Nil, Nil) => 0
      case (Nil, _) => -1
      case (_, Nil) => +1
      case (x :: xs1, y :: ys1) =>
        val fst = x.compareTo(y)
        if (fst != 0) fst else xs1.compareTo(ys1)
  }

Could be spelled as:

  implicit object ListOrd {
    implicit class Wrapper[T: Ord](xs: List[T]) extends AnyVal{
      def compareTo(ys: List[T]): Int = (xs, ys) match
        case (Nil, Nil) => 0
        case (Nil, _) => -1
        case (_, Nil) => +1
        case (x :: xs1, y :: ys1) =>
          val fst = x.compareTo(y)
          if (fst != 0) fst else xs1.compareTo(ys1)
    }
  }

Overall, as far as I can tell, this proposal has two orthogonal parts:

  • Changing the implicit search space for extension methods to include the body-scopes of in-scope implicit objects (???will we also change the implicit search space for implicit parameters, and other ad-hoc implicit conversions??? or will extension scope be something different from implicit scope)

  • A new shorthand syntactic sugar over anonymous implicit class extends AnyVal definitions to make it more concise to define a single extension method on a single type

I think it's worth discussing these two ideas separately, because each half has its own value even if implemented standalone.

For me, I think the change in implicit search space is an interesting idea, but am personally not a fan of the shorthand syntactic sugar. I don't think it adds enough convenience to the definition site to really justify the added complexity and overloaded syntax: especially the overloading of implicit class to no longer take parameters I find very unnatural. I think with some minor tweaks (e.g. adding AnyVal to implicit classes by default, perhaps allowing people to define anonymous implicits rather than making up a meaningless name every time) the existing syntax could fit very well onto the new implicit/extension search semantics.

@lloydmeta

This comment has been minimized.

Contributor

lloydmeta commented Sep 18, 2018

Just leaving a comment here for the rendered version of extension-methods.md

@odersky

This comment has been minimized.

Contributor

odersky commented Sep 18, 2018

@lihaoyi One could continue to use implicit value classes for infix methods. But then we'd have to extend the implicit search space to include applications of those classes. Implicit classes are currently also used for things other than extension methods, i.e. they can model implicit conversions. Suddenly those conversions will now be in the implicit scope just because their owning object is. Somehow, this makes me very nervous...

There are two other arguments against using implicit classes as the mechanism for extension methods:

  • they are awkward to write. The syntax is bulky and the most important thing (what gets extended?) is buried in an obscure place. By contrast, the extension method syntax follows
    the usage pattern exactly, which helps understandability.
  • they are inefficient at run-time. At run-time you don't really want a wrapper created and immediately taken apart just to invoke a method. Escape analysis might get rid of some wrappers, but that's not always guaranteed.
@odersky

This comment has been minimized.

Contributor

odersky commented Sep 18, 2018

@soronpo It would be

def (c: Circle).unary_!: InvertOfCircle = ???

!c is always expanded to c.unary_! and the extension method logic works with the expansion.

@allanrenucci

This comment has been minimized.

Member

allanrenucci commented Sep 18, 2018

I really liked the originally proposed syntax:

def circumference(this: Circle): Double = this.radius * math.Pi * 2

I understand it has issues with the meaning of this inside the body of the method. What about using a modifier instead? E.g.

def circumference(this c: Circle): Double = c.radius * math.Pi * 2
// or
extension def circumference(c: Circle): Double = c.radius * math.Pi * 2

Also, I suspect using implicit scope to resolve extension methods might be confusing. Do I need the implicit object in scope to resolve an extension? Can a standalone extension method be imported? For example, would the code snipped below compile?

package foo

implicit object CircleOps {
  def (c: Circle).circumference: Double = c.radius * math.Pi * 2
}
package bar

import foo.CircleOps.circumference
// or, import foo.CircleOps._

class Test {
  def test(c: Circle) = c.circumference
}
@LukaJCB

This comment has been minimized.

LukaJCB commented Sep 18, 2018

I think being able to import standalone extension methods as @allanrenucci showed would be a plus 👍

@odersky

This comment has been minimized.

Contributor

odersky commented Sep 18, 2018

Also, I suspect using implicit scope to resolve extension methods might be confusing. Do I need the implicit object in scope to resolve an extension?

Yes, or the implicit object is in the implicit scope of the left-hand side argument. These are the same rules as if the implicit object was an implicit decorator class.

Can a standalone extension method be imported? For example, would the code snipped below compile?

For simplicity and clarity I would say no. The implicit object has to be in scope, not the extension method itself.

@odersky

This comment has been minimized.

Contributor

odersky commented Sep 18, 2018

I think being able to import standalone extension methods as @allanrenucci showed would be a plus 👍

Why? I thought so at first as well, since it's a common convention for extension methods. But now I think it would just muddy the waters and I don't see a good use case for it. Maybe I am overlooking something.

@kaishh

This comment has been minimized.

kaishh commented Sep 18, 2018

@lihaoyi There's another aspect to extensions that's unlike implicit classes – they can be directly overridden – there's no need for Syntax conversions, etc.

odersky and others added some commits Oct 12, 2018

Fixes for hasExtensionMethod
Don't widen type before it is selected as a candidate - it might lose
precision.
Update docs/docs/reference/extension-methods.md
Co-Authored-By: odersky <odersky@gmail.com>
Handle positions in extension methods
1. Take extension methods into account when checking positions.

2. Fix position of empty type tree in method definition

Without this measure, the empty type tree gets the end position of
the preceding element. For an extension method with type parameters
this is the extension parameter list, but it should be the type parameters.
Fixing the position explicitly fixes the problem.
Avoid implicit objects that extend nothing
An implicit object that only exists as an extension method container
is a bit strange. Since extension methods can now also be made visible
by imports it makes sense to de-emphasize extension methods in implicit
values until we discuss typeclasses later in the section.

@odersky odersky force-pushed the dotty-staging:add-extensions branch from d7f341b to 49bae02 Dec 5, 2018

@smarter

smarter approved these changes Dec 7, 2018

@odersky odersky merged commit cfba20a into lampepfl:master Dec 7, 2018

3 checks passed

CLA User signed CLA
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/drone/pr the build was successful
Details

@allanrenucci allanrenucci deleted the dotty-staging:add-extensions branch Dec 7, 2018

def (p1: Probability) unary_~ : Probability = Certain - p1
def (p1: Probability) & (p2: Probability): Probability = p1 * p2
def (p1: Probability) | (p2: Probability): Probability = p1 + p2 - (p1 * p2)

This comment has been minimized.

@johnynek

johnynek Dec 7, 2018

Minor note: these formulas, though they appeared in the proposal, are a bit misleading. They are true of indendent events but not all events. Below you use them with missedTrain and caughtTrain. They are not indendent since you can’t miss and catch the train.

The real probability of missedTrain | caughtTrain is 1 but this formula won’t show that.

To fix the example it might be easiest to choose three events one imagines might be independent.

It doesn’t matter much but it might be nicer to avoid a future “well actually” (which technically this is, but I’m only doing it to give us a chance to possibly change it before some jerk posts to Twitter about how scala programmers don’t understand probability).

def apply[A: ClassTag](xs: A*): IArray[A] = initialize(Array(xs: _*))
// These should be inline but that does not work currently. Try again
// once inliner is moved to ReifyQuotes

This comment has been minimized.

@johnynek

johnynek Dec 7, 2018

Why do these need to be inline? They are already extension methods and they take no functions or by name parameters? Wouldn’t the JIT inline these just fine for us?

def (ia: IArray[A]) apply[A] (i: Int): A = (ia: Array[A])(i)
// return a sorted copy of the array
def sorted[A <: AnyRef : math.Ordering](ia: IArray[A]): IArray[A] = {

This comment has been minimized.

@johnynek

johnynek Dec 7, 2018

Might be a cool example to also define SortedIArray and return that here. Then binarySearch takes a SortedIArray.

-lower - 1
}
val xs: IArray[Long] = IArray(1L, 2L, 3L)

This comment has been minimized.

@johnynek

johnynek Dec 7, 2018

You might want to exercise sorted Here rather than building a sorted array by hand.

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