API conventions cleanup

[aturon]

This PR performs several widespread renaming to bring a number of APIs into line with new conventions.

The details of the conventions can be found here and in the guidelines.

The most important changes are:

• The unwrap operation on Option and Result is now called assert.
• The get_ref, get_mut_ref, and take_unwrap methods on Option are deprecated.
• The standard trio of iterator functions are now iter, iter_mut, and iter_owned.
• More generally, mut variants universally use a suffix _mut rather than a prefix.

There were also a few more minor renamings.

Closes #13660
Closes #13159
Closes #9784

All the original functions remain available in deprecated form. Nevertheless, this is considered a:

[breaking-change]

[aturon]

r? @brson

[lilyball]

The unwrap operation on Option and Result is now called assert.

When did this call get made? Last activity I saw on the discussion, it was not settled. Also of note, #9784 never even mentions the name assert anywhere. IIRC the only discussion that mentioned it was on discuss.rust-lang.org, but that's apparently dead right now so I can't find it.

[lilyball]

Also of note, the commit that updated the guidelines to talk about assert was authored by you and has zero citations for where the decision was made.

[aturon]

The decision was made during an API stabilization meeting, which is the current process for guideline acceptance, and took into account all of the feedback from the discussion.

[aturon]

As an aside, if you're looking for a bit more discussion on it, the assert idea has been kicking around for a while.

[aturon]

See #13159 (comment) in particular.

[lilyball]

Where are the minutes for this meeting? rust-lang/meeting-minutes only contains 3 API review meetings, and none of them mention unwrap/assert (nor do the weekly meetings).

[aturon]

I hadn't realized @brson was posting those -- I will make a PR with the notes from that meeting.

[lilyball]

I'm going to bed right now, but I'm really concerned here that you're changing the name of what's probably one of the most widely-used APIs, without any clear precedent for the name (I get the argument for moving away from unwrap but we don't have any precedent for the name assert used in this fashion), in a very opaque fashion (decided at a meeting, where there's no public minutes), with the decision being something that was never really discussed in the actual tickets (even the comment you linked to that mentions assert says it's not really a great name), and AFAIK the only real discussion on this name was on the Discourse forum, which is still not particularly widely-used and which isn't even accessible right now.

Basically, it feels like this is a decision by fiat, and it makes me very uncomfortable and feeling like the concerns I raised on the Discourse forum were never properly addressed.

I would really appreciate it if you could hold off on the unwrap->assert naming at least until we have more transparency here (e.g. the meeting minutes posted and time to review them).

[aturon]

Minutes PR is here: rust-lang/meeting-minutes#2

"Minutes" is a bit of an overstatement, though -- at API review we generally log the decisions but not the conversation, which would be a significant effort to transcribe. I can look into the possibility of transcriptions for Guidelines discussions in the future, although the idea is that we only have such discussions when there's already a pretty clear consensus on the team.

In the meeting, I made sure that the main arguments against assert were well understood by everyone. But there wasn't much further discussion, because the team already understood those downsides but was still unanimously in favor of assert.

I strongly disagree that this is an opaque process. We had substantial discussion on Discourse -- which was announced as the official venue of Guidelines discussion. Several perspectives were given and points rebutted. Every decision involves tradeoffs, and hence every decision has downsides. The downsides you pointed out about assert are valid, as are those that Niko initially pointed out. Nevertheless, on the whole we felt that the upsides outweighed them, and we made a decision.

[kud1ing]

Have you tried how a longer piece of code looks like with assert instead of unwrap? Also one could add some assert!. My expectation is that this will cause some confusion.

[reem]

I strongly disagree with this change. Option::assert feels very strange and there appears to be no community agreement on this topic. This is an extremely commonly used function and this change basically breaks almost all existing rust code for almost no gain.

assert is really a rather poor name for this because it sounds much more like it is shortcut for assert!(opt.is_some()) and does not at all imply that it is getting the value out of the Option or that it returns a value at all.

[huonw]

This is an extremely commonly used function

This is a good thing?

When code reviewing, I see a crappy code with constructs like if x.is_some() { foo(x.unwrap()) } (etc.) rather than using pattern matching or the higher order methods; this is bad because a single typo (e.g. writing if y.is_some() { foo(x.unwrap()) } or inverting the condition) leads to unhappy programs, and failure is bad, for concrete technical reasons as well as just 'crashing applications suck', e.g. unwinding causes missed optimisation.

We should regard the use of .unwrap as an assertion that the Option or Result is Some or Ok.

[cburgdorf]

I think naming something assert that actually returns a value is very misleading. I'm much in favor of @pnkfelix proposal of unwrap_or_fail.

[arcto]

Do I understand this correctly: unwrap_or and unwrap_or_else remain, but unwrap is renamed to assert? If so, then this asymmetry seems a bit arbitrary.

Why not simply unwrap_or_abort? There seems to be great support for renaming fail! to abort! on discuss.rust-lang.org.

[huonw]

As another general point, one rarely sees fromJust in Haskell code (the equivalent of unwrap), suggesting that the Rust community may be far too free about using unwrap.

@arcto Both of those unwrap_... functions are total, and thus have fairly different behaviour and semantics to unwrap.

[cburgdorf]

@huonw I guess that renaming it to something like unwrap_or_fail/unwrap_or_abort will contribute more to people abandoning this API than renaming it to assert. Renaming it to assert will just be "some sort of a surprising name for a method to get to the value" whereas unwrap_or_fail makes the fallibility explicit.

[faassen]

Has anyone considered using 'ensure' instead of 'assert'? I've seen that used in APIs before. Sorry to add to a naming discussion...

[reem]

Renaming it to something much longer and more cumbersome, like unrwap_or_fail or unrwap_or_abort depending on the outcome of the "renaiming fail!" discussion would do much more to discourage use.

@huonw I'm in agreement with you that this should be used less frequently, but that doesn't mean that it isn't used a lot now and that doesn't mean that this isn't a huge breaking change for marginal benefit, especially with an unclear and confusing name like assert replacing it.

I also echo @kballard s concern in that this process seemed a bit opaque - there was lot discussion but mostly disagreement among the community, and to see something this common changed almost silently is pretty weird.

[darthdeus]

Coming from Haskell I have to say that this doesn't make much sense to me either. fromJust is a sort of extreme measure when you've structured the program in a non-ideal way, and only in a few cases it does make sense.

assert also has its own meaning in the programming world, and this definitely doesn't feel like asserting anything.

As for the if (x.is_some()) { foo(x.unwrap()); }, this really feels like an anti-pattern where pattern matching should be used instead (no pun intended). I'm not sure if unwrap_or_fail is the best name, but discouraging this seems like a good idea.

[arcto]

When writing C++ the standard practice, for some years now, has been to assume that anything might throw an exception, unless when explicitly stated otherwise -- i.e. when the function has been explicitly declared exception safe (noexcept).

The community has reversed its previous position on exception specification. In fact, it was deprecated in C++11.

I think we should learn from this and realise that the same circumstances apply to Rust. Any function might fail for any reason (due to its implementation/environment/etc) unless explicitly marked as safe. IMO Rust lacks in this regard.

The safe position is to assume that unwrap can fail, just like any ordinary function.

What is needed above and beyond this is means to guarantee no fail, as well as transaction safety, analogous to the strong execption safety guarantee in C++.

[huonw]

that doesn't mean that it isn't used a lot now

That doesn't make it right or something that we shouldn't try to address now, before use of Rust really kicks off.

As for the if (x.is_some()) { foo(x.unwrap()); }, this really feels like an anti-pattern where pattern matching should be used instead (no pun intended).

(That was exactly my point in this comment.)

there was lot discussion but mostly disagreement among the community, and to see something this common changed almost silently is pretty weird.

Both of these statements are wrong; the discussion on discuss has

• existed for >2 weeks,
• many comments from a variety of different people,
• far more people supporting the assert change (or small variations) than not: I count two disagreements (and only @kballard's comment had any meaningful content) and a lot of people agreeing (including non-core-team community members).

[mk270]

Coming from OCaml, this whole discussion strikes me as crazy; it's like Rust's type system makes a tradeoff between ceremony and robustness, and some people want a different tradeoff.

Is there really a need for a language-wide mechanism for checking whether an option type has a substantive value or a "none"? People can roll their own pattern match; they're less likely to make mistakes that way anyway

[huonw]

@mk270 you may be misunderstanding this discussion? The discussion about unwrap vs. assert is just renaming the (library-defined) unwrap methods on precisely two types in the standard library (Option and Result), i.e. it is just a library conventions issue, not some fundamental language issue.

[kud1ing]

Having an open and transparent process does not equate to making sure everybody in the community is happy with every decision that we make.

• Only 2 people seem to have objected on http://discuss.rust-lang.org/t/settling-some-key-naming-conventions/269/15
• Only 5 people seem to have decided on this change, according to aturon/meeting-minutes@6e6eadf

Many members of the Rust community (even Mozilla employees) have expressed (strong) disagreement here and on Reddit and i don't think their input was honored in the preceding discussion and voting.

It's one thing to invite a community to discuss matters and then decide one way or another. But it's a different thing when you discuss and vote without people knowing about it and without giving them a chance to give input.

[ben0x539]

iter_owned doesn't seem so popular either and I don't personally get the reasoning for moving away from move. Are there strong feelings about using owned instead of move?

[nathantypanski]

@kud1ing I wasn't going to comment (since I try to avoid using .unwrap() on Option types, and view it as bad practice anyhow and "my fault if I do it"). But seeing how many community members are just coming here to post disagreement, I think I'll offer my opinion in support of this change.

It's one thing to invite a community to discuss and then decide one way or another, but it's a different matter when you discuss things without people knowing about it. This supports the impression of opaqueness.

AFAIK a lot of important discussion (and a lot of bikeshedding) is happening on Discuss right now. It's sort of the development forum. Among that was discussion of this change. I think Mozilla needs the power to make decisions and keep the language moving forward.

As a member of the community I agree with @pcwalton and support the benevolent oligarchy waving its hand on this one.

I have no opinion on the merits of this change, but I strongly disagree that our process is "opaque". This is a controversial change and therefore some people will be disappointed. A decision by "fiat" is precisely what is needed to make controversial changes like this.

[steveklabnik]

I have no opinion on the merits of this change, but I strongly disagree that our process is "opaque". This is a controversial change and therefore some people will be disappointed. A decision by "fiat" is precisely what is needed to make controversial changes like this.

Huuuuuge 👍 here.

Not everyone will be happy with every change. It's important to take feedback into consideration, and to take comments seriously, but that doesn't mean the language is or should be designed by upvote.

[pnkfelix]

I also agree with @pcwalton 's statement that the process here is not "opaque", and that sometimes a decision by "fiat" is sometimes necessary (and that this is probably one of these cases).

I chose to voice my opinion about this change on the discuss forum, but I absolutely do not want that comment there to be used as some sort of basis for blocking the Pull Request here.

[pcwalton]

Many members of the Rust community (even Mozilla employees) have expressed (strong) disagreement here and on Reddit and i don't think their input was honored in the preceding discussion and voting.

We can't honor everybody's input, or we won't be able to make any changes except the simplest, most uncontroversial ones. There have been many times where I've strongly disagreed with changes and they were made anyway. I don't mind that—that's part of software development.

[Florob]

Now, this might just be me, but I don't actually get the huge controversy.

Nobody is disagreeing with the sentiment of the change, but saying (and I agree) that assert() is entirely the wrong name. Yes unwrap() includes an assertion. It also returns a value though.
If I read let x = y.assert() my thoughts would be along the line of "Why does an assertion return a value? What would that value be? Maybe whether the assertion was successful, so a bool? Probably won't fail then."

That said it seems to me most people agree that unwrap() is a terrible name. Hardly anybody is arguing for not renaming the function.
It seems to me that proposing to rename it to e.g.unwrap_or_fail() would be quite uncontroversial. It fulfils all the criteria mentioned. It documents the intention (which assert() doesn't IMHO), and it is easily grepable by reviewers.

The only real argument I have heard against reconsidering the name is "But we decided this at a meeting", which honestly seems a bit childish. I get that this can lead to bikeshedding, and that it needs a dictated decision, but I feel that choosing the one name that has lots of valid arguments against it, instead of any of the others, is misguided.

[Valloric]

Assert would be a terrible name for this method; whoever came up with it seems to be ignoring every other programming language out there where assert means "ensure this is true", not "ensure true and return a value", as well as English itself. There's too much conventional wisdom around and experience with what assert means and none of that includes returning a value, which is the primary use case for the method (the fact that it fails if there's no value is a secondary concern to pretty much every single user).

[engstad]

I am also very negative on assert(). I believe it is a fundamental mistake to assume that Some 2 somehow implies success and None failure. As a matter of fact, it could be the opposite where None is the expected case and the Some is the failure case. 'assert()' would completely miss the meaning in this case. E.g.

if (x.is_some()) { println!("Something bad happened: {}", x.assert()); return; }

I think that unwrap_or_fail() is a far better choice, but I would also consider from_some().

[mitchmindtree]

It would be cool to see a community Poll (perhaps on reddit) in controversial occasions like this, where people add suggestions and everyone can place a vote - not to try and get some gospel answer but at least just to see if something generally agreed upon manages to rise to the top.

[dobkeratops]

IMO... I was perfectly happy with .unwrap(). C++ calls it '.value()' or '.value_or()'. swift calls it ! ... the interesting thing is they deem it important enough for an operator. A short word like 'unwrap()' is a nice compromise. If changing it, consider just making it the same as C++. (or the world could put effort into renaming .value() .unwrap())

Its not the same as 'assert'. You know its potentially none, so in the absence of 'else' its easy to deduce it must have a fail case.
It would be easy enough to make a tool highlight anything that can fail.. its an unambiguous name already.

[pczarn]

Currently we can grep for \bassert[_\b]|[_\b]fail!?\(|\bunwrap\( to find all direct points of failure other than indexing out of bounds.

I think that assertions are more desirable than unnecessary unwrapping. Therefore we want to distinguish both.

With unwrap renamed to *_or_fail, grepping for [_\b]fail!?\( would show major points of failure other than assertions.

[wooque]

+1 for unwrap_or_fail, also assert_some as mentioned in Reddit is great candidate, just assert is terrible name that will make lot of confusion. There should be poll for controversial decisions like these.

[DAddYE]

For what it's worth I strongly agree with @llogiq

I think myOption.assert() doesn't look right. Yes, that's bike shedding, but if it's going to change, at least do it right.

Alternative solutions:

myOption.force()
myOption.or_fail()
myOption.or_bust()
myOption.yes_really()
myOption.is_not_an_option()
myOption.or_your_money_back()

I'm particularly fond of the last one as it will steer programmers away from a function that is best used sparingly.

unwrap, fail, assert whatever, I think they should avoided at all, so I vote for: myOption.is_not_an_option() or myOption.or_your_money_back()

[adrientetar]

While assert connotes deeper consequences than unwrap (I think that's what motivates the changes?) it's actually confusing and imo less meaningful. Plus unwrap is (afaik, admittedly) quite specific to Rust and not immediately intuitive at least coming from C so you're more-or-less forced to lookup its meaning before using it – if I'm getting the reasoning behind the change here.

[lilyball]

As far as I'm aware, the primary motivator for changing unwrap() is the fact that we have other APIs that have an unwrap() method that cannot fail, and the idea was to make unwrap() be generically understood as a not-failing method.

My counter is that Option::unwrap() is almost certainly the most common usage by far of the method, and that if you want to have consistent failure semantics, the other APIs that have unwrap() should have their method renamed to something new. That said, I think the name unwrap() is short enough and evocative enough that I'm not sold on the failure semantics thing. The rule can be as simple as "if called on a single-valued type, it can't fail. If called on a multi-value type (like Option or Result), it fails if it doesn't contain the correct value".

[abonander]

I already posted this on the Reddit thread, but I just wanted to put in my two cents here. Has .require() been suggested yet? It usually appears in the context of dynamic imports in PHP or Ruby, but I think it makes sense here. You're depending on that value being available, and you don't want to continue if it's not.

I agree with the reasons above for not using .assert(), mainly because it doesn't suggest returning a value to me.

[sbstp]

Some other ideas:

• get
• acquire
• obtain
• retrieve
• pop
• take

All of those could also begin with try_ (try_retrieve).

[nikomatsakis]

Clearly, this is a controversial topic. Our original plan (see "Consensus and Decisions") was to use Discuss as the forum for discussing guidelines. We thought that the relative consensus on the Discuss thread reflected general agreement. Instead, it seems that a lot of people were just unaware of the proposal.

So we're going to amend our plan for developing guidelines. We can still have discussions on Discuss but afterwards we'll prepare an RFC summarizing the results. Of course it's often not possible to reach complete consensus, and in those cases it's ultimately the responsibility of the core team to finalize decisions and ensure they fit the overall design.

In this case, that means we can close this PR and @aturon will prepare a formal RFC describing the proposed changes and also putting them in a larger context. We will decide on this RFC in the weekly meetings as normal.

I want to emphasize an important distinction between guidelines and library stabilization. As I said, we'll use the RFC process for general guidelines that affect multiple libraries. But we will continue to use the stabilization meetings to make small-bore decisions about individual libraries, such as renaming methods for consistency or removing methods that receive little use. For those sorts of cases, a full RFC is overkill.

[rkjnsn]

While I don't feel terribly strong feelings about it, I do feel that using assert for this purpose feels odd, and would prefer unwrap_or fail or unwrap_or_abort, depending on the result of that discussion.

Furthermore, if we really want to discourage this pattern, I'd be okay with removing the method altogether and making the programmer type .unwrap_or_else(|| fail!()). This has the added advantage of allowing a custom message:

let file = file.unwrap_or_else(|| fail!("Cannot continue: failed to open file"));

[Valloric]

We thought that the relative consensus on the Discuss thread reflected general agreement. Instead, it seems that a lot of people were just unaware of the proposal.

So we're going to amend our plan for developing guidelines. We can still have discussions on Discuss but afterwards we'll prepare an RFC summarizing the results.

Thank you. This is a very reasonable solution.

[lilyball]

@nikomatsakis Thank you. That sounds like the right way to move forward.

[cburgdorf]

In Rust we trust 👍

[liigo]

-1 to assert: assert returns a value is weird.

-1 to opaque process: you (the core team) make this decision and discuss it in the corner (not many people use discuss.rust-lang.org, why not post to reddit or rfcs before this change? ).

[pnkfelix]

@liigo Before responding with such an inflammatory note, it behooves you to at least look to see if there was a comment added when the issue was closed to see if your concerns have been addressed. Which there was in this case, in particular this response from @nikomatsakis : #16436 (comment). (Better still, read over the entire comment thread, if that is feasible.)

This is not the first time your comments have reflected a lack of effort on your part to read over the general context before adding your two cents. I can understand not reading every comment in a long thread, but in this case and others, you have been particularly rude.

[liigo]

@pnkfelix I apologize for my last comment (and maybe old ones), if you think it's "particularly rude". I'll make sure never leave rude comments, I promise. So don't ban me or put me into the blacklist or something else, please! I still want to be active in the Rust community. As a note, I did contribute to Rust source, docs and issues in the past, e.g. the latest #16448 / #16928.

The following is some explain for my last comment (both -1 has its reason in original comment already):

The core team posted on discuss first and quickly made decision only two days later without public meeting report (which was delayed merging for about two weeks). Both the post and the api meeting did not explicitly claim renaming .unwrap() to .assert(). They didn't post on reddit, which is definitely much more popular than discuss.rust-lang.org. As you have seen, many many people were surprised by renaming unwrap to assert. This was the reason why I said "(make this decision and discuss it) in the corner" and "opaque process" in my last comment. I did seen some core members explain "why this is not opaque process" before, but that's not convincing IMO.

[nathantypanski]

@liigo I don't think pnkfelix has the power to "ban you" or "put you into the blacklist." Even if he did, I doubt it would be exercised. As long as you're a helpful member of the community, people will be happy to have you around. In particular, I don't think you need to justify yourself based on your contributions - those actions speak for themselves.

However, everyone participating in this thread is well-aware of all of the points you brought up: they're the core issue at hand, and the reason for nikomatsakis' resolution above. So you're not adding anything to the discussion by simply restating/summarizing the core controversy. Especially in this case - where the issue has been acknowledged, and a long-term solution agreed upon by the core team.

So pnkfenix has a point here: developers dedicate time and energy to reading your posts and placing them in the context of an entire issue. The bare minimum respect you can have for others' time is to read the threads carefully before commenting. This makes the best possible use of everyone's time, including your own, since you won't have to deal with people complaining when you want to get your opinion heard.

[pnkfelix]

Thank you @nathantypanski . I hope that @liigo will understand your note; I'll refrain from commenting further on the matter for now.

[liigo]

Thanks, @nathantypanski , I do understand your note. This issue was closed, I no longer comment here any more.