Skip to content
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

Allow collective parameters for extension methods #6985

Merged
merged 2 commits into from Aug 11, 2019

Conversation

@odersky
Copy link
Contributor

commented Aug 4, 2019

This is a trial balloon. The idea is to allow one to "pull out" the leading parameter and any type parameters of extension methods to an enclosing given clause. E.g.

given ListOps {
  def (xs: List[T]) second[T]: T = xs.tail.head
  def (xs: List[T]) third[T]: T = xs.tail.tail.head
}

The repetition in the parameters can be avoided by moving the parameters into the given instance itself. The following version is a shorthand for the code above.

given ListOps[T](xs: List[T]) {
  def second: T = xs.tail.head
  def third: T = xs.tail.tail.head
}

IF we decide to go down this route, we might also want to re-open the issue where type parameters of extension methods go. Right now, they go after the method name. An alternative was proposed that would let them go in front, like this:

given ListOps {
  def [T](xs: List[T]) second: T = xs.tail.head
  def [T](xs: List[T]) third: T = xs.tail.tail.head
}

The argument for the current choice is that parameter definition and application syntax lines up this way. The argument for parameters first is that it keeps the principle that usages follow definitions. Right now, I think the "line up definition and application syntax " argument is stronger. But with collective parameters we would have to break it, since collective type parameter definitions have to go first, which is not the point where they would be applied. So under these circumstances the scales might well shift to favor putting type parameters always first.

@milessabin

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2019

Here's another example that might add some additional support to this proposal ...

I've been porting shapeless's Typeable to Dotty, and one problem was how to translate these extension methods,

object typeable {
  implicit def typeableOps[T](t : T): TypeableOps[T] = new TypeableOps(t)
}

final class TypeableOps[T](val t : T) extends AnyVal with Serializable {
  /**
   * Cast the receiver to a value of type `U` if possible.
   * This operation will be as precise wrt erasure as possible
   * given the in-scope `Typeable` instances available.
   */
  def cast[U](implicit castU : Typeable[U]) = castU.cast(t)

  /**
   * Cast the receiver to a value of subtype `U` of the receiver's static type if
   * possible. This operation will be as precise wrt erasure as possible given
   * the in-scope `Typeable` instances available.
   */
  def narrowTo[U <: T](implicit castU: Typeable[U]) = castU.cast(t)
}

The intended usage of these is with a single explicit type argument,

something.cast[Foo]
supertype.narrowTo[Subtype]

cast translates very nicely to the new extension method syntax because we can use Any for the type of the receiver,

given Ops {
  def (t: Any) cast [U] given (tu: Typeable[U]): Option[U] = ...
  ...
}

However, narrowTo is more awkward, because as well as an explicit type argument for the target type, we need an inferred type argument for the receiver. Using extension method syntax we end up with,

given Ops {
  def (t: T) narrowTo [T, U <: T] given (tu: Typeable[U]): Option[U] = ...
  ...
}

which doesn't work because we need to specify both type arguments explicitly, or none. Yes, we could use a named type argument here, but it doesn't look great,

supertype.narrowTo[U = Subtype]

Currently I've given up on extension methods and gone with an implicit class instead.

But I think your new proposal can fix that because it looks like it'd be possible to have a separate type parameter list on the outside, ie. something like,

given Ops[T](t: T) {
  def cast [U] given (tu: Typeable[U]): Option[U] = ...
  def narrowTo [U <: T] given (tu: Typeable[U]): Option[U] = ...
}

which gets us back to pretty much where we are with the implicit class.

@hrhino

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2019

This looks very similar to the current implicit class ... extends AnyVal scheme, so at least to someone who already knows Scala the meaning should be obvious.

@ri-cisse

This comment has been minimized.

Copy link

commented Aug 6, 2019

Does it make sense to separate type parameters for the receiver and application points? For example, we can then define narrowTo as:

given Ops {
  def [T](t: T) narrowTo [U <: T] given (tu: Typeable[U]): Option[U] = ???
  // ...
}
@odersky

This comment has been minimized.

Copy link
Contributor Author

commented Aug 6, 2019

given Ops[T](t: T) {
  def cast [U] given (tu: Typeable[U]): Option[U] = ...
  def narrowTo [U <: T] given (tu: Typeable[U]): Option[U] = ...
}

Unfortunately, that would not work (yet?) The desugaring merges the type parameters together into one single section. To do otherwise, we'd need to handle curried type parameters in general. No fundamental reason why we cannot do this, but it will affect a lot of parts in the compiler.

So for the moment, implicit class (or in the future explicit emulation via a Conversion) is the only way to side step the issue.

@odersky odersky requested a review from smarter Aug 6, 2019

@odersky

This comment has been minimized.

Copy link
Contributor Author

commented Aug 6, 2019

It seems there's consensus we want something like this. We have not discussed yet the question where type parameters should go for normal extension methods. I propose to come back to that question in a separate issue.

@lavrov

This comment has been minimized.

Copy link

commented Aug 6, 2019

Isn't using given here brings the same problems Scala 2 has with implicit where it is used for many different things. I thought given is reserved for context abstraction.

*
* to
*
* <extension> def foo[Ts ++ Us](x: T) parammss ...

This comment has been minimized.

Copy link
@smarter

smarter Aug 7, 2019

Member

We should emit an error if one of the Ts and one of the Us have the same name.

compiler/src/dotty/tools/dotc/ast/Desugar.scala Outdated Show resolved Hide resolved
object A {
given ListOps[T](xs: List[T]) {
def second: T = xs.tail.head

This comment has been minimized.

Copy link
@smarter

smarter Aug 7, 2019

Member

Needs more examples involving extension methods with type and term parameters, as well as neg tests when names used for type and term parameters clash with those used for the given clause.

@odersky odersky force-pushed the dotty-staging:add-collective-params branch from 578ec15 to d54c611 Aug 7, 2019

@odersky odersky merged commit b221132 into lampepfl:master Aug 11, 2019

2 checks passed

CLA User signed CLA
Details
continuous-integration/drone/pr Build is passing
Details

@odersky odersky deleted the dotty-staging:add-collective-params branch Aug 11, 2019

@milessabin

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

I think the merging of type-parameter blocks produces quite surprising results. The example in the tests looks very odd,

given ListOps[T](xs: List[T]) {
  def zipp[U](ys: List[U]): List[(T, U)] = xs.zip(ys)
}

xs.zipp[Int, Int](xs).map(_ + _).prod

Multiple type parameter lists would be very nice to have but, as you say, would be quite an undertaking. Is that really necessary to make this example work in the same way as the corresponding implicit class?

@odersky

This comment has been minimized.

Copy link
Contributor Author

commented Aug 12, 2019

The idea is that this should desugar as follows:

implicit object ListOps {
  <extension> def zipp[T](xs: List[T])[U](ys: List[U]): List[(T, U)] = xs.zip(ys)
}
@odersky

This comment has been minimized.

Copy link
Contributor Author

commented Aug 12, 2019

Do we also want to write this directly?

def [T](xs: List{T]) zipp [U] (ys: List[U]): List[(T, U)] = xs.zip(ys)

@anatoliykmetyuk anatoliykmetyuk added this to the 0.18 Tech Preview milestone Aug 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.