Settling some key naming conventions

aturon · July 28, 2014, 5:28am

There are several important naming conventions that still need to be formally settled:

prefix mut_ versus suffix _mut (https://github.com/rust-lang/rust/issues/13660)
mut versus mut_ref (https://github.com/rust-lang/rust/pull/14299)
unwrap, get, get_ref, and take conventions (https://github.com/rust-lang/rust/issues/13159, https://github.com/rust-lang/rust/issues/9784)
iterator functions
conversion functions/methods (https://github.com/rust-lang/rust/issues/7087)

This post tries to resolve them together.

Prefix versus suffix for `mut` qualifier

https://github.com/rust-lang/rust/issues/13660

Currently the libraries are not terribly consistent about how to signal mut variants of functions; sometimes it is by a mut_ prefix, sometimes a _mut suffix, and occasionally with _mut_ appearing in the middle. While there are arguments in favor of each of the positions, we stand to gain a lot by standardizing, and to some degree we just need to make a choice.

The proposed rule is simple: prefer a _mut suffix, except when part of a type name (as in as_mut_slice), where the word mut should appear as it would appear in the type.

Rationale: using a suffix makes it easier to visually group non-mut and mut variants together (including when sorted alphabetically). It puts the emphasis on the functionality, rather than the mut qualifier.

`mut` versus `mut_ref`

https://github.com/rust-lang/rust/pull/14299

Some places in the library, we say things like as_ref and as_mut, and others we say get_ref and get_mut_ref.

In general, when types are written as part of a function name, we often abbreviate those types (e.g. iter).

Proposal: standardize on mut as being the abbreviation for a mutable reference, rather than mut_ref.

Rationale: it’s shorter, and pairs like as_ref and as_mut have a pleasant harmony that doesn’t place emphasis on one kind of reference over the other.

Accessing containers/wrappers/cells

https://github.com/rust-lang/rust/issues/13159, https://github.com/rust-lang/rust/issues/9784

Data structures that contain elements, wrap other data structures, or provide interior mutability all need to provide ways to access their internals. These methods often have variants to access the data by value, by reference, and by mutable reference. Right now, there are a variety of names used for this functionality, in some cases provided as fallible operations, in other cases not.

Containers

For a container with keys/indexes of type K and elements of type V, the proposal is:

// Infallbile element lookup:
fn get(&self, key: &K) -> Option<&V>
fn get_mut(&mut self, key: &K) -> Option<&mut V>

// Convenience for .get(key).map(|elt| elt.clone())
fn get_clone(&self, key: &K) -> Option<V>

// Fallible element lookup:
impl Index<K,V> for ...
impl IndexMut<K,V> for ...

The name find would be deprecated in favor of get under this proposal.

Wrappers/Cells

Prefer specific conversion functions like as_bytes or into_vec whenever possible. Otherwise, use:

// Infallible extraction
fn get(&self) -> &V
fn get_mut(&mut self) -> &mut V
fn unwrap(self) -> V

Wrappers/Cells around `Copy` data

// Infallible extraction
fn get(&self) -> V

`Option` and `Result`

Finally, we have the cases of the Option and Result type, which play a special role around fallibility.

For Option<V>:

// Fallible extraction
fn assert(self) -> V
fn expect(self, &str) -> V

and drop the get_ref and get_mut_ref methods in favor of as_ref().assert() and as_mut().assert().

For Result<V, E>:

// Fallible extraction
fn assert(self) -> V
fn assert_err(self) -> E

The methods like unwrap_or and unwrap_or_else keep their current names.

Rationale: taking all of the above proposals, fallibility is pushed into two clearly marked places: indexing and assert.

Iterator functions

The current iterator conventions are:

fn iter(&self) -> T  // where T implements Iterator<&U>
fn mut_iter(&mut self) -> T  // where T implements Iterator<&mut U>
fn move_iter(self) -> T  // where T implements Iterator<U>

The move name is a holdover from an older Rust, and the mut_ prefix is at odds with the proposal above. Therefore, the new proposed names are:

fn iter(&self) -> T  // where T implements Iterator<&U>
fn iter_mut(&mut self) -> T  // where T implements Iterator<&mut U>
fn into_iter(self) -> T  // where T implements Iterator<U>

Conversion functions

https://github.com/rust-lang/rust/issues/7087

The proposal is just to accept the RFC on conversions found at http://aturon.github.io/style/naming.html, which represents the status quo – which is working quite well.

chris-morgan · July 28, 2014, 6:28am

Here’s some data on the mut thing, filtered from TAGS.vi to only cover mut-related things and to remove tests and with a smidgeon of formatting (but nowhere near enough to conquer Discourse ):

adjust_upvar_borrow_kind_for_mut     src/librustc/middle/typeck/check/regionck.rs fn adjust_upvar_borrow_kind_for_mut(rcx: &Rcx,
as_mut                               src/libcore/any.rs                           fn as_mut<T: 'static>(self) -> Option<&'a mut T> {
as_mut                               src/libcore/any.rs                           fn as_mut<T: 'static>(self) -> Option<&'a mut T>;
as_mut                               src/libcore/option.rs                        pub fn as_mut<'r>(&'r mut self) -> Option<&'r mut T> {
as_mut                               src/libcore/result.rs                        pub fn as_mut<'r>(&'r mut self) -> Result<&'r mut T, &'r mut E> {
as_mut_bytes                         src/libcollections/string.rs                 pub unsafe fn as_mut_bytes<'a>(&'a mut self) -> &'a mut [u8] {
as_mut_ptr                           src/libcollections/vec.rs                    pub fn as_mut_ptr(&mut self) -> *mut T {
as_mut_ptr                           src/libcore/slice.rs                         fn as_mut_ptr(self) -> *mut T {
as_mut_ptr                           src/libcore/slice.rs                         fn as_mut_ptr(self) -> *mut T;
as_mut_ptr                           src/librustrt/c_str.rs                       pub fn as_mut_ptr(&mut self) -> *mut libc::c_char {
as_mut_slice                         src/libcollections/vec.rs                    pub fn as_mut_slice<'a>(&'a mut self) -> &'a mut [T] {
as_mut_slice                         src/libcore/option.rs                        pub fn as_mut_slice<'r>(&'r mut self) -> &'r mut [T] {
as_mut_slice                         src/libcore/slice.rs                         fn as_mut_slice(self) -> &'a mut [T] { self }
as_mut_slice                         src/libcore/slice.rs                         fn as_mut_slice(self) -> &'a mut [T];
as_mut_slice                         src/librustc/middle/subst.rs                 fn as_mut_slice<'a>(&'a mut self) -> &'a mut [T] {
as_mut_slice                         src/librustc/middle/subst.rs                 fn as_mut_slice<'a>(&'a mut self) -> &'a mut [T];
as_mut_slice                         src/libstd/c_vec.rs                          pub fn as_mut_slice<'a>(&'a mut self) -> &'a mut [T] {
as_mut_vec                           src/libcollections/string.rs                 pub unsafe fn as_mut_vec<'a>(&'a mut self) -> &'a mut Vec<u8> {
back_mut                             src/libcollections/dlist.rs                  fn back_mut<'a>(&'a mut self) -> Option<&'a mut T> {
back_mut                             src/libcollections/lib.rs                    fn back_mut<'a>(&'a mut self) -> Option<&'a mut T>;
back_mut                             src/libcollections/ringbuf.rs                fn back_mut<'a>(&'a mut self) -> Option<&'a mut T> {
borrow_mut                           src/libcore/cell.rs                          pub fn borrow_mut<'a>(&'a self) -> RefMut<'a, T> {
call_mut                             src/libcore/ops.rs                           fn call_mut(&mut self, args: Args) -> Result;
check_for_aliasable_mutable_writes   src/librustc/middle/borrowck/check_loans.rs  fn check_for_aliasable_mutable_writes(this: &CheckLoanCtxt,
check_unused_mut_pat                 src/librustc/lint/builtin.rs                 fn check_unused_mut_pat(&self, cx: &Context, pats: &[Gc<ast::Pat>]) {
copy_mut_lifetime                    src/libcore/mem.rs                           pub unsafe fn copy_mut_lifetime<'a, S, T>(_ptr: &'a mut S,
deref_mut                            src/libcore/cell.rs                          fn deref_mut<'a>(&'a mut self) -> &'a mut T {
deref_mut                            src/libcore/ops.rs                           fn deref_mut<'a>(&'a mut self) -> &'a mut Result;
deref_mut                            src/librustrt/exclusive.rs                   fn deref_mut<'a>(&'a mut self) -> &'a mut T { &mut *self._data }
deref_mut                            src/librustrt/local_ptr.rs                   fn deref_mut<'a>(&'a mut self) -> &'a mut T {
deref_mut                            src/libsync/lock.rs                          fn deref_mut<'a>(&'a mut self) -> &'a mut T { &mut *self._data }
expr_mut_addr_of                     src/libsyntax/ext/build.rs                   fn expr_mut_addr_of(&self, sp: Span, e: Gc<ast::Expr>) -> Gc<ast::Expr> {
expr_mut_addr_of                     src/libsyntax/ext/build.rs                   fn expr_mut_addr_of(&self, sp: Span, e: Gc<ast::Expr>) -> Gc<ast::Expr>;
find_mut                             src/libcollections/lib.rs                    fn find_mut<'a>(&'a mut self, key: &K) -> Option<&'a mut V>;
find_mut                             src/libcollections/smallintmap.rs            fn find_mut<'a>(&'a mut self, key: &uint) -> Option<&'a mut V> {
find_mut                             src/libcollections/treemap.rs                fn find_mut<'a>(&'a mut self, key: &K) -> Option<&'a mut V> {
find_mut                             src/libcollections/trie.rs                   fn find_mut<'a>(&'a mut self, key: &uint) -> Option<&'a mut T> {
find_mut                             src/libcollections/trie.rs                   fn find_mut<'r, T>(child: &'r mut Child<T>, key: uint, idx: uint) -> Option<&'r mut T> {
find_mut                             src/libstd/collections/hashmap.rs            fn find_mut<'a>(&'a mut self, k: &K) -> Option<&'a mut V> {
find_mut_with                        src/libcollections/treemap.rs                pub fn find_mut_with<'a>(&'a mut self, f:|&K| -> Ordering) -> Option<&'a mut V> {
from_mutbl                           src/librustc/middle/mem_categorization.rs    pub fn from_mutbl(m: ast::Mutability) -> MutabilityCategory {
from_mutbl                           src/librustc/middle/ty.rs                    pub fn from_mutbl(m: ast::Mutability) -> BorrowKind {
front_mut                            src/libcollections/dlist.rs                  fn front_mut<'a>(&'a mut self) -> Option<&'a mut T> {
front_mut                            src/libcollections/lib.rs                    fn front_mut<'a>(&'a mut self) -> Option<&'a mut T>;
front_mut                            src/libcollections/ringbuf.rs                fn front_mut<'a>(&'a mut self) -> Option<&'a mut T> {
get_mut                              src/libcollections/ringbuf.rs                pub fn get_mut<'a>(&'a mut self, i: uint) -> &'a mut T {
get_mut                              src/libcollections/vec.rs                    pub fn get_mut<'a>(&'a mut self, index: uint) -> &'a mut T {
get_mut                              src/libcore/slice.rs                         fn get_mut(self, index: uint) -> Option<&'a mut T> {
get_mut                              src/libcore/slice.rs                         fn get_mut(self, index: uint) -> Option<&'a mut T>;
get_mut                              src/librustc/middle/subst.rs                 fn get_mut<'a>(&'a mut self, index: uint) -> Option<&'a mut T> {
get_mut                              src/librustc/middle/subst.rs                 fn get_mut<'a>(&'a mut self, index: uint) -> Option<&'a mut T>;
get_mut                              src/librustc/middle/subst.rs                 pub fn get_mut<'a>(&'a mut self,
get_mut                              src/libstd/c_vec.rs                          pub fn get_mut<'a>(&'a mut self, ofs: uint) -> Option<&'a mut T> {
get_mut                              src/libstd/collections/hashmap.rs            pub fn get_mut<'a>(&'a mut self, k: &K) -> &'a mut V {
get_mut                              src/libterm/lib.rs                           fn get_mut<'a>(&'a mut self) -> &'a mut T;
get_mut                              src/libterm/terminfo/mod.rs                  fn get_mut<'a>(&'a mut self) -> &'a mut T { &mut self.out }
get_mut                              src/libterm/win.rs                           fn get_mut<'a>(&'a mut self) -> &'a mut T { &mut self.buf }
get_mut_ref                          src/libcollections/bitv.rs                   pub fn get_mut_ref<'a>(&'a mut self) -> &'a mut Bitv {
get_mut_ref                          src/libcore/option.rs                        pub fn get_mut_ref<'a>(&'a mut self) -> &'a mut T {
get_mut_ref                          src/libstd/io/buffered.rs                    fn get_mut_ref<'a>(&'a mut self) -> &'a mut BufferedWriter<W> {
get_mut_slice                        src/librustc/middle/subst.rs                 fn get_mut_slice<'a>(&'a mut self, space: ParamSpace) -> &'a mut [T] {
index_mut                            src/libcollections/vec.rs                    fn index_mut<'a>(&'a mut self, index: &uint) -> &'a mut T {
index_mut                            src/libcore/ops.rs                           fn index_mut<'a>(&'a mut self, index: &Index) -> &'a mut Result;
is_immutable                         src/librustc/middle/mem_categorization.rs    pub fn is_immutable(&self) -> bool {
is_mutable                           src/librustc/middle/mem_categorization.rs    pub fn is_mutable(&self) -> bool {
mark_variable_as_used_mut            src/librustc/middle/borrowck/check_loans.rs  fn mark_variable_as_used_mut(this: &CheckLoanCtxt,
mk_mut_ptr                           src/librustc/middle/ty.rs                    pub fn mk_mut_ptr(cx: &ctxt, ty: t) -> t {
mk_mut_rptr                          src/librustc/middle/ty.rs                    pub fn mk_mut_rptr(cx: &ctxt, r: Region, ty: t) -> t {
read_all_mut                         src/libstd/collections/hashmap.rs            pub fn read_all_mut<'a>(&'a mut self, index: &FullIndex)
read_mut                             src/libstd/collections/hashmap.rs            pub fn read_mut<'a>(&'a mut self, index: &FullIndex) -> (&'a K, &'a mut V) {
report_reassigned_immutable_variable src/librustc/middle/borrowck/mod.rs          pub fn report_reassigned_immutable_variable(&self,
resolve_immut                        src/libcollections/dlist.rs                  fn resolve_immut<'a>(&self) -> Option<&'a T> {
safe_type_for_static_mut             src/librustc/middle/check_static.rs          fn safe_type_for_static_mut(cx: &ty::ctxt, e: &ast::Expr) -> Option<String> {
tree_find_mut_with                   src/libcollections/treemap.rs                fn tree_find_mut_with<'r, K, V>(node: &'r mut Option<Box<TreeNode<K, V>>>,
try_borrow_mut                       src/libcore/cell.rs                          pub fn try_borrow_mut<'a>(&'a self) -> Option<RefMut<'a, T>> {
unsafe_mut_ref                       src/libcore/slice.rs                         unsafe fn unsafe_mut_ref(self, index: uint) -> &'a mut T {
unsafe_mut_ref                       src/libcore/slice.rs                         unsafe fn unsafe_mut_ref(self, index: uint) -> &'a mut T;
with_mut_ref                         src/librustrt/c_str.rs                       pub fn with_mut_ref<T>(&mut self, f: |*mut libc::c_char| -> T) -> T {
with_resolve_table_mut               src/libsyntax/ext/mtwt.rs                    fn with_resolve_table_mut<T>(op: |&mut ResolveTable| -> T) -> T {
write_mut_qualifier                  src/libdebug/repr.rs                         pub fn write_mut_qualifier(&mut self, mtbl: uint) -> bool {

The ones that are not ending in _mut (with trait definitions removed to reduce duplication) are:

as_mut_bytes
as_mut_ptr × 3
as_mut_slice × 5
as_mut_vec
check_for_aliasable_mutable_writes
check_unused_mut_pat
copy_mut_lifetime
expr_mut_addr_of
find_mut_with
from_mutbl × 2
get_mut_ref × 3
get_mut_slice
is_immutable
is_mutable
mk_mut_ptr
mk_mut_rptr
report_reassigned_immutable_variable
resolve_immut
tree_find_mut_with
unsafe_mut_ref
with_mut_ref
write_mut_qualifier

sfackler · July 28, 2014, 7:11am

This seems reasonable as a whole, but I’m particularly in favor of the Option::unwrap/Result::unwrap rename. The name unwrap is currently used both in that context as well as an infallible context to pull apart wrapper types like MemReader, which makes it hard to visually scan for ugly failure sites that should be cleaned up.

chris-morgan · July 28, 2014, 7:20am

I just realised that I accidentally filtered out all the ones that started with mut_.

mut_bits_to_string     src/librustc/middle/dataflow.rs   fn mut_bits_to_string(words: &mut [uint]) -> String {
mut_bound              src/libcollections/trie.rs        fn mut_bound<'a>(&'a mut self, key: uint, upper: bool) -> MutEntries<'a, T> {
mut_buf_as_slice       src/libcore/slice.rs              pub unsafe fn mut_buf_as_slice<T,
mut_chunks             src/libcore/slice.rs              fn mut_chunks(self, chunk_size: uint) -> MutChunks<'a, T> {
mut_chunks             src/libcore/slice.rs              fn mut_chunks(self, chunk_size: uint) -> MutChunks<'a, T>;
mut_deref              src/libcollections/treemap.rs     fn mut_deref<K, V>(x: &mut Option<Box<TreeNode<K, V>>>)
mut_edge_data          src/librustc/middle/graph.rs      pub fn mut_edge_data<'a>(&'a mut self, idx: EdgeIndex) -> &'a mut E {
mut_inner              src/libsync/comm/mod.rs           unsafe fn mut_inner<'a>(&'a self) -> &'a mut Flavor<T> {
mut_iter               src/libcollections/dlist.rs       pub fn mut_iter<'a>(&'a mut self) -> MutItems<'a, T> {
mut_iter               src/libcollections/ringbuf.rs     pub fn mut_iter<'a>(&'a mut self) -> MutItems<'a, T> {
mut_iter               src/libcollections/smallintmap.rs pub fn mut_iter<'r>(&'r mut self) -> MutEntries<'r, V> {
mut_iter               src/libcollections/treemap.rs     pub fn mut_iter<'a>(&'a mut self) -> MutEntries<'a, K, V> {
mut_iter               src/libcollections/trie.rs        pub fn mut_iter<'a>(&'a mut self) -> MutEntries<'a, T> {
mut_iter               src/libcollections/vec.rs         pub fn mut_iter<'a>(&'a mut self) -> MutItems<'a,T> {
mut_iter               src/libcore/option.rs             pub fn mut_iter<'r>(&'r mut self) -> Item<&'r mut T> {
mut_iter               src/libcore/slice.rs              fn mut_iter(self) -> MutItems<'a, T> {
mut_iter               src/libcore/slice.rs              fn mut_iter(self) -> MutItems<'a, T>;
mut_iter               src/librustc/middle/subst.rs      fn mut_iter<'a>(&'a mut self) -> MutItems<'a, T> {
mut_iter               src/librustc/middle/subst.rs      fn mut_iter<'a>(&'a mut self) -> MutItems<'a, T>;
mut_iter               src/libstd/collections/hashmap.rs pub fn mut_iter<'a>(&'a mut self) -> MutEntries<'a, K, V> {
mut_iter               src/libstd/collections/hashmap.rs pub fn mut_iter<'a>(&'a mut self) -> MutEntries<'a, K, V> {
mut_iter_for_traversal src/libcollections/treemap.rs     fn mut_iter_for_traversal<'a>(&'a mut self) -> MutEntries<'a, K, V> {
mut_iterator           src/libcollections/slice.rs       fn mut_iterator(b: &mut Bencher) {
mut_last               src/libcollections/vec.rs         pub fn mut_last<'a>(&'a mut self) -> Option<&'a mut T> {
mut_last               src/libcore/slice.rs              fn mut_last(self) -> Option<&'a mut T> {
mut_last               src/libcore/slice.rs              fn mut_last(self) -> Option<&'a mut T>;
mut_lower_bound        src/libcollections/treemap.rs     pub fn mut_lower_bound<'a>(&'a mut self, k: &K) -> MutEntries<'a, K, V> {
mut_lower_bound        src/libcollections/trie.rs        pub fn mut_lower_bound<'a>(&'a mut self, key: uint) -> MutEntries<'a, T> {
mut_node_data          src/librustc/middle/graph.rs      pub fn mut_node_data<'a>(&'a mut self, idx: NodeIndex) -> &'a mut N {
mut_null               src/libcore/ptr.rs                pub fn mut_null<T>() -> *mut T { 0 as *mut T }
mut_offset             src/libgreen/context.rs           pub fn mut_offset<T>(ptr: *mut T, count: int) -> *mut T {
mut_pop_ref            src/libcore/slice.rs              fn mut_pop_ref(&mut self) -> Option<&'a mut T> {
mut_pop_ref            src/libcore/slice.rs              fn mut_pop_ref(&mut self) -> Option<&'a mut T>;
mut_ref_slice          src/libcore/slice.rs              pub fn mut_ref_slice<'a, A>(s: &'a mut A) -> &'a mut [A] {
mut_regions            src/librustc/middle/subst.rs      pub fn mut_regions<'a>(&'a mut self) -> &'a mut VecPerParamSpace<ty::Region> {
mut_release_borrow_mut src/libcoretest/cell.rs           fn mut_release_borrow_mut() {
mut_rev_iter           src/libcollections/treemap.rs     pub fn mut_rev_iter<'a>(&'a mut self) -> RevMutEntries<'a, K, V> {
mut_shift_ref          src/libcore/slice.rs              fn mut_shift_ref(&mut self) -> Option<&'a mut T> {
mut_shift_ref          src/libcore/slice.rs              fn mut_shift_ref(&mut self) -> Option<&'a mut T>;
mut_slice              src/libcollections/vec.rs         pub fn mut_slice<'a>(&'a mut self, start: uint, end: uint)
mut_slice              src/libcore/slice.rs              fn mut_slice(self, start: uint, end: uint) -> &'a mut [T] {
mut_slice              src/libcore/slice.rs              fn mut_slice(self, start: uint, end: uint) -> &'a mut [T];
mut_slice_from         src/libcollections/vec.rs         pub fn mut_slice_from<'a>(&'a mut self, start: uint) -> &'a mut [T] {
mut_slice_from         src/libcore/slice.rs              fn mut_slice_from(self, start: uint) -> &'a mut [T] {
mut_slice_from         src/libcore/slice.rs              fn mut_slice_from(self, start: uint) -> &'a mut [T];
mut_slice_to           src/libcollections/vec.rs         pub fn mut_slice_to<'a>(&'a mut self, end: uint) -> &'a mut [T] {
mut_slice_to           src/libcore/slice.rs              fn mut_slice_to(self, end: uint) -> &'a mut [T] {
mut_slice_to           src/libcore/slice.rs              fn mut_slice_to(self, end: uint) -> &'a mut [T];
mut_split              src/libcore/slice.rs              fn mut_split(self, pred: |&T|: 'a -> bool) -> MutSplits<'a, T> {
mut_split              src/libcore/slice.rs              fn mut_split(self, pred: |&T|: 'a -> bool) -> MutSplits<'a, T>;
mut_split_at           src/libcollections/vec.rs         pub fn mut_split_at<'a>(&'a mut self, mid: uint) -> (&'a mut [T], &'a mut [T]) {
mut_split_at           src/libcore/slice.rs              fn mut_split_at(self, mid: uint) -> (&'a mut [T], &'a mut [T]) {
mut_split_at           src/libcore/slice.rs              fn mut_split_at(self, mid: uint) -> (&'a mut [T], &'a mut [T]);
mut_upper_bound        src/libcollections/treemap.rs     pub fn mut_upper_bound<'a>(&'a mut self, k: &K) -> MutEntries<'a, K, V> {
mut_upper_bound        src/libcollections/trie.rs        pub fn mut_upper_bound<'a>(&'a mut self, key: uint) -> MutEntries<'a, T> {

mut_bits_to_string
mut_bound
mut_buf_as_slice
mut_chunks
mut_deref
mut_edge_data
mut_inner
mut_iter × 11
mut_iter_for_traversal
mut_iterator
mut_last × 2
mut_lower_bound × 2
mut_node_data
mut_null
mut_offset
mut_pop_ref
mut_ref_slice
mut_regions
mut_release_borrow_mut
mut_rev_iter
mut_shift_ref
mut_slice × 2
mut_slice_from × 2
mut_slice_to × 2
mut_split
mut_split_at × 2
mut_upper_bound × 2

bachm · July 28, 2014, 9:29am

unwrap on Option/Result is overused, renaming it to assert seems like a good move.

I’m not convinced about the name assert_err though. It sounds like it should fail on Ok. assert_ok is better, or even expect_ok for a bit of symmetry while keeping a distinction.

tbu · July 28, 2014, 10:11am

Yea, I think this is the point. assert_err fails on Ok and assert fails on Err.

bachm · July 28, 2014, 10:32am

assert fails on None or Err respectively. expect fails on None with a custom message. So I would expect what’s currently assert_err to fail on Err, with the ability to read the error details.

tbu · July 28, 2014, 10:46am

I don’t know, it sounds natural to me assert_err – assert that there is an error.

alexcrichton · July 28, 2014, 2:13pm

Deprecating find in favor of get will likely depend on https://github.com/rust-lang/rust/issues/12825 (just as a point of information). What’s your rationale for get vs find as well? The term get clearly says in the return type that it’s fallible, but the name find implies more fallibility to me than get.

The {Option, Result}::unwrap rename to assert seems great!

The renaming of mut_iter to iter_mut is interesting in that it makes into_iter the outlier as the only iterator that isn’t prefixed with iter_. I definitely agree, however, that mut_iter is at odds with the _mut naming conventions at the start.

nikomatsakis · July 28, 2014, 2:22pm

I am basically in favor of this entire proposal. I particularly like the rename to assert.

aturon · July 28, 2014, 3:01pm

@alexcrichton The rationale for get over find is that the term applies more broadly. For example, Vec::find to get a pointer to an element at an index seems a bit strange, since find to me implies some amount of searching. And for wrappers/cells where you don’t have an index, it doesn’t make sense at all.

Just to be clear, when you say “fallible” you don’t mean invokes fail! but rather might not succeed? I agree that find is more suggestive in that sense. (As an aside: this is one reason that I find “failure” a problematic name – it becomes hard to talk about an operation “failing” in the sense of returning Err as opposed to calling fail!.)

We could have the convention be: either find or get, depending on whether searching is involved.

Finally, I agree that into_iter is a bit of a shame (though I think an improvement on move_iter). We could consider _val suffix for “by-value” variants, but I think that’s less clear.

alexcrichton · July 28, 2014, 3:17pm

Sounds good to me! (just wanted to make sure the rationale was written down). I think we should prefer consistency across all containers (like Vec) and get seems god to me.

brson · July 28, 2014, 8:24pm

I’m in favor of the whole thing basically too.

My notes from reading:

Wrappers use get/get_mut to get reference to inside. Contrast with as_ for free conversions. get doesn’t say anything about costs, like ‘as’; some are free, some not. get as unwrap for Copy is wierd. In general, get seems to be overloaded for lots of purposes, but maybe that’s the way it has to be.

I like using the word assert to meain ‘this might fail’.

Iter conventions make things more consistent.

lilyball · July 29, 2014, 12:05am

+1 on everything except for the unwrap() -> assert(). I’m actually surprised that nobody else here has voiced a disagreement with this.

To me, assert() means “validate that a condition is true”. It does not do anything beyond that validation; it certainly doesn’t return a value. If I see a construct like foo.assert(), that looks to me like foo must be some representation of a condition to assert (or alternatively, this is an unconditional assertion being fired in some system represented by foo).

Furthermore, I cannot think of any precedent for assert() actually returning some value derived from its receiver (or from an argument). Similarly, every use of assert() I can think of can be replaced with the equivalent of { let _ = condition; } (for side-effects) without changing the meaning of the program (assuming the assertion would have never failed).

sinistersnare · July 29, 2014, 2:39am

+1 to @eridius on unwrap -> assert , assert has no precedence for returning a value, but i do agree that maybe a different name for unwrap might be useful.

aturon · July 29, 2014, 4:56am

Thanks for the feedback, @eridius. I agree that there’s not precedent for the assert usage here, but:

Although assert doesn’t usually return a value, its other connotations are a good match for what Option::unwrap is currently doing. I think these connotations will serve well to give programmers pause before using the method.
The fact that you write .assert(), without any argument, is an immediate signal that this is a different-than-usual take on assert.
To me it feels similar to expect.

Your read that foo.assert() means that foo represents a condition to be asserted is exactly right: we’re asserting that the variant is Ok, and as a by-product getting its value.

Overall, I don’t think we should let precedent hold us back. OTOH, if you’ve got a good alternative that achieves the same goals, I’d love to hear it!

glaebhoerl · July 29, 2014, 9:19am

What about assert_some(), assert_ok(), and assert_err(), so that it’s more obvious what you’re asserting?

cmr · July 29, 2014, 9:29am

Seems too similar to assert!(foo.is_some())… I do think unwrap is a good name, and it’s not clear to me why we’d want to avoid it. Is the concern the similarity to RefCell::unwrap, and that using the same name for fallible and infallible things is not confusing? I am struggling to find a better name, though…

glaebhoerl · July 29, 2014, 9:45am

FWIW, Haskell calls theirs fromJust, and we could also call ours from_some(), which nicely parallels is_some().

Personally however, I think the long-term benefits of assert_*() in terms of much-needed painful clarity would outweigh the short-term costs of potential confusion with assert!.

huon · July 29, 2014, 10:54am

+1 to assert (or similar): one of the first things I pick up when reviewing code is patterns like

if x.is_some() {
    let y = x.unwrap();
    // ...
}

rather than using the type system properly with a match or one of the Option methods (this is_some/unwrap pairing occurs a lot in beginners’ code). A more “dangerous”-sounding name, that accurately reflects how it should be used, will hopefully guide people to a correct solution.

Topic		Replies	Views
Why was `mut_slice_from` renamed to `slice_from_mut`? bikeshed (deprecated)	5	1567	March 25, 2019
Add mut-abstractions language design	11	1090	April 9, 2024
`str::actually_strip_prefix` libs	11	1059	June 5, 2021
Cleaning up `ref`, `&`, `*` confusion language design	22	1611	August 31, 2022
Use of ref, must, & in let statements guidelines	1	2130	March 25, 2019

Settling some key naming conventions

Prefix versus suffix for mut qualifier

mut versus mut_ref