Allow two type parameter sections in extension methods. #10950

odersky · 2020-12-29T18:19:05Z

We now allow extension methods to be written with type parameter clauses after extension and/or after def. This allows
extension methods to be expressed more naturally, e.g.

   trait Functor[F[_]]:
      extension [A](x: F[A]) def map [B](f: A => B): F[B]

This required quite a lot of changes to internal data structures. Notably, DefDefs take one combined paramss field that
contains both type and value parameters.

For the moment only extension methods can have two type parameter lists. We will most likely lift soon the syntactic restriction that normal methods can only have one type parameter list which must come first. Technically, it would only require a small change in the parser. But we should sort out some questions first before we do this. E.g.

should we support curried type parameters? There might still be some residual bugs related to this that would need to be sorted out.
should we support partial type applications of functions with curried type parameters? On the one hand, that's the whole point of having curried type parameters in the first place. On the other hand, we need to be aware that this will create a big push to currify libraries in order to allow for some selective type arguments to be inferred. Is that what we recommend? Maybe we should also look at alternatives like named type arguments at the same time?

That's why I recommend that the change to extension methods goes in for RC-1, but the other changes should come after 3.0.

Todos:

Sort out the test and community build problems which seem to stem from classpath confusion
Update Tasty reflect to allow for multiple type parameters as well, mirroring what the compiler does

odersky · 2021-01-01T11:47:53Z

@nicolasstucki I need your help to push this over the finish line.

The root problem is the asExprOf definition in quotes. Its type parameter [X] needs to go from the extension to the method itself. I did that in 11122ed with non-bootstrapped and bootstrapped versions of Quotes.scala and QuotesImpl.scala. This works, except that macro tests are still failing because they see the non-bootstrapped version of Quotes.scala. Likewise, the community build is failing for the same reason.

Can you try to sort these things out and get everything to green? I believe this is the most important thing to do for 3.0RC1. I see now what a mess the previous restriction on type parameters of extension methods created. If the current state makes it into RC-1, it would be super-hard to change it later.

I disabled the macro tests in 17e693b, so maybe start by reverting that commit and observe what fails.

odersky · 2021-01-01T11:51:48Z

Here's the root problem of all the failing tests:

-- Error: /__w/dotty/dotty/community-build/src/scala/dotty/communitybuild/FieldsImpl.scala:16:62 
Error:  16 |    val selects = symbols.map(Select(projectsTree, _).asExprOf[T])
Error:     |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error:     |method (using x$1: quoted.Type[T]): quoted.Expr[T] does not take type parameters
Error:  -- Error: /__w/dotty/dotty/community-build/src/scala/dotty/communitybuild/projects.scala:620:45 
Error:  620 |def allProjects = projects.reflectedFields.of[CommunityProject].sortBy(_.project)
Error:      |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error:      |    Could not find class dotty.communitybuild.FieldsImpl$ in classpath
Error:      | This location contains code that was inlined from projects.scala:620
Error:      | This location contains code that was inlined from Fields.scala:6
Error:  two errors found
Error:  two errors found

It clearly gets the wrong version of asExprOf, i.e. the one that takes the type parameter after extension. That's the version in
src-non-bootstapped.

odersky · 2021-01-01T11:56:19Z

Actually, I might have found a workaround in b4dd60b. So if it's just that we might be able to go ahead anyway. But it's still an issue that we seem to do some build steps on the wrong classpath.

EDIT: No, that fix is not sufficient. Now we get failures for sacalatest with asExprOf. It looks like the community build is run with the wrong classpath.

odersky · 2021-01-01T14:44:57Z

Rebased to latest master

odersky · 2021-01-02T08:52:20Z

test performance please

dottybot · 2021-01-02T08:53:10Z

performance test scheduled: 1 job(s) in queue, 0 running.

dottybot · 2021-01-02T11:55:26Z

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/10950/ to see the changes.

Benchmarks is based on merging with master (141bf9e)

julienrf

This is great work! I have minor questions about the specification.

julienrf · 2021-01-02T12:56:05Z

docs/docs/reference/contextual/right-associative-extension-methods.md

+            (x: X)                            // <-- extensionParam
+            (using d: D)                      // <-- trailingUsing
+    def +:: (y: Y)(using e: E)(z: Z)          // <-- otherParams
+```


How would such a method be called?

(y)(z) +:: x

?

It would be (y +:: x)(z).

julienrf · 2021-01-02T12:58:13Z

docs/docs/reference/contextual/extension-methods.md

+only if the method is referenced as a regular method:
+```scala
+map[Int](List(1, 2, 3))(_ + 1)
+```


Can we pass both type parameters? Int and String?

Sure, map[Int](List(1, 2, 3))[String](_ + 1).

docs/docs/reference/contextual/extension-methods.md

Formulate newExpr without recourse to IntegratedTypeArgs. Since with scala#10950 extension methods no lonnger require IntegratedTypeArgs either it means we will be able to drop it altogether.

nicolasstucki · 2021-01-04T10:08:25Z

@nicolasstucki I need your help to push this over the finish line.

The root problem is the asExprOf definition in quotes. Its type parameter [X] needs to go from the extension to the method itself. I did that in 11122ed with non-bootstrapped and bootstrapped versions of Quotes.scala and QuotesImpl.scala. This works, except that macro tests are still failing because they see the non-bootstrapped version of Quotes.scala. Likewise, the community build is failing for the same reason.

Can you try to sort these things out and get everything to green? I believe this is the most important thing to do for 3.0RC1. I see now what a mess the previous restriction on type parameters of extension methods created. If the current state makes it into RC-1, it would be super-hard to change it later.

I disabled the macro tests in 17e693b, so maybe start by reverting that commit and observe what fails.

I added a commit with the fix and reenabled the tests. The issue is that there were two definitions of asExprOf and only one was updated.

odersky · 2021-01-04T10:34:29Z

@nicolasstucki Doh! I should have seen that! Instead I tried lots of other things...

soronpo · 2021-01-04T20:11:42Z

That's why I recommend that the change to extension methods goes in for RC-1, but the other changes should come after 3.0.

I don't know the internals, but IMO, it's at least worthy verifying that TASTy supports the data structure now. The functionality can be added much later.

odersky · 2021-01-04T20:12:32Z

TASTy supports it already.

odersky · 2021-01-04T20:13:19Z

@nicolasstucki I think this is ready for review now.

compiler/src/dotty/tools/dotc/ast/TreeInfo.scala

compiler/src/dotty/tools/dotc/ast/tpd.scala

nicolasstucki · 2021-01-05T13:06:42Z

@odersky This PR will need to be rebased on #10998 once that PR is merged. That PR is there to make sure we do not lose the history of Quotes.scala and IArray.scala when moved to src-bootstrapped.

Rebasing should be simple, just keep all the changes from this branch in the files of src-bootstrapped and keep the files from origin in src-non-bootstrapped.

nicolasstucki · 2021-01-05T13:09:06Z

I can do the rebasing

Fixes scala#9530

Leading using clauses now scope over both left and right parameter.

This is a first step towards passing reference trees for all parameters together.

Using two different tags for empty parameter lists and parameter splits leads to a more straightforward encoding. Also: shift tags around to create more free slots for single element trees.

Macro tests seem to use the wrong version of Quotes (non-bootstrapped, where it should be bootstrapped).

To do this, get rid of all uses of `decomposeCall`.

Add tests with dependencies from second type parameter clause to first type parameter and extension parameter clauses.

Make overloading resulution work in the case where type parameters follow value parameters.

odersky · 2021-01-06T10:49:46Z

I think we should concentrate to get #10940 in, which subsumes this PR.

odersky · 2021-01-06T14:59:25Z

Subsumed by #10940

odersky marked this pull request as draft December 29, 2020 18:20

odersky force-pushed the add-multi-tparams branch 2 times, most recently from 5210c8c to 5885ecf Compare December 31, 2020 13:22

odersky mentioned this pull request Dec 31, 2020

Allow leading context parameters in extension methods #10940

Merged

odersky force-pushed the add-multi-tparams branch from 5885ecf to f3827dd Compare December 31, 2020 15:15

nicolasstucki self-assigned this Jan 1, 2021

odersky changed the title ~~Change internal datstructures to allow for multiple type param clauses~~ Allow two type parameter sections in extension methods. Jan 1, 2021

odersky marked this pull request as ready for review January 1, 2021 12:19

odersky added this to the 3.0.0-RC1 milestone Jan 1, 2021

odersky force-pushed the add-multi-tparams branch from 034d033 to e30a6fa Compare January 1, 2021 14:44

julienrf reviewed Jan 2, 2021

View reviewed changes

odersky mentioned this pull request Jan 2, 2021

Simplify newExpr #10973

Merged

odersky force-pushed the add-multi-tparams branch from 060013a to b3bafdb Compare January 4, 2021 13:22

odersky requested a review from nicolasstucki January 4, 2021 20:13

nicolasstucki mentioned this pull request Jan 5, 2021

Move Quotes and IArray to src-bootstrapped #10998

Merged

nicolasstucki reviewed Jan 5, 2021

View reviewed changes

compiler/src/dotty/tools/dotc/ast/TreeInfo.scala Show resolved Hide resolved

compiler/src/dotty/tools/dotc/ast/tpd.scala Show resolved Hide resolved

odersky force-pushed the add-multi-tparams branch from 03e5698 to b93b40b Compare January 5, 2021 10:53

odersky and others added 22 commits January 5, 2021 14:11

Allow leading context parameters in extension methods

ec5c9ed

Fixes scala#9530

Fix scala3doc test

a132934

Refine encoding of right-associative extension methods

42c1da8

Leading using clauses now scope over both left and right parameter.

Fix wording according to review suggestions

520e357

Make polyDefDef pass type trees instead of types

7de6d01

This is a first step towards passing reference trees for all parameters together.

make tpd.appliedToArgss also work for type arguments

8a8a4f1

Improve handling of parameters in Tasty format

a53b8fe

Using two different tags for empty parameter lists and parameter splits leads to a more straightforward encoding. Also: shift tags around to create more free slots for single element trees.

Merge type params and term params in DefDef nodes

fb87082

Optimize termParamss

8340bd3

Use single parameter list for tpd.DefDef methods

ba8d0ab

Eliminate polyDefDef def and calls

d3f045f

Allow two type parameter lists in extension methods

8bcbd6e

Disable macro tests

5dd7801

Macro tests seem to use the wrong version of Quotes (non-bootstrapped, where it should be bootstrapped).

Workaround for community-build failure

78562e9

Update reference docs

055a4b5

Fix definition of asExprOf on Tree

55eb393

Update extension docs

871c820

Drop IntegratedTypeArgs

d9bd6cd

Fix inlining with multiple type argument clauses

e08919f

To do this, get rid of all uses of `decomposeCall`.

Add tests with dependencies from second type parameter clause

89fd8f4

Add tests with dependencies from second type parameter clause to first type parameter and extension parameter clauses.

Make overloading resolution work for embedded type params

102bfd5

Make overloading resulution work in the case where type parameters follow value parameters.

Revert patches of asExprOf

5cbe0ab

nicolasstucki force-pushed the add-multi-tparams branch from b93b40b to 5cbe0ab Compare January 5, 2021 13:25

odersky closed this Jan 6, 2021

Sporarum mentioned this pull request Feb 17, 2022

Extension methods ambiguity when type argument is added #14451

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow two type parameter sections in extension methods. #10950

Allow two type parameter sections in extension methods. #10950

odersky commented Dec 29, 2020 •

edited

Loading

odersky commented Jan 1, 2021 •

edited

Loading

odersky commented Jan 1, 2021

odersky commented Jan 1, 2021 •

edited

Loading

odersky commented Jan 1, 2021

odersky commented Jan 2, 2021

dottybot commented Jan 2, 2021

dottybot commented Jan 2, 2021

julienrf left a comment

julienrf Jan 2, 2021

odersky Jan 2, 2021

julienrf Jan 2, 2021

odersky Jan 2, 2021

nicolasstucki commented Jan 4, 2021

odersky commented Jan 4, 2021

soronpo commented Jan 4, 2021

odersky commented Jan 4, 2021

odersky commented Jan 4, 2021

nicolasstucki commented Jan 5, 2021 •

edited

Loading

nicolasstucki commented Jan 5, 2021

odersky commented Jan 6, 2021

odersky commented Jan 6, 2021

Allow two type parameter sections in extension methods. #10950

Allow two type parameter sections in extension methods. #10950

Conversation

odersky commented Dec 29, 2020 • edited Loading

odersky commented Jan 1, 2021 • edited Loading

odersky commented Jan 1, 2021

odersky commented Jan 1, 2021 • edited Loading

odersky commented Jan 1, 2021

odersky commented Jan 2, 2021

dottybot commented Jan 2, 2021

dottybot commented Jan 2, 2021

julienrf left a comment

Choose a reason for hiding this comment

julienrf Jan 2, 2021

Choose a reason for hiding this comment

odersky Jan 2, 2021

Choose a reason for hiding this comment

julienrf Jan 2, 2021

Choose a reason for hiding this comment

odersky Jan 2, 2021

Choose a reason for hiding this comment

nicolasstucki commented Jan 4, 2021

odersky commented Jan 4, 2021

soronpo commented Jan 4, 2021

odersky commented Jan 4, 2021

odersky commented Jan 4, 2021

nicolasstucki commented Jan 5, 2021 • edited Loading

nicolasstucki commented Jan 5, 2021

odersky commented Jan 6, 2021

odersky commented Jan 6, 2021

odersky commented Dec 29, 2020 •

edited

Loading

odersky commented Jan 1, 2021 •

edited

Loading

odersky commented Jan 1, 2021 •

edited

Loading

nicolasstucki commented Jan 5, 2021 •

edited

Loading