scala3 icon indicating copy to clipboard operation
scala3 copied to clipboard

Adding Clause Interleaving to method definitions

Open Sporarum opened this issue 4 years ago • 5 comments

This aims to add the ability to declare functions with many clauses of type parameters, instead of at most one, and to allow those clauses to be interleaved with term clauses:

def foo[A](x: A)[B](y: B) = (x, y)

All user-facing details can be found in the Scala 3 new features doc, and in the SIP proposal

The implementation details are described below in the commit messages

The community's opinion of the feature can be found on the scala contributors forum (note however that the description there is somewhat outdated)

Dependencies

This PR depends on https://github.com/lampepfl/dotty/pull/15899 and https://github.com/lampepfl/dotty/pull/14810 Therefore, these should be merged, and this subsequently rebased on master, before this can be merged

While this feature needs to be accepted by the SIP committee before it can be part of the language, since this PR adds it as an experimental feature, there is no need to wait for the decision to merge this.

The feature will be available with import scala.language.experimental.clauseInterleaving

(Since all the experimental mechanisms are added by one commit, simply reversing this commit will make the feature non-experimental)

Future Work

  1. Implement given aliases with clause interweaving: (to have types depends on terms)
given myGiven[T](using x: T)[U](using y: U) = (x, y)
  1. Add interleaved clauses to the left-hand side of extension methods:
extension (using Int)[A](using A)(a: A)[B](using B)
  def foo: (A, B) = ???
  1. Investigate usefulness/details of clause interweaving for classes and type currying for types:
class Foo[A](a: A)[B](b: B)
new Foo(0)("Hello!") // type: Foo[Int][String] ?

type Bar[A][B] = Map[A, B]
Bar[Char] // should this mean [B] =>> Bar[Char][B] ?

(Started as a semester project with supervision from @smarter, now part of my missions as an intern at the scala center)

Sporarum avatar Dec 01 '21 10:12 Sporarum

I'm not sure if you thought about the type system when a method application is split to blocks. Before this SIP type arguments are inferred as Nothing because they all "belong" to the first block. If this SIP is added, I expect the following to compile:

  class Box[T](x: T)
  def huge[T1, T2](
    t1: T1, t2 : T2, r : Int
  )[T3, T4](t3: T3, r2: Int, t4: T4)[T5](t5 : T5*): Box[(T1, T2, T3, T4, T5)] = ???

  val h1p1 = huge((1, 2), (3, 4), 5)
  val h1p2 = h1p1((6, 7), 8, (9, 10))
  val h1p3 = h1p2(11, 12)
  val h1pTest: Box[((Int, Int), (Int, Int), (Int, Int), (Int, Int), Int)] = h1p3

soronpo avatar Aug 24 '22 16:08 soronpo

I think this doc might need updating also https://github.com/lampepfl/dotty/blob/7d322ce0f74cb3b66441fa06a5f47ad9cf89f1b0/docs/_docs/reference/contextual/right-associative-extension-methods.md

bishabosha avatar Aug 24 '22 19:08 bishabosha

I'm not sure if you thought about the type system when a method application is split to blocks. Before this SIP type arguments are inferred as Nothing because they all "belong" to the first block. If this SIP is added, I expect the following to compile:

  class Box[T](x: T)
  def huge[T1, T2](
    t1: T1, t2 : T2, r : Int
  )[T3, T4](t3: T3, r2: Int, t4: T4)[T5](t5 : T5*): Box[(T1, T2, T3, T4, T5)] = ???

  val h1p1 = huge((1, 2), (3, 4), 5)
  val h1p2 = h1p1((6, 7), 8, (9, 10))
  val h1p3 = h1p2(11, 12)
  val h1pTest: Box[((Int, Int), (Int, Int), (Int, Int), (Int, Int), Int)] = h1p3

I share your expectation, sadly this problem is not related specifically to this proposal, but actually more general:

def small[T](x: T): T = x
val sm1 = small
val res: Int = sm1[Int](1) // method apply in trait Function1 does not take type parameters

https://scastie.scala-lang.org/6CfJWHWoSqCDdOc53uWKzA

The above works if you create a function explicitly:

def small[T](x: T): T = x
val sm1 = [R] => (x: R) => small[R](x)
sm1[Int](1) // 1

https://scastie.scala-lang.org/vdIK4Pp3TYOdiPO2oecrEA

Of course we would like the compiler to do this transformation for us, a feature called polymorphic eta-expansion. I implemented this feature for my semester project: https://github.com/lampepfl/dotty/pull/14015 https://contributors.scala-lang.org/t/polymorphic-eta-expansion/5516 The implementation, however, breaks TASTy compatibility, which is why I will be developing a second implementation as part of my internship

You might be able to try both at the same time on this branch, it was however quickly duct-taped together as an experiment, so a lot of edge-cases are not handled correctly

Sporarum avatar Aug 24 '22 20:08 Sporarum

Should interleaved definitions show a special error like "clause interleaving is an experimental feature, you can enable it with ..." ?

Sporarum avatar Aug 25 '22 13:08 Sporarum

Should interleaved definitions show a special error like "clause interleaving is an experimental feature, you can enable it with ..." ?

I don’t think so.

julienrf avatar Aug 25 '22 13:08 julienrf

@smarter I will probably rework the commits and update the code slightly, but feel free to review the tests (and more if you feel like it)

Sporarum avatar Dec 13 '22 14:12 Sporarum

I'm in particular having trouble with where to put my test cases, since there are so many existing test files already

Sporarum avatar Dec 13 '22 14:12 Sporarum

I reworked the commits, I think this PR is in a really good shape !

The commit should be modular enough that going commit by commit should make understanding it easier

So I invite you to review it (In particular @smarter)

PS: The "Make clause interleaving experimental" should be self-contained enough to illustrate well how to make something experimental

Sporarum avatar Dec 20 '22 16:12 Sporarum

The failing Scaladoc test will be fixed once I rebase on main (files contain "... 2022" instead of "... 2023", happy new year !)

Sporarum avatar Jan 05 '23 12:01 Sporarum

As all dependencies have been merged with main, this PR can be merged as soon as you think it is good enough !

@smarter did I correctly address your comments ? Is there anything else that needs changing ?

Also should someone else give it a look, @sjrd maybe ?

Sporarum avatar Jan 06 '23 10:01 Sporarum

IMO at least some of the neg tests should include a .check file to verify the error messages themselves.

soronpo avatar Jan 06 '23 10:01 soronpo

Also I think we need extension method interactions in the tests as well.

soronpo avatar Jan 06 '23 10:01 soronpo

@sjrd I believe I have addressed your comments, I didn't rebase yet so you can more easily see what I did to address them. I have only put one run test, it is relatively thorough, but I wouldn't mind some guidance if you think more is needed

@smarter Could you give it another look, I think I changed what you asked me too, but I need your confirmation

Sporarum avatar Jan 16 '23 11:01 Sporarum

Note to myself: I need to rollback the reference/syntax.md in the "make experimental" commit

Sporarum avatar Jan 16 '23 13:01 Sporarum

Looks fine for me except for the Java interop test we discussed off-line.

sjrd avatar Jan 16 '23 13:01 sjrd

Testing the java interop lead me to discover it does not work for extension methods either, see https://github.com/lampepfl/dotty/issues/16727

As such I believe java interop to be outside of the scope of this PR

Sporarum avatar Jan 19 '23 12:01 Sporarum

@smarter The last unreviewed changes are the ones added in commits Fix right-associative-extension-methods.md and Uniformise naming of clauses in right associative extension methods

Sporarum avatar Feb 02 '23 16:02 Sporarum