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

We need exhaustive, coherent documentation on implicit copyability #11540

Closed
bstrie opened this issue Jan 14, 2014 · 15 comments
Closed

We need exhaustive, coherent documentation on implicit copyability #11540

bstrie opened this issue Jan 14, 2014 · 15 comments

Comments

@bstrie
Copy link
Contributor

bstrie commented Jan 14, 2014

Quiz time! What do the following two programs do?

fn main() {
    let mut x = 1;
    let y = (0, &mut x);
    fn foo((a, b): (int, &mut int)) {*b += a;}
    foo(y);
    foo(y);
}
fn main() {
    let mut x = 1;
    let y = &mut x;
    fn foo(b: &mut int) {*b += 0;}
    foo(y);
    foo(y);
}

ANSWERS:

  1. The first program fails to compile at the second invocation of foo with error: use of moved value: y.
  2. The second program compiles and runs fine.

This is because, apparently, a &mut embedded in a tuple will make that tuple ineligible for implicit copyability, but a lone &mut by itself can be implicitly copied till the cows come home.

I was very surprised at this behavior. And as far as I know, nowhere do we document these sorts of subtleties. This is bad. We can't have a coherent story on explaining unique pointers if we don't clearly lay out when a copy happens and when a move happens. Someone needs to write a reference.

@olsonjeffery
Copy link
Contributor

I've noticed a lot of strange behavior around putting references with and w/o lifetimes into tuples that wouldn't be encountered if you were passing the reference on its own. This comes up a lot when you have methods on a struct that want to return a lifetime-constrainted reference to interior data plus some additional data (eg finding a &'a Tile in a Grid, plus it's coordinates)

better documentation would be great. of course, id just like more flexibility in this area.

@pnkfelix
Copy link
Member

I don't think it is accurate to see the second example as implicit copies.

(If those were really copies, then you'd be able to pass y twice into a hypothetical program like:

fn bar(c: &mut int, d: &mut int) { ... }
bar(y, y);

but that would be wrong since it would introduce aliasing.)

I think the right mental model for the second example is that y (or rather *y) is implicitly reborrowed. That is, the two foo calls are sugar for the following:

foo(&mut *y);
foo(&mut *y);

(I think some of the proposals we've discussed in recent memory would force the programmer to write the above unsugared form explicitly; but the team has not come to any conclusions about whether that would do more harm than good.)

I don't know if this distinction in terminology will help clear this up, but to my mind it makes it clearer that there are distinct borrows (with distinct non-overlapping scopes) occurring here, not arbitrary copies.

In fact, if you attempt to write out the bar example I just gave, you get an error about borrowing *y as mutable, not about copying:

% cat /tmp/g.rs
fn main() {
    let mut x = 1;
    let y = &mut x;
    fn bar(b: &mut int, c: &mut int) {*b += 0; *c += 0; }
    bar(y, y);
}
% rustc /tmp/g.rs
/tmp/g.rs:5:11: 5:12 error: cannot borrow `*y` as mutable more than once at a time
/tmp/g.rs:5     bar(y, y);
                       ^
/tmp/g.rs:5:8: 5:9 note: previous borrow of `*y` as mutable occurs here
/tmp/g.rs:5     bar(y, y);
                    ^
error: aborting due to previous error
task 'rustc' failed at 'explicit failure', /Users/fklock/Dev/Mozilla/rust.git/src/libsyntax/diagnostic.rs:102
task '<main>' failed at 'explicit failure', /Users/fklock/Dev/Mozilla/rust.git/src/librustc/lib.rs:443

Once you see your second example as performing implicit reborrows, then one sees that one must write your first example to use explicit reborrows instead of attempting to make copies, e.g. via something like this:

fn main() {
    struct T<'a> {i: int, j: &'a mut int}
    let mut x = 1;
    let y = T{i: 0, j: &mut x};
    fn foo(T{i: a, j: b}: T) {*b += a;}
    foo(T{ j: &mut *y.j, ..y});
    foo(T{ j: &mut *y.j, ..y});
}

or, again making use of implicit reborrows:

fn main() {
    struct T<'a> {i: int, j: &'a mut int}
    let mut x = 1;
    let y = T{i: 0, j: &mut x};
    fn foo(T{i: a, j: b}: T) {*b += a;}
    foo(T{ j: y.j, ..y});
    foo(T{ j: y.j, ..y});
}

@pnkfelix
Copy link
Member

I freely admit that its very confusing that

    foo(T{ j: y.j, ..y});
    foo(T{ j: y.j, ..y});

compiles while

    foo(y);
    foo(y);

fails to compile (with the same "y moved here because it has type which is non-copyable" error message).

@olsonjeffery
Copy link
Contributor

@pnkfelix I guess we're all focusing on different things in this discussion, but I feel like your response (having to use structs where bstrie used tuples) illustrates the inconsistent behavior.

Either way, none of this neglects the original point of this issue: Better documentation is needed.

@pnkfelix
Copy link
Member

I was mainly trying to discourage someone from attempting to "fix" the documentation by describing this behavior in terms of implicit copies, since I don't think that would be a correct way to describe this anomaly.

@bstrie
Copy link
Contributor Author

bstrie commented Jan 14, 2014

Indeed, if I'm using incorrect terminology then feel free to correct me. But this area of the language is sufficiently mystifying that it's hard for me to tell the difference between what is a bug and what is intended. We also need to be able to explain this to new users, especially since this is a potential performance hazard since huuuuuuge things can be implicitly copyable and people coming from C++ will be interested to learn our behavior in this area.

@bstrie
Copy link
Contributor Author

bstrie commented Jan 14, 2014

In the "explain this to new users" vein, I should note that the original impetus for this issue was because of a C programmer who had just watched Niko's recent presentation and was baffled as to why the following program doesn't fail to compile:

struct numgroup {
        x : int,
        y : int
}

fn main() {
        let i = numgroup { x:1, y:2 };
        add(i, i);
        add(i, i);
        println!("{}", add(i, i));
}

fn add(x: numgroup, y: numgroup) -> int {
        ((x.x) + (x.y)) + ((y.x) + (y.y))
}

@thestinger
Copy link
Contributor

All types in Rust can be implicitly copied via a shallow copy. If the type
contains a destructor, &mut pointer or a closure that's not 'static, it
will move ownership to the destination of the copy.

As far as I know, that's the full set of rules. I had planned on writing it
in the tutorial, but the necessary concepts other than destructors aren't
introduced yet when move semantics are covered. I'm not sure how important
it is to cover all 3 there, rather than just having the full rules in the
manual and linking to it.

@pnkfelix
Copy link
Member

Update: my terminology ("reborrow") may be slightly off; I think @nikomatsakis has been using the term "auto-borrow" for introduction of &* and &mut * as necessary.

See also: #10504 and Niko's blog for more discussion/historical context.

@nikomatsakis
Copy link
Contributor

I use the term "reborrow" to refer to an expression like &*b where b has type &T (in other words, borrowing the referent of a borrowed pointer).

@pnkfelix
Copy link
Member

Okay, so I may not have been totally off in calling this an "implicit reborrow".

I.e. perhaps an "auto-borrow of a borrowed-pointer" is the same thing as an "implicit reborrow"

@bstrie
Copy link
Contributor Author

bstrie commented Jan 15, 2014

If the type contains a destructor, &mut pointer or a closure that's not 'static

@thestinger the bit about static closures doesn't appear to be correct, the following program fails to compile:

fn foo(f: 'static ||->int, x: int) -> int {
    if x == 0 {
        f() + f()
    }
    else {
        foo(f, x-1) +  // note: `f` moved here because it has type `'static || -> int`, which is a non-copyable stack closure (capture it in a new closure, e.g. `|x| f(x)`, to override)
        foo(f, x-1)  // error: use of moved value: `f`
    }
}

fn main() {
    foo(|| 5, 2);
}

@aturon
Copy link
Member

aturon commented Sep 23, 2014

cc @steveklabnik

@steveklabnik
Copy link
Member

Now that Copy is opt-in, I'd say this isn't needed. Thoughts?

@aturon
Copy link
Member

aturon commented Jan 12, 2015

@steveklabnik Not as written, no, but I do think we'll need thorough documentation on what Copy means, when you can apply it, and when you should apply it.

steveklabnik added a commit to steveklabnik/rust that referenced this issue Jan 16, 2015
flip1995 pushed a commit to flip1995/rust that referenced this issue May 17, 2024
add new lint that disallow renaming parameters in trait functions

fixes: rust-lang#11443
fixes: rust-lang#486

changelog: add new lint [`renamed_function_params`]

Note that the lint name is not final, because I have a bad reputation in naming things, and I don't trust myself.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants