Skip to content

fn foo(&mut self) with a &mut [T] called with x.foo() copies the x slice instead of passing by reference #19147

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
erickt opened this issue Nov 20, 2014 · 5 comments · Fixed by #19617
Milestone

Comments

@erickt
Copy link
Contributor

erickt commented Nov 20, 2014

Here's an example:

use std::raw;
use std::mem;
use std::slice;
use std::io;
use std::io::{IoResult, IoError};

trait MyWriter {
    fn my_write(&mut self, buf: &[u8]) -> IoResult<()>;
}

impl<'a> MyWriter for &'a mut [u8] {
    fn my_write(&mut self, buf: &[u8]) -> IoResult<()> {
        let x: *mut &mut [u8] = self;
        println!("MyWriter ptr: {}", x);

        // return an error if the entire write does not fit in the buffer
        let write_len = buf.len();
        if write_len > self.len() {
            return Err(IoError {
                kind: io::OtherIoError,
                desc: "Trying to write past end of buffer",
                detail: None
            })
        }

        slice::bytes::copy_memory(*self, buf);

        println!("before: {}", self);

        unsafe {
            *self = mem::transmute(raw::Slice {
                data: self.as_ptr().offset(write_len as int),
                len: self.len() - write_len,
            });
        }

        println!("after:  {}", self);

        Ok(())
    }
}

fn main() {
    let mut buf = [0_u8, .. 6];
    println!("ptr: {}", buf);

    {
        let mut writer = buf.as_mut_slice();
        println!("writer: {}", writer);
        writer.my_write(&[0, 1, 2]).unwrap();
        println!("writer: {}", writer);
        writer.my_write(&[3, 4, 5]).unwrap();
        println!("writer: {}", writer);
    }
    println!("buf:    {}", buf);
    println!("");

    let mut buf = [0_u8, .. 6];
    println!("buf:    {}", buf);

    {
        let mut writer = buf.as_mut_slice();
        println!("writer: {}", writer);
        (&mut writer).my_write(&[0, 1, 2]).unwrap();
        println!("writer: {}", writer);
        (&mut writer).my_write(&[3, 4, 5]).unwrap();
        println!("writer: {}", writer);
    }
    println!("buf:    {}", buf);
    println!("");

    let mut buf = [0_u8, .. 6];
    println!("buf:    {}", buf);

    {
        let mut writer = buf.as_mut_slice();
        println!("writer: {}", writer);
        MyWriter::my_write(&mut writer, &[0, 1, 2]).unwrap();
        println!("writer: {}", writer);
        MyWriter::my_write(&mut writer, &[3, 4, 5]).unwrap();
        println!("writer: {}", writer);
    }
    println!("buf:    {}", buf);
    println!("");
}

This prints out the following output. You can see that using the writer.my_write(...) form passes in a different self address for each call, whereas explicitly using a &mut writer passes in the same address.

cc @nikomatsakis and @aturon. Niko on irc was suspecting this might be related to auto-ref-ref.

@erickt
Copy link
Contributor Author

erickt commented Nov 20, 2014

Oops, left out the output:

ptr: [0, 0, 0, 0, 0, 0]
writer: [0, 0, 0, 0, 0, 0]
MyWriter ptr: 0x7fff530d5528
before: [0, 1, 2, 0, 0, 0]
after:  [0, 0, 0]
writer: [0, 1, 2, 0, 0, 0]
MyWriter ptr: 0x7fff530d5408
before: [3, 4, 5, 0, 0, 0]
after:  [0, 0, 0]
writer: [3, 4, 5, 0, 0, 0]
buf:    [3, 4, 5, 0, 0, 0]

buf:    [0, 0, 0, 0, 0, 0]
writer: [0, 0, 0, 0, 0, 0]
MyWriter ptr: 0x7fff530d5170
before: [0, 1, 2, 0, 0, 0]
after:  [0, 0, 0]
writer: [0, 0, 0]
MyWriter ptr: 0x7fff530d5170
before: [3, 4, 5]
after:  []
writer: []
buf:    [0, 1, 2, 3, 4, 5]

buf:    [0, 0, 0, 0, 0, 0]
writer: [0, 0, 0, 0, 0, 0]
MyWriter ptr: 0x7fff530d4cc8
before: [0, 1, 2, 0, 0, 0]
after:  [0, 0, 0]
writer: [0, 0, 0]
MyWriter ptr: 0x7fff530d4cc8
before: [3, 4, 5]
after:  []
writer: []
buf:    [0, 1, 2, 3, 4, 5]

@nikomatsakis
Copy link
Contributor

This seems to be a recent regression, but I'm not quite sure where yet. Method resolution seems to be resolving as I expect, so it could be a trans error, but I'm not sure yet.

@nikomatsakis
Copy link
Contributor

I realized that this is in fact the fault of the method resolution. The problem is that we always 'reborrow' -- we should only do that when not auto-refing.

@nikomatsakis
Copy link
Contributor

Nominating, this bug has to get fixed. (Luckily it's easy, just don't want to forget.)

@brson
Copy link
Contributor

brson commented Dec 11, 2014

P-backcompat-lang, 1.0. fix enqueued.

bors added a commit that referenced this issue Dec 12, 2014
**First commit.** Patch up debruijn indices. Fixes #19537. 

**Second commit.** Stop reborrowing so much. Fixes #19147. Fixes #19261.

r? @nick29581
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants