Change to new given syntax #7210

odersky · 2019-09-12T17:14:12Z

This PR implements the given syntax changes described in
https://github.com/lampepfl/dotty/tree/master/docs/docs/reference/contextual-new

arturopala · 2019-09-12T17:56:08Z

I'm curious why the colon between identifier and curly brace?

nicolasstucki · 2019-09-12T19:54:47Z

tests/pos/reference/delegates.scala

@@ -152,14 +152,14 @@ object Instances extends Common {
 object PostConditions {
  opaque type WrappedResult[T] = T

-  private given WrappedResult {
+  private given extension WrappedResult: {
    def apply[T](x: T): WrappedResult[T] = x


With the : the it starts looking a bit to similar to a structural type.

Do we really need the : here given that we already have the extension keyword?

odersky · 2019-09-12T20:40:23Z

I have now changed the syntax to

    given ops: extension { ... }

I believe that way it is more regular:

A named given always uses id : after the given keyword.
Use extension instead of a base class if you just define extension methods.

With collective extension parameters it's

    given ops: extension [T](x: S) { ... }

I believe that is clearer than the :-less alternative

   given extension ops[T](x: S) { ... }

This second alternative makes it look like ops is parameterized , which is wrong, of course.

sake92 · 2019-09-13T07:36:28Z

Sorry if I misunderstand something, but comparing
implicit class Indexer[T](collection: Seq[T]) to
given extension Indexer[T](collection: Seq[T]),
they look kinda same right?

Why not have just extension keyword: extension Indexer[T](collection: Seq[T])?
And also anonymous extension[T](collection: Seq[T]), although it reads kinda weird... xD

odersky · 2019-09-13T07:50:58Z

The third variation of the theme drops the extension identifier again. So it's

    given { ... }

    given ops: { ... }

In the end, extension is unnecessary and risks being more confusing than enlightening.

Adding extension makes it look like this is the standard way to specify extension methods. But that's not true. You can perfectly well implement a class and implement its extension methods or add new ones. I.e. the following also defines extension methods:

    given X {
      def (x: T) foo (y: T)
    }

If one adds an extension (since after all, there is an extension method, right?) the meaning changes in unintended ways:

    given extension X {
      def (x: T) foo (y: T)
    }

Now, X is the name of the extension itself instead of the name of the implemented class!

Adding extension after the : is bad in a different way. Compare

    given foo: extension[T](x: T) { ... }

with

    given foo: extensn[T](x: T) { ... }

The two meanings are radically different. In the first case [T] and (x: T) bind new names whereas in the second case they are arguments that use existing names x and T. This is bad since extension has to be a soft keyword, so we cannot rely on syntax highlighting to show us the difference. It's also bad for a casual reader of Scala code who does not know that the identifier extension is used in this way. And, from a standpoint of grammar design I find it unappealing that the precise wording of an identifier determines whether what follows is a binding or using occurrence.

In the end, I believe it is better like this:

The name of a parent in a given can be left out, which means that we define only extension methods and nothing else.
In that case, if the extension method block { ... } is prefixed by a parameter list, that parameter list is applied as a prefix to each method in the extension block.

The rules are quite simple and clear that way.

It's given extension { ... } or given extension ops: { ... }

Change from ``` given extension foo: { ... } ``` to ``` given foo: extension { ... } ``` The new syntax is more regular and pleasing to the eye. It avoids the unusual `: { ... }` symbol combination.

We should get to a new stabilized "given" syntax first. Once that is done, shapeless can be updated and re-integrated.

Drop `given match` and `implict match` syntax. The docs on typeclass derivation still have to be updated to conform to the new terminology.

We have a migration problem for implicit function types and closures. We cannot go in one step from ``` given A => B given (x: A) => B ``` to ``` (given A) => B (given x: A) => B ``` because this becomes ambiguous inside parentheses. We have to migrate using intermediate steps: 1. re-allow delegate instead of given for these types and closures 2. disallow given in the old position, and migrate all code over to delegate 3. allow given in the new position. 4. disallow delegate and migrate all code over to given in the new position.

Replace with `delegate A => B` in tests and with `ImplicitFunctionType[A, B]` in bootstrap code. Also replace `given x: T => e` with `delegate x: T => e`, except in bootstrap code, where the latter is not legal. These measures are necessary for migration to new given scheme.

Drop given parameters that come after the implemented type, since these will not be allowed in the future. We cannot disable the feature right now, since bootstrap code uses it.

The chnages to imports broke the rename test outputs. We should come back o fix this. I'll open an issue.

The new given import syntax required a wholesale refactorings of imports. It's shorter, clearer, and more strictly typed now.

odersky · 2019-09-17T09:03:39Z

This PR is maximally lenient, allowing all kinds of syntaxes:

the final given syntax
the previous given syntax with infix given and as
the delegate syntax

The reason is that bootstrap code currently uses both of the superseded styles. The idea is to merge this PR into the next bootstrap compiler. Then restrict everything to the new syntax and fix all code and remaining tests to conform to it.

To be able to remove remnants of the old given/delegate syntaxes.

nicolasstucki · 2019-09-17T10:09:32Z

community-build/test/scala/dotty/communitybuild/CommunityBuildTest.scala

@@ -171,7 +171,7 @@ class CommunityBuildTest {
    extraSbtArgs  = Seq("-Dscala.build.compileWithDotty=true")
  )

-  @Test def shapeless = test(
+  /*@Test*/ def shapeless = test(


TODO: Open an issue for this

nicolasstucki · 2019-09-17T11:48:54Z

language-server/test/dotty/tools/languageserver/RenameTest.scala

@@ -89,7 +89,7 @@ class RenameTest {
    testRenameFrom(m4)
  }

-  @Test def renameRenamedImport: Unit = {
+  /*@Test*/ def renameRenamedImport: Unit = {


TODO create an issue to update these tests

Removed in scala/scala3#7210

Highlight the `given` in imports as a keyword and names as type entities. As implemented in scala/scala3#7210

Highlight the `given` in imports as a keyword and names as type entities. * As implemented in scala/scala3#7210 * Also fixes scala/scala3#7233

Mocuto · 2019-09-24T07:31:36Z

Where can I see the discussion for why the infix given notation was dropped? I very much preferred it over the change introduced in this PR and was surprised to see the latest Dotty release candidate drop it.

EDIT: #7151

soronpo · 2019-09-24T11:44:03Z

@odersky Could you comment why use given and not extension here for extension methods? I'm not referring to given extension which is indeed redundant, but I don't understand why do we need given at all here.

nicolasstucki reviewed Sep 12, 2019

View reviewed changes

odersky force-pushed the new-given-syntax branch from c259790 to dcbc5b4 Compare September 14, 2019 09:52

odersky changed the title ~~Change to new extension method syntax~~ Change to new given syntax Sep 14, 2019

odersky force-pushed the new-given-syntax branch 4 times, most recently from 3d97e85 to 10668ab Compare September 15, 2019 20:17

odersky mentioned this pull request Sep 16, 2019

Bring back some language server rename tests #7234

Open

odersky added 18 commits September 16, 2019 18:24

Change to new extension method syntax

73d1c0a

It's given extension { ... } or given extension ops: { ... }

Change named extension syntax

77fbcdb

Change from ``` given extension foo: { ... } ``` to ``` given foo: extension { ... } ``` The new syntax is more regular and pleasing to the eye. It avoids the unusual `: { ... }` symbol combination.

Drop extension keyword

35da844

Implement new given syntax

9023963

Temporarily disable shapeless in the community build

b5bec30

We should get to a new stabilized "given" syntax first. Once that is done, shapeless can be updated and re-integrated.

Adapt xml-interpolator to new extension method syntax

4d2f216

Drop implicit match and given match

a6ecc14

Drop `given match` and `implict match` syntax. The docs on typeclass derivation still have to be updated to conform to the new terminology.

fix test

2852095

Drop "delegate for" in tests

5ce4475

Implement new syntax for implicit closures

3933284

Switch to new syntax for implicit function types and closures

aa3f364

Drop trailing given parameters

c5dac5b

Drop given parameters that come after the implemented type, since these will not be allowed in the future. We cannot disable the feature right now, since bootstrap code uses it.

Drop given as in instance definitions

ce973a2

Drop infix given for parameters and arguments

78047d0

Adapt CB projects to new given syntax

b7a80dc

Disable failing rename tests in language server

4b2f9cd

The chnages to imports broke the rename test outputs. We should come back o fix this. I'll open an issue.

odersky added 4 commits September 16, 2019 18:24

Refactor imports and implement new given import syntax

bc00dc6

The new given import syntax required a wholesale refactorings of imports. It's shorter, clearer, and more strictly typed now.

Adapt semanticDB to new given imports

d2cbcc0

Fix newline insertion with imports ending with given

afe3d2f

Change more tests to new given syntax

9fe3f70

odersky force-pushed the new-given-syntax branch from e8ec617 to 9fe3f70 Compare September 16, 2019 16:30

Fix tests

d79c66e

This comment has been minimized.

Sign in to view

Fixes to syntax and docs

d724f2b

smarter assigned nicolasstucki Sep 17, 2019

Re-bootstrap the compiler on the previous commit

32b5382

To be able to remove remnants of the old given/delegate syntaxes.

nicolasstucki approved these changes Sep 17, 2019

View reviewed changes

nicolasstucki merged commit 3d7bb5b into scala:master Sep 17, 2019

nicolasstucki deleted the new-given-syntax branch September 17, 2019 12:11

This was referenced Sep 17, 2019

Remove old delegate/given syntax #7238

Closed

Update Shapeless 3 to new given syntax #7239

Closed

Update broken language server rename tests #7240

Closed

nicolasstucki added a commit to scala/vscode-scala-syntax that referenced this pull request Sep 17, 2019

Remove outdated as (Dotty syntax)

d0b9bbd

Removed in scala/scala3#7210

nicolasstucki mentioned this pull request Sep 17, 2019

Remove outdated as (Dotty syntax) scala/vscode-scala-syntax#53

Merged

nicolasstucki added a commit to scala/vscode-scala-syntax that referenced this pull request Sep 17, 2019

Add given imports syntax

29b046a

Highlight the `given` in imports as a keyword and names as type entities. As implemented in scala/scala3#7210

nicolasstucki added a commit to scala/vscode-scala-syntax that referenced this pull request Sep 17, 2019

Fix #55: Add given imports syntax

666c463

Highlight the `given` in imports as a keyword and names as type entities. * As implemented in scala/scala3#7210 * Also fixes scala/scala3#7233

nicolasstucki mentioned this pull request Sep 17, 2019

Fix #55: Add given imports syntax scala/vscode-scala-syntax#56

Merged

milessabin mentioned this pull request Sep 17, 2019

Update and reenable shapeless in community build #7244

Merged

odersky mentioned this pull request Sep 18, 2019

Decide on final syntax for given parameters #7151

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Change to new given syntax #7210

Change to new given syntax #7210

Uh oh!

odersky commented Sep 12, 2019 •

edited

Loading

Uh oh!

arturopala commented Sep 12, 2019

Uh oh!

nicolasstucki Sep 12, 2019

Uh oh!

nicolasstucki Sep 12, 2019

Uh oh!

odersky commented Sep 12, 2019 •

edited

Loading

Uh oh!

sake92 commented Sep 13, 2019

Uh oh!

odersky commented Sep 13, 2019 •

edited

Loading

Uh oh!

This comment has been minimized.

odersky commented Sep 17, 2019 •

edited

Loading

Uh oh!

nicolasstucki Sep 17, 2019

Uh oh!

nicolasstucki Sep 17, 2019

Uh oh!

Mocuto commented Sep 24, 2019 •

edited

Loading

Uh oh!

soronpo commented Sep 24, 2019

Uh oh!

Uh oh!

Change to new given syntax #7210

Change to new given syntax #7210

Uh oh!

Conversation

odersky commented Sep 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arturopala commented Sep 12, 2019

Uh oh!

nicolasstucki Sep 12, 2019

Choose a reason for hiding this comment

Uh oh!

nicolasstucki Sep 12, 2019

Choose a reason for hiding this comment

Uh oh!

odersky commented Sep 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sake92 commented Sep 13, 2019

Uh oh!

odersky commented Sep 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

odersky commented Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolasstucki Sep 17, 2019

Choose a reason for hiding this comment

Uh oh!

nicolasstucki Sep 17, 2019

Choose a reason for hiding this comment

Uh oh!

Mocuto commented Sep 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

soronpo commented Sep 24, 2019

Uh oh!

Uh oh!

odersky commented Sep 12, 2019 •

edited

Loading

odersky commented Sep 12, 2019 •

edited

Loading

odersky commented Sep 13, 2019 •

edited

Loading

odersky commented Sep 17, 2019 •

edited

Loading

Mocuto commented Sep 24, 2019 •

edited

Loading