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

Unless/Until RFC #2384

Closed
wants to merge 4 commits into from
Closed

Unless/Until RFC #2384

wants to merge 4 commits into from

Conversation

myrrlyn
Copy link

@myrrlyn myrrlyn commented Apr 2, 2018

This RFC reserves two keywords for the next available period of keyword reservation. The unless and until keywords act as logical inverses of if and while, and are intended for use where inversion of the condition being tested is not readily available.

Rendered

@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the RFC. label Apr 2, 2018
}
```

evalueates the conditional `COND`, executing the loop if the test failed and

Choose a reason for hiding this comment

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

Small typo on evalueates here.

Copy link
Author

Choose a reason for hiding this comment

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

:headdesk:

thanks

@guizmaii
Copy link

guizmaii commented Apr 2, 2018

Hi,

I'm not (for now at least) a Rust user. I'm mostly a Scala developer these days and I'm a former Java and Ruby developer.

As a former Ruby developer, I can say that, IMO, the unless keyword is a bad idea.

People tend to write very bad code with this keyword. Adding a lot of double negations where a simple if would have produced simpler code to understand.

This keyword adds more complexity in the code than producing any good effect on written code.

Jules

@myrrlyn
Copy link
Author

myrrlyn commented Apr 2, 2018

@guizmaii,

At present, if is insufficient to write the full set of conditions that may be required in a program, particularly when matching on patterns instead of values. Working around the lack of Boolean operators on patterns creates un-ergonomics syntax and increases the potential for logic errors in the resulting condition.

I fully agree that unless should not be considered an equal choice to if when writing, and expressed a rough guide to when unless would be preferable to if in cases where both keywords would be capable of expressing the desired condition.

unless ! should always trigger a lint warning to replace with if, and I can add a section stating this.
Unfortunately, there is at present no if ! for patterns, which is the primary motivation of the RFC.

Lastly, in my own work with Ruby, I have found unless to have very specific use sites that make it valuable to have, even where if ! is shorter. Since Rust does not have trailing conditionals, would a lint on unless ! or unless {} else {} suffice to satisfy your concerns about ambiguous writing?

@jonhoo
Copy link
Contributor

jonhoo commented Apr 2, 2018

Re negative pattern matching, this has been discussed some over in #929 (comment), and I know @petrochenkov has been working on more expressive if let patterns. I share some of @guizmaii's concerns about unless and until adding additional noise to code, but I think I could be swayed either way. I don't think adding these just for the purposes of negative pattern matching is a good reason though.

@Centril
Copy link
Contributor

Centril commented Apr 2, 2018

Nominating for triage during the next lang-team meeting on Thursday as we need to deal with any keyword reservations speedily.

@myrrlyn
Copy link
Author

myrrlyn commented Apr 2, 2018

@jonhoo, honestly, I expect that this will wind up being primarily a driver for a full Boolean algebra on patterns. Short of while let !, though, there's not a nice way to express looping until a stop pattern and in my personal experience, that occurs enough that sugar would be nice. If I turn out to be in the minority, so be it.

@sgrif
Copy link
Contributor

sgrif commented Apr 2, 2018

This RFC represents a lot of what I was afraid of when epochs/editions were proposed. The only thing worse than a breaking change to support a feature is a breaking change "just in case". While I can't find the exact line (#2052 was a long RFC), there is text in that RFC along the lines of "epochs allowing minor breaking changes should not be used as an excuse to make breaking changes". This RFC feels like the direct antithesis of that.

}
else {
// do significant work on the positive execution
}
Copy link
Contributor

Choose a reason for hiding this comment

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

if !(execute_machine() is FsmState::Stop) { ... }
if execute_machine() is not FsmState::Stop { ... }
if execute_machine() !is FsmState::Stop { ... }

#929 (comment)
#2260 (comment)
:)

Copy link
Author

Choose a reason for hiding this comment

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

I knew I'd miss out on existing knowledge, thanks!

"these two concepts are logically equivalent" (`Eq` trait, `==` and `!=`
operators) and "this value is shaped like that pattern" (`let`, `match`).

Another concept is to introduce a *negative binding*, `!let`, which does not
Copy link
Member

Choose a reason for hiding this comment

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

I personally would like to go all out and have both if !let ... and if let ... && ... work, making let in a conditional expression basically a matches! with binding. However, this does make "let-as-expression" more of a thing but it's still this weird kind of expression that's only an expression in certain contexts. So idk.

@AlexEne
Copy link
Member

AlexEne commented Apr 3, 2018

Let's start from this example (from the rfc):

unless COND {
    cond_is_false();
}
else {
    cond_is_true();
}

Is unless a while not, or a if not? Skimming over the code can confuse newbies, or even seasoned developers that didn't get a chance to see this construct before.
But let's leave that aside, let's say we learn what they mean.

Now let's imagine the following to be possible and valid code:

unless !COND {
    cond_is_true();
}
else {
    cond_is_false();
}

The first example already confuses me, but the second being an option is super scary me and probably any newcomers.

if/while statements can get really ugly as they are now in programming, negating everything with some keywords and allowing boolean expressions inside them but reversing the meaning of the blocks executed is something that may not be worth for the benefits it brings.
I am unsure what a better solution would be, but I think I prefer the loop { if let break } version over this new keyword.

Probably a better option would like to make if let statements more like if statements, allowing negation in them (if not let / if !let even if let {} else if let {not currently possible} ).

@myrrlyn
Copy link
Author

myrrlyn commented Apr 3, 2018

@AlexEne, unless corresponds to if, and if I did not make this sufficiently clear then I apologize and would appreciate ambiguous snippets to improve.

Please see commit 43ad167 for unless !; if this is still unclear I can improve the language.

@Pistahh
Copy link

Pistahh commented Apr 3, 2018

In some (human) languages there is no direct one word translation for "unless", e.g. in my native language (hungarian) it is roughly translated to "if not only". Each time when I see "unless" in some code (I had Perl in my past) I have to stop, then negate the whole expression in my head and pretend that it was an "if" statement. It is even more hard and confusing if it has "not" after an "and"/"or". Having the same condition with "if" may be more complicated, but at least it is written down.

And I heard the same complain from some British people too.

"Unless" may look simpler if the condition is simple, but once it has more components, some negated, some not, it just becomes extra complicated to understand.

With these in mind, please consider that for me most of the examples demonstrating how "unless" would make code simpler makes it actually harder to understand.

Please keep the language simple. "Unless" just complicates it.

@skade
Copy link
Contributor

skade commented Apr 3, 2018

I agree with @sgrif's comment strongly. There seems to be a rush to block things for edition 2018 to allow RFCs to be written on top of these that might themselves not succeed. I also see a high chance that until is used as a variable name in projects.

This should either be run as a fully-fledged RFC, with the goal for implementation being edition 2018, or none. Currently, it's only a collection of (advanced) reasoning why such a change could be useful, but at the end only argues for blocking these words, as they could be used for something. This is definitely a weak reasoning for a change that will likely have practical impact when porting over to 2018.

Also, an argument for != could be made, as it is currently disallowed syntax. It wouldn't be breaking, mitigating a lot of issues.

@BurntSushi
Copy link
Member

I have always found unless/until difficult to reason through, although that may be a personal quirk of mine. Regardless, because of that, I hope that it doesn't find its way into Rust.

@clarfonthey
Copy link
Contributor

clarfonthey commented Apr 3, 2018

I'm really, really against unless anduntil because of how confusing it can get to mix them with if and while. Plus, they really only feel more natural to English speakers, and don't translate well.

IMHO, just take these keywords and bury them; Ruby and CoffeeScript have them but even in those languages I prefer to not use them because of the above reasons

@Ichoran
Copy link

Ichoran commented Apr 3, 2018

If there are places where logical inversion is not readily accessible, let's make it accessible rather than adding new keywords.

For instance, if there were a "bool-let-expr" where any let statements act as pattern matches that are true if they succeed and add the bound variables (if true and universal) to both the rest of the expression and the following block, then you would get full boolean algebra with pattern matching, and no extra keywords.

@clarfonthey
Copy link
Contributor

@skade that came off as way angrier than I intended; I've edited my comment. :)

@scottmcm
Copy link
Member

scottmcm commented Apr 4, 2018

I'm definitely opposed to "unless A is just if !(A)", since then I don't know what to use.

The version of this I could like, and have articulated before, is for guards. Note, for example, that assert!(some(expression).here) is if !some(expression).here { panic!() }, so there is accepted value in the "what I want to be true for the rest of the function" phrasing. Extending that thought pattern to unless some(expression).here { return None } seems quite reasonable -- so long as, and this is critical, the body of the unless diverges (-> !) and an else is prohibited.

Edit: As another example, failure's ensure!(cond, error) is unless cond { bail!(error) }.

@pwoolcoc
Copy link

pwoolcoc commented Apr 4, 2018

I'll join in with the comments against this. I've done perl professionally and, though I eventually trained myself to replace "unless" with "if not" in my head, it's just extra cognitive load that is unnecessary.

I do wish rust had gone with keywords for !, &&, and ||, especially since I find if not some_condition() much more readable than if !some_condition() since it's easier to see the negation, but that ship sailed long ago

@Centril Centril self-assigned this Apr 12, 2018
@joshtriplett
Copy link
Member

We discussed this in the @rust-lang/lang meeting today, and the general consensus was to close. Nobody was particularly enthusiastic about this change, or about seeing the language go in this direction. (There was a little bit of discussion about specific use cases or aspects of matching, related to some of the if not let ideas going around at the moment, but nothing strong enough to support the general addition of unless or until.)

@rfcbot fcp close

@rfcbot
Copy link
Collaborator

rfcbot commented Apr 12, 2018

Team member @joshtriplett has proposed to close this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added the proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. label Apr 12, 2018
@rfcbot
Copy link
Collaborator

rfcbot commented Apr 19, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. and removed proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. labels Apr 19, 2018
@warlord500
Copy link

the only advantage, I could really see if is that unless and until are used to express negative patterns, adding more ways to do a thing, usually just leads to trouble. the only major reason, you should have multiple ways to do things if that specific that method reduces boiler plate for a common purpose.

I have no problem reading many conditions with not in it. many times there is a question of where to put the not but adding one more certainly wont help.

@rfcbot
Copy link
Collaborator

rfcbot commented Apr 29, 2018

The final comment period is now complete.

@Centril
Copy link
Contributor

Centril commented Apr 29, 2018

Thank you @myrrlyn for the RFC!

Per review #2384 (comment), this RFC is hereby closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.