-
Notifications
You must be signed in to change notification settings - Fork 30
SIP-47 - Clause Interleaving #47
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
Conversation
what about def pair[A1, A2](a1: A1, a2: A2)[B1, B2, B3](b1: B1, b2: B2, b3: B3): (A1, B3) = (a1, b3) or def pair[A1, A2][B1, B2, B3](a1: A1, a2: A2)(b1: B1, b2: B2, b3: B3): (A1, B3) = (a1, b3) |
Yes of course ! And if anything's not clear, feel free to ask |
While I agree that what is proposed in this SIP would be a valuable addition to Scala. I remember myself having to write intermediate classes with However, right now in Scala 3 there are some ways to achieve that. For example, if I'm not mistaken: def getOrElse(key: Key)[V >: key.Value](default: V): V = ??? from the proposal, can be right now defined as def getOrElse: (k:Key) => [V >: k.Value] => (default: V) => V =
(k: Key) => [V] => (default: V) => ??? Of course, the current approach is much less visually pleasing and has some (quite arbitrary) limitations. For example, it is not legal to declare def makePair: [A] => [B] => (A, B) = ??? while I assume your proposal would allow def makePair[A][B]: (A, B) = ??? I think the proposal itself should address that. It should make a statement about why we should add a new thing to the language that will have only very niche use cases, instead of improving already existing mechanisms. |
@Kordyjan, thank you for your feedback ! |
@Kordyjan I added a section addressing what you asked, what do you think ? |
Sorry for not answering earlier. The section indeed addresses what I thought was missing in the proposal. About the proposal itself: I think it is well thought-out. It feels much more like lifting an arbitrary limitation than adding a new feature to the language. As I said, it would have made much code that I wrote in the past clearer and easier to follow. My only fear was that it would make it possible to write unnecessarily complicated signatures for a small benefit, making libraries relying heavily on this feature scary for the newcomers. However, I haven't found anything worse than what the current language features allow library authors to create. Therefore I'm provisionally voting to accept this proposal. However, I'm curious about the opinions of other reviewers. |
@soronpo, @chrisandrews-ms, I have not heard from you, what do you think of the proposal ? |
Apologies, I remembered I was not assigned to it at the beginning, and didn't notice I was assigned after. |
Do you mean
Might not actually be that challenging, as the implementation boils down to "modify the parser", since the generated tree is already valid in the rest of the compiler |
I will add two things to the proposal later today: def getOrElse(k:Key): [V >: k.Value] => (default: V) => V =
(k: Key) => [V] => (default: V) => ??? Poses problem with overloading resolution, if there is another Add a section clarifying that Clause Interleaving also applies to the right-hand side of extension methods |
I remembered scala/scala3#14451 but now I see it will not be resolved.
My guess is that type inference and overloading checks are not that generic. Hoping to be proven wrong. |
Also explains how overloading resolutions make the alternatives unattractive
I'm in favor of accepting too. I can see the utility of this proposal, especially for partial inference of type parameters, a problem that has come up a few times in our Scala 2 codebase. The SIP seems well thought out and uncontroversial. A couple minor questions:
|
That is correct, I'll update the proposal
The implementation is pretty much ready, yes, but it adds it under |
I maintain my recommendation that the Committee should vote during the next meeting and accept the proposal. |
Same |
I also recommend to accept |
Also, recommend accepting |
Out of curiosity, why does the committee believe these two answers should be mutually exclusive? They are both available for non-type parameters. |
To prevent API entropy and abuse by "creative" developers. In short, it was easier to get approval for the strict version than the general version. It's always possible to submit the following SIP in the future, if indeed there are good use-cases. Small well-calculated and steady increments are good for languages. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also recommend accept.
The proposal was accepted during our last SIP meeting! This means that the Committee agrees that the proposal can be implemented as an experimental feature. |
Thank you, I'm happy to hear that ! |
DefDef ::= DefSig [‘:’ Type] ‘=’ Expr | ||
DefSig ::= id [DefParamClauses] [DefImplicitClause] | ||
DefParamClauses ::= DefParamClauseChunk {DefParamClauseChunk} | ||
DefParamClauseChunk ::= [DefTypeParamClause] TermOrUsingParamClause {TermOrUsingParamClause} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this mean that this is not valid anymore:
def foo[A](implicit a: A) = ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, nicely spotted, my intent was for it to be allowed,
but it appears I made a mistake with the grammar
Since the implementation team (which is also myself) can make changes to the proposal, I believe this can fall under this umbrella
I will think about what's the clearest way to fix the grammar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should fix it:
DefDef ::= DefSig [‘:’ Type] ‘=’ Expr
- DefSig ::= id [DefParamClauses] [DefImplicitClause]
+ DefSig ::= id [DefParamClauses] [DefTypeParamClause] [DefImplicitClause]
DefParamClauses ::= DefParamClauseChunk {DefParamClauseChunk}
DefParamClauseChunk ::= [DefTypeParamClause] TermOrUsingParamClause {TermOrUsingParamClause}
There is another quirk of this definition however, it allows to parse def f(a: Int)(b: Int)
in two different ways (one vs two DefParamClauseChunk
), is this an issue ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn’t have a look at the complete grammar but what you propose does not look correct, what about the following?
DefDef ::= DefSig [‘:’ Type] ‘=’ Expr
DefSig ::= id [DefParamClauses]
DefParamClauses ::= DefParamClauseChunk {DefParamClauseChunk}
DefParamClauseChunk ::= DefTypeParamClause | TermOrUsingParamClause | DefImplicitClause
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alas, this allows two consecutive type
clauses, or multiple implicit
clauses.
here's another option:
DefSig ::= id [DefParamClauses] [DefImplicitClause]
DefParamClauses ::= DefParamClauseChunkTypeOpt {DefParamClauseChunkTypeReq}
DefParamClauseChunkTypeOpt ::= [DefTypeParamClause] {TermOrUsingParamClause}
DefParamClauseChunkTypeReq ::= DefTypeParamClause {TermOrUsingParamClause}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, that doesn't work either... perhaps this:
DefSig ::= id [DefParamClauses] [DefImplicitClause]
DefParamClauses ::= [DefTypeParamClause] {DefParamClauseChunk} [TermOrUsingParamClause]
DefParamClauseChunk ::= TermOrUsingParamClause {TermOrUsingParamClause} DefTypeParamClause
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should fix it:
DefDef ::= DefSig [‘:’ Type] ‘=’ Expr - DefSig ::= id [DefParamClauses] [DefImplicitClause] + DefSig ::= id [DefParamClauses] [DefTypeParamClause] [DefImplicitClause] DefParamClauses ::= DefParamClauseChunk {DefParamClauseChunk} DefParamClauseChunk ::= [DefTypeParamClause] TermOrUsingParamClause {TermOrUsingParamClause}There is another quirk of this definition however, it allows to parse
def f(a: Int)(b: Int)
in two different ways (one vs twoDefParamClauseChunk
), is this an issue ?
I didn’t have a look at the complete grammar but what you propose does not look correct
@julienrf Could you expand on this, I can't manage to find a failing example with the above syntax
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I talked about it some more with @julienrf and we came to a very easy and clean solution:
Use the pre-"no type currying" syntax, and add an end of line comment that says "and type clauses cannot be adjacent"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- DefSig ::= id [DefParamClauses] [DefImplicitClause]
+ DefSig ::= id [DefParamClauses] [DefImplicitClause] -- and two DefTypeParamClause cannot be adjacent
- DefParamClauses ::= DefParamClauseChunk {DefParamClauseChunk}
- DefParamClauseChunk ::= [DefTypeParamClause] TermOrUsingParamClause {TermOrUsingParamClause}
- TermOrUsingParamClause ::= DefTermParamClause
- | UsingParamClause
+ DefParamClauses ::= DefParamClause { DefParamClause }
+ DefParamClause ::= DefTypeParamClause
+ | DefTermParamClause
+ | UsingParamClause
I think this syntax is easier to understand than the formal one
@julienrf How should I go about updating the proposal ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please open a PR in this repository with your changes.
DefDef ::= DefSig [‘:’ Type] ‘=’ Expr | ||
DefSig ::= id [DefParamClauses] [DefImplicitClause] | ||
DefParamClauses ::= DefParamClauseChunk {DefParamClauseChunk} | ||
DefParamClauseChunk ::= [DefTypeParamClause] TermOrUsingParamClause {TermOrUsingParamClause} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alas, this allows two consecutive type
clauses, or multiple implicit
clauses.
here's another option:
DefSig ::= id [DefParamClauses] [DefImplicitClause]
DefParamClauses ::= DefParamClauseChunkTypeOpt {DefParamClauseChunkTypeReq}
DefParamClauseChunkTypeOpt ::= [DefTypeParamClause] {TermOrUsingParamClause}
DefParamClauseChunkTypeReq ::= DefTypeParamClause {TermOrUsingParamClause}
UsingParamClause ::= [nl] ‘(’ ‘using’ (DefTermParams | FunArgTypes) ‘)’ | ||
DefImplicitClause ::= [nl] ‘(’ ‘implicit’ DefTermParams ‘)’ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd also rename
TermOrUsingParamClause ::= DefTermParamClause
| UsingParamClause
UsingParamClause ::= [nl] ‘(’ ‘using’ (DefTermParams | FunArgTypes) ‘)’
DefImplicitClause ::= [nl] ‘(’ ‘implicit’ DefTermParams ‘)’
to
DefNonTypeParamClause ::= DefTermParamClause | DefUsingParamClause
DefUsingParamClause ::= [nl] ‘(’ ‘using’ (DefTermParams | FunArgTypes) ‘)’
DefImplicitParamClause ::= [nl] ‘(’ ‘implicit’ DefTermParams ‘)’
As noted in scala#47, the grammar did not correspond to the rest of the proposal, this PR solves that
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: ```scala def foo[A](x: A)[B](y: B) = (x, y) ``` All user-facing details can be found in the [Scala 3 new features doc](https://github.com/lampepfl/dotty/blob/2a079f92bfc05420e90987d908c41589ac94418d/docs/_docs/reference/other-new-features/generalized-method-syntax.md), and in the [SIP proposal](scala/improvement-proposals#47) 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](https://contributors.scala-lang.org/t/clause-interweaving-allowing-def-f-t-x-t-u-y-u) (note however that the description there is somewhat outdated) ### Dependencies This feature has been [accepted by the SIP committee for implementation](scala/improvement-proposals#47 (comment)), it can therefore become part of the language as an experimental feature at any time. The feature will be available with `import scala.language.experimental.clauseInterleaving` ### How to make non-experimental `git revert` the commits named "Make clause interleaving experimental" and "Add import to tests" ### Future Work 1. Implement given aliases with clause interweaving: (to have types depends on terms) ```scala given myGiven[T](using x: T)[U](using y: U) = (x, y) ``` 2. Add interleaved clauses to the left-hand side of extension methods: ```scala extension (using Int)[A](using A)(a: A)[B](using B) def foo: (A, B) = ??? ``` 3. Investigate usefulness/details of clause interweaving for classes and type currying for types: ```scala 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)
As noted in scala#47, the grammar did not correspond to the rest of the proposal, this PR solves that
Edit: Other PRs have since modified this document, for the latest version, go here
We propose to generalize method signatures to allow any number of type parameter lists, interleaved with term parameter lists and using parameter lists. As a simple example, it would allow to define
Here is also a more complicated and contrived example that highlights all the possible interactions:
(taken from the summary section)