-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fewer braces are part of the language from 3.3.x #14561
Conversation
@odersky I wanted to enable this feature, but it seems not everything is ironed-out just yet. Error: 29 | assertTrue((new js.Object: Any).isInstanceOf[Object])
Error: | ^
Error: | an identifier expected, but ')' found If we sill consider the above a legal syntax, then there seem to be a bug in the implementation of |
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.
We can't just enable a feature like this. It needs to get discussed and approved by the SIP committee first.
I would personally be very much in favor of including this in 3.2. |
I completely lost track of the governance for language changes. I have not seen the SIP committee convene in ages, and that includes the majority of Scala 3 changes that occurred and still occurring. It would be helpful if we were consistent about applying this policy. |
My understanding is that the scala center is in the process of restarting the sip committee but I don't know what the timeline is. I don't think we made any language change since 3.0 that would have required going through it so far. |
I pushed the fix in #14562 to see if all tests pass, including the community build, if this feature is always enabled. |
I would say |
The whole point of experimental is to add things to the compiler before they've been accepted as official language changes so we can gain experience with them. |
Of course, but that was a change to the language to allow that. |
401ac97
to
db5b654
Compare
db5b654
to
b0e1031
Compare
Seems some neg check files need to be updated. |
Since no meetings have been held since this post, I guess the list under "no concensus or no (closed) public discussion" in https://contributors.scala-lang.org/t/scala-3-feature-status-overview/4165/6 is somewhat accurate, and it includes a bunch of stuff which is already part of Scala 3 which probably would have been in the category of "required to go through SIP process"? Including indentation syntax. If the SIP committee now wants to get involved in a tweak to that feature, as this is, well, ok, but please hurry instead of just blocking this because there will be some meetings at some point in the future (it sounds there's not even a date set, if you don't know the timeline to even begin). |
Agreed, bu this should be discussed on https://contributors.scala-lang.org/, not here. |
The file @deprecated def f = 0
def t7a = f: @nowarn("cat=deprecation")
def t7b = f:
@nowarn("msg=deprecated")
def t7c = f: // warning (deprecation)
@nowarn("msg=fish") // error (unused nowarn) What is the meaning of |
It does collide, but I think that's OK. We cannot have a type ascription with a newline between a def f(x: A):
B should work. But that's as far as it goes. I don't think we can go further; the example in the test clearly has a different meaning under fewer braces. I also don't think we'll see code like this in the wild. If you must span several lines, then it should be done like this: def t7b = f
: @nowarn("msg=deprecated") |
OK, I'll update the test accordingly. I also doubt this kind of syntax exists in the wild, and even if it does, there shouldn't be a problem to update it to accommodate the |
It now fails with the following:
Why are experimental flags part of the binary compatibility check? |
I guess they shouldn't be. @dwijnand Do we need a Mima exclude, or would this work anyway in the next minor version? |
If we annotate the |
Does this change need to happen in this PR or should I add Mima exclusions? |
Exclusions are fine. |
I'd love to see this back in the 3.2 release. |
Me too |
other support from the forum https://contributors.scala-lang.org/t/make-fewerbraces-available-outside-snapshot-releases/5024/48 |
3.2 or bust! My body needs this 😉 😄 |
I know this is going to disappoint some people, but I think we should hold off this change and make it go through the SIP process (which should restart soon, but not in time for the 3.2.0 release, unfortunately). |
@julienrf Is there an explanation why extension method language changes got merged without SIP approval and "fewerBraces" requires it? |
@soronpo There were so many changes in Scala 3 that the Scala Improvement Process was not strictly followed otherwise that would have been too inefficient. The “short process” was described in this thread. In summary, while there was no proper proposal, those changes have still been discussed by the committee, and the community was also consulted (here). That being said, what makes fewerBraces a little bit different from other features such as extension methods is that it may affect the code we write every day, in lots of places. |
Oh, that’s a good point. I see that Jamie has followed up in the meantime. |
makes sense, but shiny new, want now :D (I agree, however!) |
I think we'll close for now, and will re-open when the SIP gives this a green light. |
This PR removes the
language.experimental.fewerBraces
flag and always enables the "fewerBraces" syntax feature.