-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New proposal for extension methods #5114
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
Conversation
It's cute that this is exactly 1000 PR's after the last outstanding attempt to do extension methods 😉 |
1259aaf
to
d79d658
Compare
|
||
```scala | ||
implicit object StringOps { | ||
def (xs: Seq[String).longestStrings = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo, Seq[String]
instead
@buzden Yes, I left the |
def result[T](implicit er: WrappedResult[T]): T = WrappedResult.unwrap(er) | ||
|
||
implicit object Ensuring { | ||
def (x: T).ensuring(condition: implicit WrappedResult[T] => Boolean): T = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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...
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 |
@soronpo |
Seems very elegant. Could the type parameters go before? It feels weird to use something that is defined later. |
@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... |
@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] |
Looks pretty cool, thanks a lot for working on this! :) I have a question w.r.t. type class usage. 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 |
@LukaJCB For By contrast, you do not need to "open" the |
@odersky Thanks, that makes sense and is pretty cool! I have a follow up question. 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? Right now in Cats we use an import that allows us to import specific syntax, so e.g. |
Correct. |
This + SAM can implement scope injection, if SAM was extended to make SAM's 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation is missing here, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be
def pure[A](a:A): F[A]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, yes.
What about prefix unary extension definitions? implicit object CircleOps {
def unary_!(c: Circle): InvertOfCircle = ???
//OR?
def (c: Circle).unary_!: InvertOfCircle = ???
} |
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 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 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:
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 |
Just leaving a comment here for the rendered version of |
@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:
|
@soronpo It would be
|
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 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
} |
I think being able to import standalone extension methods as @allanrenucci showed would be a plus 👍 |
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.
For simplicity and clarity I would say no. The implicit object has to be in scope, not the extension method itself. |
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. |
@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. |
Don't widen type before it is selected as a candidate - it might lose precision.
Co-Authored-By: odersky <[email protected]>
(reverted from commit cf68e01)
(reverted from commit dc36648)
(reverted from commit 06bd529)
(reverted from commit d6ba157)
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.
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.
d7f341b
to
49bae02
Compare
|
||
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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might want to exercise sorted
Here rather than building a sorted array by hand.
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