-
Notifications
You must be signed in to change notification settings - Fork 1k
SIP-NN - Match infix & prefix types to meet expression rules #674
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
Added comment about dotty
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.
Hello @soronpo,
Thanks for submitting this proposal! I've left some comments that you may find useful. Let me know what you think about them!
For those eager to discuss this proposal (in a more high-level fashion than just reviewing), please head over to the Scala Contributors proposal thread.
|
||
### Extended proposal alternative | ||
It is possible to extend this proposal and allow the developer to annotate the expected associativity and precedence per operation. | ||
(Author's note: I personally don't like this, but if such a solution is better for the community, then I will gladly modify this SIP to reflect that.) |
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 don't think this is a good idea as it adds a significant layer of complexity on the language and forces Scala developers to inspect the library-defined operations to know what's their precedence and how they can be used.
I would propose this is not included in this document and, if someone wants this feature, that a new proposal is submitted!
|
||
|
||
|
||
--- |
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.
Are these dashes intended?
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.
Yes, I like to keep things separate, but since you asked, removed.
|
||
* **Prefix operators bracketless unary use**: While expressions have prefix unary operators, there are none for types. See the [Prefix Operations](http://scala-lang.org/files/archive/spec/2.12/06-expressions.html#prefix-operations) section of the Scala specification. | ||
This is a lacking feature of the type language Scala offers. See also interactions of this feature with other Scala features, further down this text. | ||
(Author's note: Not crucial as infix precedence, but good for completeness) |
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.
Can you remove author's notes? 😄. The document will be exposed to all the community and add a little bit of overhead to the document.
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.
Moving notes to commit description.
* **Infix operator precedence and associativity**: | ||
Infix types are 'mostly' left-associative, | ||
while the expression operations are more intuitive with different precedence weights. | ||
Please see [Infix Types](http://scala-lang.org/files/archive/spec/2.12/03-types.html#infix-types) and [Infix Operations](http://scala-lang.org/files/archive/spec/2.12/06-expressions.html#infix-operations) sections of the Scala specifications for more details. |
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.
Can you roughly explain what the current rules for infix operations are? A little bit of context will help the reader understand what you're proposing.
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.
OK. Gave a short description and provided an example. I can also place a screenshot of the entire section.
Currently scala allows symbol operators (`-`, `*`, `~~>`, etc.) for both type names and definition names. | ||
Unfortunately, there is a 'surprise' element since the two differ in behaviour: | ||
|
||
* **Infix operator precedence and associativity**: |
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.
Should we make these subtitles instead of bold text?
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.
Will gladly do so. What do you mean by 'subtitles'? I'm not that familiar with MD.
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.
Something like '###' would be enough 😄.
This means that if unary prefix types are added, then `+42` will be a type expansion of `unary_+[42]`. | ||
|
||
#### Scala meta | ||
Open question how this SIP affects `scala-meta`. |
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.
Scalameta's parser would need to be changed, but I think this section can go away. The trailing commas SIP doesn't mention Scala Meta (IIRC) and it's a detail not relevant to discuss the proposal in depth (any change to parser rules needs changes in tooling).
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.
OK. Will remove the 'Scala meta' subsection.
--- | ||
|
||
## Backward Compatibility | ||
Changing infix type associativity and precedence affects code that uses type operations and conforms to the current specification. |
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.
FTR, it would be very interesting to see how often type alias precedence is used. If you really care about this (and want to enrich the proposal with high-quality data that helps the Committee decide), you can explore it using Scala Meta. Run it it common Scala projects and give us some stats (note that this is optional 😉).
See the following [Typelevel Scala issue](https://github.com/typelevel/scala/issues/69) for the suggestion. | ||
|
||
### Other languages | ||
Would love some help to complete what happens in different programming 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.
Can we put this in the description instead of the document? 😄
### Other languages | ||
Would love some help to complete what happens in different programming languages. | ||
|
||
### Discussions |
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.
Can we make more explicit that any comment on this proposal should go to the Discourse thread? Something like: "If you're interested in discussing this proposal, head over to this Scala Contributors thread and let me know what you think".
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.
Added at the top.
|
||
### Discussions | ||
[Scala Contributors](https://contributors.scala-lang.org/t/sip-nn-make-infix-type-alias-precedence-like-expression-operator-precedence/471) | ||
|
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.
Can we put this into "Bibliography"?
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.
Done
- Removed author's notes, and Adding them here: 1. Unary types are not crucial as infix type precedence, but good for completeness. 2. For the proposal section, I can copy-paste the specifications from expression and modify them to types, if it so required. 3. Would love some help to complete what happens in different programming languages. 4. I think it is very unlikely that variance annotation will interact with literal types, so the confusion is not that critical. 5. Backward compatibility: I don't know if providing a flag to select the precedence is good or not. IMHO, it is better to create a tool that adds brackets to convert code to the old associativity. @jvican's suggestion to use a tool to analyze libraries and check if they are compatible or not is interesting. Don't know if I have time to make it. It is more useful for me to explore the scala compiler code, and make a PR for this SIP 😄 - Modified the example to `/` instead of `*` because currently dotty has bug and does not allow `*` infix type. - Removed scala meta section. - Removed extended proposal alternative: It is possible to extend this proposal and allow the developer to annotate the expected associativity and precedence per operation. I personally don't like this, but if such a solution is better for the community, then I will gladly modify this SIP to reflect that. See the following [Typelevel Scala issue](typelevel/scala#69) for the suggestion.
--- | ||
layout: sip | ||
disqus: true | ||
title: SIP-NN - Make types behave like expressions |
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 think the current title is too general, it's not clear what is this sip about if you only read the title.
May I suggest:
Types have precedence and associativity rules as expressions
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.
The proposal is more than just infix types.
Match infix & prefix types to meet expression rules
is better?
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 agree with @DarkDimius that a better name is desirable -- this one makes you wonder: "Behave like expression in regard to what?". I think Dmitry's suggestion is good, I would just add 'Make' at the beginning of the sentence: Make types have precedence and associativity rules as expressions
. I'm not sure whether 'like' would be more appropriate in that case, maybe you can choose @soronpo, my non-native English is not that good.
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.
Yeah, I think that's way better @soronpo, let's go with that name!
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.
Thanks for taking into account my feedback @soronpo, I've answered some of your last questions and I think this is good to go. I'd like to merge as soon as we fix the problems in the CI (unknown ETA, but hopefully soon).
Currently scala allows symbol operators (`-`, `*`, `~~>`, etc.) for both type names and definition names. | ||
Unfortunately, there is a 'surprise' element since the two differ in behaviour: | ||
|
||
* **Infix operator precedence and associativity**: |
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.
Something like '###' would be enough 😄.
|
||
--- | ||
## Proposal | ||
The proposal is split into two; type infix precedence, and prefix unary types. Note to the SIP committee: It might be better to vote on the two parts separately. |
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.
Let's do it this way, yes. We'll see how this experiment goes, we haven't actually had this situation happened yet.
```scala | ||
val works : 1 + (2 * 3) + 4 = 11 | ||
val fails : 1 + 2 * 3 + 4 = 11 //left associative:(((1+2)*3)+4))) = 13 | ||
``` |
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.
Yes, good enough.
--- | ||
layout: sip | ||
disqus: true | ||
title: SIP-NN - Make types behave like expressions |
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 agree with @DarkDimius that a better name is desirable -- this one makes you wonder: "Behave like expression in regard to what?". I think Dmitry's suggestion is good, I would just add 'Make' at the beginning of the sentence: Make types have precedence and associativity rules as expressions
. I'm not sure whether 'like' would be more appropriate in that case, maybe you can choose @soronpo, my non-native English is not that good.
- Changed title to reflect more what's inside the SIP. - Visual modifications
Can you please rebase on top of #685? |
Hi @soronpo, are you sure you have rebased from latest master? Can you try again? Drone has worked for @DarkDimius PRs, but something is off here because Drone hasn't even recognised your latest commit (and this happens because |
@jvican I screwed something in the template, because this looks awful at http://docs.scala-lang.org/sips/pending/make-types-behave-like-expressions.html |
It probably has to do with spaces, can you check with the template of the |
Perhaps #687 will resolve this. I compared with |
I think it's the good way to fix it , thanks! 👍 |
//Error! | ||
//Left expands to Plus[Plus[N1,Div[N2,N3]],N4] (Surprising) | ||
//Right expands to Plus[Div[Plus[N1,N2],N3],N4] | ||
implicitly[(N1 + N2 / N3 + N4) =:= (N1 + (N2 / N3) + N4)] |
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 think "Left" and "Right" are mixed up here.
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.
Thanks. Fixed that in #855
👍 for this SIP. Here's perhaps a more concise demonstration of the problem: type +[A, B] = Either[A, B]
type *[A, B] = (A, B)
type O = Nothing
type I = Unit
1 + 1 * 0 == (1 + (1 * 0)) // true
implicitly[(I + I * O) =:= (I + (I * O))] // error: Cannot prove that I + I * O =:= I + (I * O). |
Updated PR in #855 |
No description provided.