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 leading context parameters in extension methods #10940

Merged
merged 28 commits into from
Jan 6, 2021

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Dec 28, 2020

Fixes #9530

@odersky
Copy link
Contributor Author

odersky commented Dec 28, 2020

@nicolasstucki If you have more examples where this would be useful, it would be good to try them out, and either use them in the library or add them as test cases.

@prolativ
Copy link
Contributor

It would be good to also add some documentation for this feature.

I played a bit with this branch and it looks that when having code like this

object Foo
trait Bar
given bar: Bar with {}
extension (using Bar)(foo: Foo.type) def xxxx = 1

you cannot provide the using section explicitly when using extension syntax, e.g.

Foo.xxxx(using bar) // won't compile

Instead you need to fallback to a dotless method call

xxxx(using bar)(Foo)

Is this intentional? To me this seems somehow inconsistent as in case of both type parameters and postfix using clauses the parameters of an extension can be passed explicitly in all notations (even though type parameters in the definition of an extension are placed before the extension receiver parameter)

extension [A](foo: Foo.type)(using Bar) def yyyy = 1
val a = Foo.yyyy
val b = Foo.yyyy[Int](using bar)
val c = yyyy[Int](Foo)(using bar)

Changes introduced in this PR will require some adjustments to code completion as before we could assume that the first parameter of an extension method would always be the receiver, which will not necessarily be true anymore

@odersky
Copy link
Contributor Author

odersky commented Dec 28, 2020

Is this intentional? To me this seems somehow inconsistent as in case of both type parameters and postfix using clauses the parameters of an extension can be passed explicitly in all notations (even though type parameters in the definition of an extension are placed before the extension receiver parameter)

Yes, that's intentional. Leading using clauses can be passed only if the method is called directly. The analogy with type parameters is unfortunate, but we plan to change the rules for type parameters so that type parameters can follow both extension and def name. In that case, the type parameters following extension also can be passed only in non-extension calls.

Who will take care of the changes in code completion?

@prolativ
Copy link
Contributor

Ok, that makes sense. I can take care of code completion

@prolativ
Copy link
Contributor

@odersky I played a bit with this branch and it looks like leading using clauses make the search of extensions in companion objects and givens in scope not work.
According to the spec:

There are four possible ways for an extension method to be applicable:

1. The extension method is visible under a simple name, by being defined or inherited or imported in a scope enclosing the reference.
2. The extension method is a member of some given instance that is visible at the point of the reference.
3. The reference is of the form r.m and the extension method is defined in the implicit scope of the type of r.
4. The reference is of the form r.m and the extension method is defined in some given instance in the implicit scope of the type of r.

but if you look at this example

trait Food
case class Banana(color: String) extends Food

trait Diet[A <: Animal]:
  type F <: Food
  def food: Seq[F]

trait Animal
object Animal:
  extension [A <: Animal](using diet: Diet[A])(animal: A) def food1 = diet.food
  extension [A <: Animal](animal: A)(using diet: Diet[A]) def food2 = diet.food

extension [A <: Animal](using diet: Diet[A])(animal: A) def food3 = diet.food
extension [A <: Animal](animal: A)(using diet: Diet[A]) def food4 = diet.food

trait Monkey extends Animal

given Diet[Monkey] with
  type F = Banana
  def food: Seq[Banana] = Seq(new Banana("yellow"), Banana("green"))

trait FoodOps
given FoodOps with
  extension [A <: Animal](using diet: Diet[A])(animal: A) def food5 = diet.food
  extension [A <: Animal](animal: A)(using diet: Diet[A]) def food6 = diet.food


val monkey = new Monkey {}

val foods = Seq(
  monkey.food1, // doesn't compile
  monkey.food2, // compiles
  monkey.food3, // compiles
  monkey.food4, // compiles
  monkey.food5, // doesn't compile
  monkey.food6, // compiles
)

you'll see that for cases 2 and 3 from the spec this doesn't work as expected (as case 4 is kind of a composition of 2 and 3, I guess it doesn't work either). Passing the type parameter to .foodN doesn't help

@odersky
Copy link
Contributor Author

odersky commented Dec 31, 2020

@prolativ I see. This will be difficult to fix. I believe our best bet is to wait for #10950 to make it in. That will simplify handling to type arguments to extension methods and might already fix the problem. If not, we have a simpler base to work with.

@prolativ
Copy link
Contributor

prolativ commented Jan 4, 2021

Allowing multiple type parameters sections doesn't seem to be going to change anything here. The problem here is that when an extension method is found in an companion object, it is discarded from the search because when the receiver type is compared to the first parameter of the extension, they don't match anymore because the first parameter can now be a using clause. But if the extension is available directly in the current scope a totally different mechanism is used to find it and this works. For extensions located inside implicits the search uses the same mechanism as for implicit conversions. I'm wondering whether there is any particular reason why we don't use the same mechanism to find extensions in all cases

@nicolasstucki
Copy link
Contributor

@nicolasstucki If you have more examples where this would be useful, it would be good to try them out, and either use them in the library or add them as test cases.

I added some tests and adapted some code in dokka to use extension methods on reflect symbols directly (this is only a first step). From the reflection use cases perspective it looks good.

@nicolasstucki
Copy link
Contributor

@odersky #10950 will not help with this case as we cannot move the type parameter to the def. I would have expected the expected food1 and food2 to behave the same. Is there any reason why they should not?

trait Animal
object Animal:
  extension [A <: Animal](using diet: Diet[A])(animal: A) def food1 = diet.food
  ...
extension [A <: Animal](using diet: Diet[A])(animal: A) def food3 = diet.food

...
monkey.food1, // doesn't compile
...
monkey.food3, // compiles
...

@odersky odersky force-pushed the fix-#9530 branch 2 times, most recently from 90535f1 to 872082e Compare January 5, 2021 10:54
tests/pos/i9530.scala Outdated Show resolved Hide resolved
@nicolasstucki
Copy link
Contributor

Rebased and resolved confilcts

@nicolasstucki
Copy link
Contributor

@odersky can you double-check the commits I added and then merge?

@odersky
Copy link
Contributor Author

odersky commented Jan 6, 2021

@nicolasstucki Looks all good now. Thanks for your help!

@odersky odersky merged commit fad5b8d into scala:master Jan 6, 2021
@odersky odersky deleted the fix-#9530 branch January 6, 2021 14:57
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 12, 2021
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 12, 2021
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 12, 2021
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 12, 2021
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 13, 2021
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 13, 2021
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 13, 2021
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 13, 2021
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 13, 2021
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 15, 2021
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 15, 2021
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 18, 2021
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 19, 2021
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 19, 2021
@anatoliykmetyuk anatoliykmetyuk added the release-notes Should be mentioned in the release notes label Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes Should be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support contextual dependent extensions
4 participants