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

Stabilise SIP-47 (Adding Clause Interleaving to method definitions) #20861

Merged
merged 3 commits into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/config/Feature.scala
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ object Feature:

def namedTypeArgsEnabled(using Context) = enabled(namedTypeArguments)

def clauseInterleavingEnabled(using Context) = enabled(clauseInterleaving)
def clauseInterleavingEnabled(using Context) =
sourceVersion.isAtLeast(`3.6`) || enabled(clauseInterleaving)
Copy link
Member

@bishabosha bishabosha Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shouldn't be mixing source version with language import checks, they should be kept separate,
e.g. in the parser do

if sourceVersion.isAtLeast(`3.6`) || in.featureEnabled(Feature.clauseInterleaving) then

Copy link
Member Author

@hamzaremmal hamzaremmal Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure to see why we should do that ?
edit: fewerBraces was stabilised in 3.3 and we do similarly:

def fewerBracesEnabled(using Context) =

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I guess its fine then, curious that in parser they just have Feature.fewerBracesEnabled without explicit using

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I was currious too. I guess it might be a no-op as it is, therefore a bug ?

hamzaremmal marked this conversation as resolved.
Show resolved Hide resolved

def genericNumberLiteralsEnabled(using Context) = enabled(genericNumberLiterals)

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/config/SourceVersion.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ enum SourceVersion:
def isAtMost(v: SourceVersion) = stable.ordinal <= v.ordinal

object SourceVersion extends Property.Key[SourceVersion]:
def defaultSourceVersion = `3.5`
def defaultSourceVersion = `3.6`

/** language versions that may appear in a language import, are deprecated, but not removed from the standard library. */
val illegalSourceVersionNames = List("3.1-migration").map(_.toTermName)
Expand Down
7 changes: 1 addition & 6 deletions compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3836,9 +3836,6 @@ object Parsers {

/** DefDef ::= DefSig [‘:’ Type] [‘=’ Expr]
* | this TypelessClauses [DefImplicitClause] `=' ConstrExpr
* DefSig ::= id [DefTypeParamClause] DefTermParamClauses
*
* if clauseInterleaving is enabled:
* DefSig ::= id [DefParamClauses] [DefImplicitClause]
*/
def defDefOrDcl(start: Offset, mods: Modifiers, numLeadParams: Int = 0): DefDef = atSpan(start, nameStart) {
Expand Down Expand Up @@ -3878,13 +3875,11 @@ object Parsers {
val ident = termIdent()
var name = ident.name.asTermName
val paramss =
if in.featureEnabled(Feature.clauseInterleaving) then
bishabosha marked this conversation as resolved.
Show resolved Hide resolved
// If you are making interleaving stable manually, please refer to the PR introducing it instead, section "How to make non-experimental"
if Feature.clauseInterleavingEnabled(using in.languageImportContext) then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks out of line with other code that's version dependent, Why the using in.languageImportContext?
That's done nowhere else in the parser. Why not just the original test?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a doc comment further up that needs to be updated.


     * if clauseInterleaving is enabled:
     *  DefSig  ::=  id [DefParamClauses] [DefImplicitClause]
     * 

As far as Parser is concerned, clauseInterleaving is now enabled, so the grammar should reflect this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks out of line with other code that's version dependent, Why the using in.languageImportContext?
That's done nowhere else in the parser. Why not just the original test?

The issue there is that we also need to check if -source is configured and if it is less than 3.6, we should require the language import. While in.featureEnabled(Feature.clauseInterleaving) only checks that there is an import statement, it doesn't check the source version. Feature.clauseInterleavingEnabled does both of them but it seems that the context doesn't contain that information and the in.languageImportContext does (which is exactly what in.featureEnabled does)

hamzaremmal marked this conversation as resolved.
Show resolved Hide resolved
typeOrTermParamClauses(ParamOwner.Def, numLeadParams)
else
val tparams = typeParamClauseOpt(ParamOwner.Def)
val vparamss = termParamClauses(ParamOwner.Def, numLeadParams)

joinParams(tparams, vparamss)

var tpt = fromWithinReturnType { typedOpt() }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,9 @@
---
layout: doc-page
title: "Generalized Method Syntax"
nightlyOf: https://docs.scala-lang.org/scala3/reference/experimental/generalized-method-syntax.html
nightlyOf: https://docs.scala-lang.org/scala3/reference/other-new-features/generalized-method-syntax.html
---

This feature is not yet part of the Scala 3 language definition. It can be made available by a language import:

```scala
import scala.language.experimental.clauseInterleaving
```

The inclusion of using clauses is not the only way in which methods have been updated, type parameter clauses are now allowed in any number and at any position.

## Syntax Changes
Expand Down Expand Up @@ -51,7 +45,7 @@ trait DB {
}
```

Note that simply replacing `V` by `k.Value` would not be equivalent. For example, if `k.Value` is `Some[Int]`, only the above allows:
Note that simply replacing `V` by `k.Value` would not be equivalent. For example, if `k.Value` is `Some[Int]`, only the above allows:
`getOrElse(k)[Option[Int]](None)`, which returns a `Number`.

## Details
Expand Down
1 change: 1 addition & 0 deletions library/src/scala/runtime/stdLibPatches/language.scala
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ object language:
* @see [[https://github.com/scala/improvement-proposals/blob/main/content/clause-interleaving.md]]
*/
@compileTimeOnly("`clauseInterleaving` can only be used at compile time in import statements")
@deprecated("`clauseInterleaving` is now standard, no language import is needed", since = "3.6")
object clauseInterleaving

/** Experimental support for pure function type syntax
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ import java.nio.file.Path

class SignatureHelpInterleavingSuite extends BaseSignatureHelpSuite:

override protected def scalacOptions(classpath: Seq[Path]): Seq[String] =
List("-language:experimental.clauseInterleaving")

@Test def `proper-position-1` =
check(
"""
Expand Down
2 changes: 0 additions & 2 deletions scaladoc-testcases/src/tests/extensionParams.scala
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ extension (using Unit)(a: Int)
def f14(): Any
= ???

import scala.language.experimental.clauseInterleaving

extension (using String)(using Int)(a: Animal)(using Unit)(using Number)
def f16(b: Any)[T](c: T): T
= ???
Expand Down
2 changes: 0 additions & 2 deletions scaladoc-testcases/src/tests/methodsAndConstructors.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package tests.methodsAndConstructors

import scala.language.experimental.clauseInterleaving

class A
class B extends A
class C
Expand Down
14 changes: 2 additions & 12 deletions tests/neg/given-loop-prevention.check
Original file line number Diff line number Diff line change
@@ -1,14 +1,4 @@
-- Error: tests/neg/given-loop-prevention.scala:10:36 ------------------------------------------------------------------
-- [E172] Type Error: tests/neg/given-loop-prevention.scala:10:36 ------------------------------------------------------
10 | given List[Foo] = List(summon[Foo]) // error
| ^
| Result of implicit search for Foo will change.
| Current result Baz.given_Foo will be no longer eligible
| because it is not defined before the search position.
| Result with new rules: No Matching Implicit.
| To opt into the new rules, compile with `-source future` or use
| the `scala.language.future` language import.
|
| To fix the problem without the language import, you could try one of the following:
| - use a `given ... with` clause as the enclosing given,
| - rearrange definitions so that Baz.given_Foo comes earlier,
| - use an explicit argument.
| No given instance of type Foo was found for parameter x of method summon in object Predef
2 changes: 0 additions & 2 deletions tests/neg/i20415.scala

This file was deleted.

14 changes: 0 additions & 14 deletions tests/neg/i6716.check

This file was deleted.

17 changes: 6 additions & 11 deletions tests/neg/i6716.scala
Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@

trait Monad[T]:
def id: String
class Foo
object Foo {
given Monad[Foo] with { def id = "Foo" }
}

opaque type Bar = Foo
object Bar {
given Monad[Bar] = summon[Monad[Foo]] // error
given Foo with {}
given List[Foo] = List(summon[Foo]) // ok
}

object Test extends App {
println(summon[Monad[Foo]].id)
println(summon[Monad[Bar]].id)
object Baz {
@annotation.nowarn
given List[Foo] = List(summon[Foo]) // error
given Foo with {}
}
30 changes: 7 additions & 23 deletions tests/neg/i7294.check
Original file line number Diff line number Diff line change
@@ -1,25 +1,9 @@
-- Error: tests/neg/i7294.scala:7:10 -----------------------------------------------------------------------------------
7 | case x: T => x.g(10) // error // error
| ^
| Result of implicit search for scala.reflect.TypeTest[Nothing, T] will change.
| Current result foo.f will be no longer eligible
| because it is not defined before the search position.
| Result with new rules: No Matching Implicit.
| To opt into the new rules, compile with `-source future` or use
| the `scala.language.future` language import.
|
| To fix the problem without the language import, you could try one of the following:
| - use a `given ... with` clause as the enclosing given,
| - rearrange definitions so that foo.f comes earlier,
| - use an explicit argument.
|
| where: T is a type in given instance f with bounds <: foo.Foo
-- [E007] Type Mismatch Error: tests/neg/i7294.scala:7:18 --------------------------------------------------------------
7 | case x: T => x.g(10) // error // error
| ^^^^^^^
| Found: Any
| Required: T
|
| where: T is a type in given instance f with bounds <: foo.Foo
-- [E007] Type Mismatch Error: tests/neg/i7294.scala:7:15 --------------------------------------------------------------
7 | case x: T => x.g(10) // error
| ^
| Found: (x : Nothing)
| Required: ?{ g: ? }
| Note that implicit conversions were not tried because the result of an implicit conversion
| must be more specific than ?{ g: [applied to (10) returning T] }
|
| longer explanation available when compiling with `-explain`
2 changes: 1 addition & 1 deletion tests/neg/i7294.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ package foo
trait Foo { def g(x: Any): Any }

inline given f[T <: Foo]: T = ??? match {
case x: T => x.g(10) // error // error
case x: T => x.g(10) // error
}

@main def Test = f
3 changes: 1 addition & 2 deletions tests/neg/interleaving-ab.scala
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import scala.language.experimental.clauseInterleaving

object Ab:
given String = ""
given Double = 0

def illegal[A][B](x: A)(using B): B = summon[B] // error: Type parameter lists must be separated by a term or using parameter list

def ab[A](x: A)[B](using B): B = summon[B]
def test =
ab[Int](0: Int) // error
1 change: 0 additions & 1 deletion tests/neg/interleaving-params.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import scala.language.experimental.clauseInterleaving

class Params{
def bar[T](x: T)[T]: String = ??? // error
Expand Down
1 change: 0 additions & 1 deletion tests/neg/interleaving-signatureCollision.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import scala.language.experimental.clauseInterleaving

object signatureCollision:
def f[T](x: T)[U](y: U) = (x,y)
Expand Down
28 changes: 14 additions & 14 deletions tests/neg/interleaving-typeApply.check
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
-- [E057] Type Mismatch Error: tests/neg/interleaving-typeApply.scala:10:11 --------------------------------------------
10 | f3[String]() // error
| ^
| Type argument String does not conform to upper bound Int
|
| longer explanation available when compiling with `-explain`
-- [E057] Type Mismatch Error: tests/neg/interleaving-typeApply.scala:11:16 --------------------------------------------
11 | f5[Int][Unit] // error
-- [E057] Type Mismatch Error: tests/neg/interleaving-typeApply.scala:9:11 ---------------------------------------------
9 | f3[String]() // error
| ^
| Type argument String does not conform to upper bound Int
|
| longer explanation available when compiling with `-explain`
-- [E057] Type Mismatch Error: tests/neg/interleaving-typeApply.scala:10:16 --------------------------------------------
10 | f5[Int][Unit] // error
| ^
| Type argument Unit does not conform to upper bound String
|
| longer explanation available when compiling with `-explain`
-- [E057] Type Mismatch Error: tests/neg/interleaving-typeApply.scala:12:19 --------------------------------------------
12 | f5[String][Unit] // error // error
-- [E057] Type Mismatch Error: tests/neg/interleaving-typeApply.scala:11:19 --------------------------------------------
11 | f5[String][Unit] // error // error
| ^
| Type argument Unit does not conform to upper bound String
|
| longer explanation available when compiling with `-explain`
-- [E057] Type Mismatch Error: tests/neg/interleaving-typeApply.scala:12:11 --------------------------------------------
12 | f5[String][Unit] // error // error
-- [E057] Type Mismatch Error: tests/neg/interleaving-typeApply.scala:11:11 --------------------------------------------
11 | f5[String][Unit] // error // error
| ^
| Type argument String does not conform to upper bound Int
|
| longer explanation available when compiling with `-explain`
-- [E057] Type Mismatch Error: tests/neg/interleaving-typeApply.scala:13:11 --------------------------------------------
13 | f7[String]()[Unit] // error
-- [E057] Type Mismatch Error: tests/neg/interleaving-typeApply.scala:12:11 --------------------------------------------
12 | f7[String]()[Unit] // error
| ^
| Type argument String does not conform to upper bound Int
|
Expand Down
3 changes: 1 addition & 2 deletions tests/neg/interleaving-typeApply.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import scala.language.experimental.clauseInterleaving

object typeApply:

def f3[T <: Int](using DummyImplicit)[U <: String](): T => T = ???
def f5[T <: Int](using DummyImplicit)[U <: String]: [X <: Unit] => X => X = ???
def f7[T <: Int](using DummyImplicit)[U <: String]()[X <: Unit]: X => X = ???
Expand Down
1 change: 0 additions & 1 deletion tests/neg/interleaving-unmatched.scala
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import scala.language.experimental.clauseInterleaving

object unmatched:
def f1[T (x: T)] = ??? // error
Expand Down
4 changes: 4 additions & 0 deletions tests/neg/interleavingExperimental.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-- [E040] Syntax Error: tests/neg/interleavingExperimental.scala:3:15 --------------------------------------------------
3 |def ba[A](x: A)[B](using B): B = summon[B] // error: clauseInterleaving was experimental until 3.6
| ^
| '=' expected, but '[' found
3 changes: 3 additions & 0 deletions tests/neg/interleavingExperimental.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
//> using options --source 3.5

def ba[A](x: A)[B](using B): B = summon[B] // error: clauseInterleaving was experimental until 3.6
48 changes: 0 additions & 48 deletions tests/neg/looping-givens.check

This file was deleted.

11 changes: 0 additions & 11 deletions tests/neg/looping-givens.scala

This file was deleted.

8 changes: 4 additions & 4 deletions tests/neg/namedTypeParams.check
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,11 @@
| illegal repeated type application
| You might have meant something like:
| Test.f[Y = String, Int]
-- [E102] Syntax Error: tests/neg/namedTypeParams.scala:33:9 -----------------------------------------------------------
33 | f2[Y = String][X = Int](1, "") // error: Y is undefined
-- [E102] Syntax Error: tests/neg/namedTypeParams.scala:32:9 -----------------------------------------------------------
32 | f2[Y = String][X = Int](1, "") // error: Y is undefined
| ^^^^^^
| Type parameter Y is undefined. Expected one of X.
-- [E102] Syntax Error: tests/neg/namedTypeParams.scala:34:9 -----------------------------------------------------------
34 | f2[Y = String](1, "") // error: Y is undefined
-- [E102] Syntax Error: tests/neg/namedTypeParams.scala:33:9 -----------------------------------------------------------
33 | f2[Y = String](1, "") // error: Y is undefined
| ^^^^^^
| Type parameter Y is undefined. Expected one of X.
1 change: 0 additions & 1 deletion tests/neg/namedTypeParams.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ object Test:

object TestInterleaving:
import language.experimental.namedTypeArguments
import language.experimental.clauseInterleaving
def f2[X](using DummyImplicit)[Y](x: X, y: Y): Int = ???

f2[Y = String][X = Int](1, "") // error: Y is undefined
Expand Down
Loading
Loading