Best idiom for a function taking in a trait mutably?


#1

Hi guys, I’m reposting here a question that I posted a month ago in the users forums, but didn’t get a reply. I hope you don’t mind posting here in the internals forum, it might actually not be entirely inappropriate as it touches on what is best idiomatic code, which is tangentially related to language design.

So, I want to write a function that takes in a mutably-borrowed trait argument, and I’m wondering what is the best code for writing the function signature. See these two options below:

use std::io;
use std::io::BufReader;

pub fn reading_func_B<R : io::BufRead + ?Sized>(reader: &mut R) {
    reader.fill_buf().unwrap(); // etc ...
}

pub fn reading_func_C<R : io::BufRead>(mut reader: R) {
    reader.fill_buf().unwrap(); // etc ...
}

fn scratchpad() {
    
    let reader = &mut BufReader::new("string".as_bytes());
    let mut reader_mut = &mut BufReader::new("string".as_bytes());
    
    loop {
        reading_func_B(reader);
        reading_func_C::<&mut _>(reader);
        reading_func_C(reader as &mut _); // alternative calling syntax
        
        reading_func_C(&mut reader_mut); // alternative calling argument
    }
    
}

Which is better between B and C? Are they equivalent in terms of expressive power and performance? And if so, which one would be more idiomatic (notice B and C require different calling syntax for certain arguments)


#2

My understanding is that the two functions are not semantically equivalent.

Remember that in Rust, argument lists are in fact patterns unlike other languages like C++. so in your B function your reader argument is a unique / mutable borrow of type R which is constrained by the traits in the angle brackets whereas in your C function you take ownership of the reader argument. the mut on the left side of reader is not part of the function’s signature. It is equivalent to typing:

pub fn reading_func_C<R: io::BufRead>(reader: R) {
    let mut reader = reader;
    ....
}

in the same way, Rust allows pattern matching & destructuring a tuple argument. The following functions have equivalent signatures:

fn A ( tuple: (i32, i32) ) {}
fn B ( (a, b): (i32, i32) ) {}

#3

This trait is a little special though, because the reference also implemented the trait directly, with:

impl<'a, B: BufRead + ?Sized> BufRead for &'a mut B

So when your function takes something by value implementing this trait, a reference value also works. It gives the caller the choice whether to give you its actual object or just a reference.

Semantically, you should still think about whether it makes sense for your function to consume the reader, though.


#4

Oh wow, I didn’t know about that at all… I thought any borrow of a “MyTrait” trait matched implicitly to the type parameter <T : MyTrait>(myt : R) , but it doesn’t…

It turns out, not just for BufRead, but apparently for many other Traits in the standard library, there is some code that adds a similar impl for the borrow and/or mutable borrow reference. This seems to mask a big language design limitation - has this been discussed before? Or are those impls there because of some sort of backward compatibility issue?


#5

I’m not sure of the history, but I doubt it’s something that can always be inferred. A couple of examples:

  • From::from needs to return a new Self value. That’s fine for impl From<i32> for i64, but the same for &mut i64 couldn’t pull a new reference out of thin air.

  • IntoIterator::into_iter consumes self, and a reference may not be able to mimic the same functionality. In many cases, IntoIterator actually does exist for references too, but they do something different than the base case. For Vec<T> it creates an Iterator which produces T, &Vec<T> produces &T, and &mut Vec<T> produces &mut T. That’s all logical, but not something the compiler could do automatically.

Maybe the compiler could figure this out for traits which never deal with self or Self by value, like AsRef. Call this feature “auto-deref impls”, perhaps.

On the other hand, BufRead and Iterator both have methods which consume self, yet they have blanket &mut T implementations too. I think this works only because their required methods still only need &mut self, so the rest of their methods must have provided implementations that don’t actually need the self by value, even if they’ll consume a self arg. So, maybe this could be inferred too.

Hmm… I think “auto-deref impls” might actually be a good possible feature! (Now to search RFCs, because it seems too simple to not have been thought of already.)


#6

I’m not sure it’s a limitation; references are values just like anything else. I guess it depends on your perspective though :smile:


#7

Well, either “limitation”, or perhaps another word is better: shortcoming, “code smell” -> even though it applies to language design, not actual code.

But it does look a bit odd. libstd has a lot of these:

libstd/collections/hash/map.rs:impl<'a, K, V, S> IntoIterator for &'a HashMap<K, V, S>
libstd/collections/hash/map.rs:impl<'a, K, V, S> IntoIterator for &'a mut HashMap<K, V, S>
libstd/collections/hash/set.rs:impl<'a, 'b, T, S> BitOr<&'b HashSet<T, S>> for &'a HashSet<T, S>
libstd/collections/hash/set.rs:impl<'a, 'b, T, S> BitAnd<&'b HashSet<T, S>> for &'a HashSet<T, S>
libstd/collections/hash/set.rs:impl<'a, 'b, T, S> BitXor<&'b HashSet<T, S>> for &'a HashSet<T, S>
libstd/collections/hash/set.rs:impl<'a, 'b, T, S> Sub<&'b HashSet<T, S>> for &'a HashSet<T, S>
libstd/collections/hash/set.rs:impl<'a, T, S> IntoIterator for &'a HashSet<T, S>
libstd/collections/hash/table.rs:impl<'t, K, V> Put<K, V> for &'t mut RawTable<K, V> {
libstd/ffi/c_str.rs:impl<'a> Default for &'a CStr {
libstd/ffi/os_str.rs:impl<'a> Default for &'a OsStr {
libstd/fs.rs:impl<'a> Read for &'a File {
libstd/fs.rs:impl<'a> Write for &'a File {
libstd/fs.rs:impl<'a> Seek for &'a File {
libstd/io/impls.rs:impl<'a, R: Read + ?Sized> Read for &'a mut R {
libstd/io/impls.rs:impl<'a, W: Write + ?Sized> Write for &'a mut W {
libstd/io/impls.rs:impl<'a, S: Seek + ?Sized> Seek for &'a mut S {
libstd/io/impls.rs:impl<'a, B: BufRead + ?Sized> BufRead for &'a mut B {
libstd/io/impls.rs:impl<'a> Read for &'a [u8] {
libstd/io/impls.rs:impl<'a> BufRead for &'a [u8] {
libstd/io/impls.rs:impl<'a> Write for &'a mut [u8] {
libstd/net/addr.rs:impl<'a> IntoInner<(*const c::sockaddr, c::socklen_t)> for &'a SocketAddr {
libstd/net/addr.rs:impl<'a> ToSocketAddrs for &'a [SocketAddr] {
libstd/net/addr.rs:impl<'a, T: ToSocketAddrs + ?Sized> ToSocketAddrs for &'a T {
libstd/net/tcp.rs:impl<'a> Read for &'a TcpStream {
libstd/net/tcp.rs:impl<'a> Write for &'a TcpStream {
libstd/panic.rs:impl<'a, T: ?Sized> !UnwindSafe for &'a mut T {}
libstd/panic.rs:impl<'a, T: RefUnwindSafe + ?Sized> UnwindSafe for &'a T {}
libstd/path.rs:impl<'a> IntoIterator for &'a PathBuf {
libstd/path.rs:impl<'a> IntoIterator for &'a Path {
libstd/sync/mpsc/mod.rs:impl<'a, T> IntoIterator for &'a Receiver<T> {
libstd/sys/unix/ext/net.rs:impl<'a> io::Read for &'a UnixStream {
libstd/sys/unix/ext/net.rs:impl<'a> io::Write for &'a UnixStream {
libstd/sys/unix/ext/net.rs:impl<'a> IntoIterator for &'a UnixListener {
libstd/sys/unix/fd.rs:impl<'a> Read for &'a FileDesc {
libstd/sys/windows/handle.rs:impl<'a> Read for &'a RawHandle {
libstd/sys/windows/net.rs:impl<'a> Read for &'a Socket {
libstd/sys/windows/stdio.rs:impl<'a> Read for &'a Stdin {

If it’s useful for most traits that the borrow also implicitly matches the trait, then it makes a case that perhaps this should be built-in the language itself. Hence me asking if this was discussed already, there could even have been an RFC already.

It seems useful in the simple case, although it’s not clear to me at the moment the implications it would have for genericity and interactions with other language features.


#8

And also:

libcollections/binary_heap.rs:impl<'a, T> IntoIterator for &'a BinaryHeap<T> where T: Ord {
libcollections/btree/map.rs:impl<'a, K: 'a, V: 'a> IntoIterator for &'a BTreeMap<K, V> {
libcollections/btree/map.rs:impl<'a, K: 'a, V: 'a> IntoIterator for &'a mut BTreeMap<K, V> {
libcollections/btree/set.rs:impl<'a, T> IntoIterator for &'a BTreeSet<T> {
libcollections/btree/set.rs:impl<'a, 'b, T: Ord + Clone> Sub<&'b BTreeSet<T>> for &'a BTreeSet<T> {
libcollections/btree/set.rs:impl<'a, 'b, T: Ord + Clone> BitXor<&'b BTreeSet<T>> for &'a BTreeSet<T> {
libcollections/btree/set.rs:impl<'a, 'b, T: Ord + Clone> BitAnd<&'b BTreeSet<T>> for &'a BTreeSet<T> {
libcollections/btree/set.rs:impl<'a, 'b, T: Ord + Clone> BitOr<&'b BTreeSet<T>> for &'a BTreeSet<T> {
libcollections/enum_set.rs:impl<'a, E> IntoIterator for &'a EnumSet<E> where E: CLike
libcollections/linked_list.rs:impl<'a, T> IntoIterator for &'a LinkedList<T> {
libcollections/linked_list.rs:impl<'a, T> IntoIterator for &'a mut LinkedList<T> {
libcollections/string.rs:impl<'a, 'b> Pattern<'a> for &'b String {
libcollections/vec.rs:impl<'a, T> IntoIterator for &'a Vec<T> {
libcollections/vec.rs:impl<'a, T> IntoIterator for &'a mut Vec<T> {
libcollections/vec_deque.rs:impl<'a, T> IntoIterator for &'a VecDeque<T> {
libcollections/vec_deque.rs:impl<'a, T> IntoIterator for &'a mut VecDeque<T> {
libcollectionstest/string.rs:impl<'a> IntoCow<'a, str> for &'a str {
libcore/array.rs:            impl<'a, T> IntoIterator for &'a [T; $N] {
libcore/array.rs:            impl<'a, T> IntoIterator for &'a mut [T; $N] {
libcore/borrow.rs:impl<'a, T: ?Sized> Borrow<T> for &'a T {
libcore/borrow.rs:impl<'a, T: ?Sized> Borrow<T> for &'a mut T {
libcore/borrow.rs:impl<'a, T: ?Sized> BorrowMut<T> for &'a mut T {
libcore/clone.rs:impl<'a, T: ?Sized> Clone for &'a T {
libcore/cmp.rs:    impl<'a, 'b, A: ?Sized, B: ?Sized> PartialEq<&'b B> for &'a A where A: PartialEq<B> {
libcore/cmp.rs:    impl<'a, 'b, A: ?Sized, B: ?Sized> PartialOrd<&'b B> for &'a A where A: PartialOrd<B> {
libcore/cmp.rs:    impl<'a, A: ?Sized> Ord for &'a A where A: Ord {
libcore/cmp.rs:    impl<'a, A: ?Sized> Eq for &'a A where A: Eq {}
libcore/cmp.rs:    impl<'a, 'b, A: ?Sized, B: ?Sized> PartialEq<&'b mut B> for &'a mut A where A: PartialEq<B> {
libcore/cmp.rs:    impl<'a, 'b, A: ?Sized, B: ?Sized> PartialOrd<&'b mut B> for &'a mut A where A: PartialOrd<B> {
libcore/cmp.rs:    impl<'a, A: ?Sized> Ord for &'a mut A where A: Ord {
libcore/cmp.rs:    impl<'a, A: ?Sized> Eq for &'a mut A where A: Eq {}
libcore/cmp.rs:    impl<'a, 'b, A: ?Sized, B: ?Sized> PartialEq<&'b mut B> for &'a A where A: PartialEq<B> {
libcore/cmp.rs:    impl<'a, 'b, A: ?Sized, B: ?Sized> PartialEq<&'b B> for &'a mut A where A: PartialEq<B> {
libcore/convert.rs:impl<'a, T: ?Sized, U: ?Sized> AsRef<U> for &'a T where T: AsRef<U> {
libcore/convert.rs:impl<'a, T: ?Sized, U: ?Sized> AsRef<U> for &'a mut T where T: AsRef<U> {
libcore/convert.rs:impl<'a, T: ?Sized, U: ?Sized> AsMut<U> for &'a mut T where T: AsMut<U> {
libcore/fmt/mod.rs:impl<'a, W: Write + ?Sized> Write for &'a mut W {
libcore/fmt/mod.rs:        impl<'a, T: ?Sized + $tr> $tr for &'a T {
libcore/fmt/mod.rs:        impl<'a, T: ?Sized + $tr> $tr for &'a mut T {
libcore/fmt/mod.rs:impl<'a, T: ?Sized> Pointer for &'a T {
libcore/fmt/mod.rs:impl<'a, T: ?Sized> Pointer for &'a mut T {
libcore/hash/mod.rs:    impl<'a, T: ?Sized + Hash> Hash for &'a T {
libcore/hash/mod.rs:    impl<'a, T: ?Sized + Hash> Hash for &'a mut T {
libcore/iter/iterator.rs:impl<'a, I: Iterator + ?Sized> Iterator for &'a mut I {
libcore/iter/traits.rs:impl<'a, I: DoubleEndedIterator + ?Sized> DoubleEndedIterator for &'a mut I {
libcore/iter/traits.rs:impl<'a, I: ExactSizeIterator + ?Sized> ExactSizeIterator for &'a mut I {}
libcore/marker.rs:    unsafe impl<'a, T: Sync + ?Sized> Send for &'a T {}
libcore/marker.rs:    unsafe impl<'a, T: Send + ?Sized> Send for &'a mut T {}
libcore/ops.rs:        impl<'a> $imp for &'a $t {
libcore/ops.rs:        impl<'a> $imp<$u> for &'a $t {
libcore/ops.rs:        impl<'a, 'b> $imp<&'a $u> for &'b $t {
libcore/ops.rs:impl<'a, T: ?Sized> Deref for &'a T {
libcore/ops.rs:impl<'a, T: ?Sized> Deref for &'a mut T {
libcore/ops.rs:impl<'a, T: ?Sized> DerefMut for &'a mut T {
libcore/ops.rs:    impl<'a,A,F:?Sized> Fn<A> for &'a F
libcore/ops.rs:    impl<'a,A,F:?Sized> FnMut<A> for &'a F
libcore/ops.rs:    impl<'a,A,F:?Sized> FnOnce<A> for &'a F
libcore/ops.rs:    impl<'a,A,F:?Sized> FnMut<A> for &'a mut F
libcore/ops.rs:    impl<'a,A,F:?Sized> FnOnce<A> for &'a mut F
libcore/ops.rs:impl<'a, T: ?Sized+Unsize<U>, U: ?Sized> CoerceUnsized<&'a mut U> for &'a mut T {}
libcore/ops.rs:impl<'a, 'b: 'a, T: ?Sized+Unsize<U>, U: ?Sized> CoerceUnsized<&'a U> for &'b mut T {}
libcore/ops.rs:impl<'a, T: ?Sized+Unsize<U>, U: ?Sized> CoerceUnsized<*mut U> for &'a mut T {}
libcore/ops.rs:impl<'a, T: ?Sized+Unsize<U>, U: ?Sized> CoerceUnsized<*const U> for &'a mut T {}
libcore/ops.rs:impl<'a, 'b: 'a, T: ?Sized+Unsize<U>, U: ?Sized> CoerceUnsized<&'a U> for &'b T {}
libcore/ops.rs:impl<'a, T: ?Sized+Unsize<U>, U: ?Sized> CoerceUnsized<*const U> for &'a T {}
libcore/option.rs:impl<'a, T> IntoIterator for &'a Option<T> {
libcore/option.rs:impl<'a, T> IntoIterator for &'a mut Option<T> {
libcore/result.rs:impl<'a, T, E> IntoIterator for &'a Result<T, E> {
libcore/result.rs:impl<'a, T, E> IntoIterator for &'a mut Result<T, E> {
libcore/slice.rs:impl<'a, T> Default for &'a [T] {
libcore/slice.rs:impl<'a, T> Default for &'a mut [T] {
libcore/slice.rs:impl<'a, T> IntoIterator for &'a [T] {
libcore/slice.rs:impl<'a, T> IntoIterator for &'a mut [T] {
libcore/str/mod.rs:impl<'a> Default for &'a str {
libcore/str/pattern.rs:impl<'a> CharEq for &'a [char] {
libcore/str/pattern.rs:impl<'a, 'b> Pattern<'a> for &'b [char] {
libcore/str/pattern.rs:impl<'a, 'b, 'c> Pattern<'a> for &'c &'b str {
libcore/str/pattern.rs:impl<'a, 'b> Pattern<'a> for &'b str {

:fearful: :fearful:


#9

Your grep is too broad. This in particular is one of the examples I gave earlier:

libcollections/vec.rs:impl<'a, T> IntoIterator for &'a Vec<T> {
libcollections/vec.rs:impl<'a, T> IntoIterator for &'a mut Vec<T> {

These two and IntoIterator for Vec<T> all have distinct behavior. They all create different Iterator types which yield a different Item type. It happens that the Item matches the reference/indirection in each of these, but they require different code to accomplish this.

I think it’s only interesting to talk about automating those where the reference’s behavior is exactly just a wrapper around the base type’s implementation.


#10

(Note that this is basically true for all the IntoIterator cases, not just Vec.)


#11

Not every trait implemented on a reference is the same case. You only want to search for cases like:

impl <'a, T: Trait + ?Sized> Trait for &'a mut T

There are only a few matches.

AFAIUI the main reason for those impls is that you can call methods constrained with where T: Sized on trait objects.


#12

That both &mut R and R implement Read is super useful: It makes the iterators, structs etc that use R where R: Read by value generic over whether it should own or borrow the reader.


#13

Yup, this is the only case that would make sense to discuss.