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

Improve symmetry when calling a function with a borrowed parameter #10504

Closed
wycats opened this issue Nov 15, 2013 · 23 comments
Closed

Improve symmetry when calling a function with a borrowed parameter #10504

wycats opened this issue Nov 15, 2013 · 23 comments
Milestone

Comments

@wycats
Copy link
Contributor

wycats commented Nov 15, 2013

Currently, if I have a function like this:

fn hello(world: &World) {}

I can call it like this:

hello(~World{})

But not like this:

hello(World{})

// I would have to do
hello(&World{})

According to @alexcrichton, this is because the first example automatically coerces the ~World into a &~World, but the second example does not.

The lack of symmetry here is somewhat confusing, and makes it hard to understand (at least at first), how exactly borrowed pointers work with different callers.

@alexcrichton
Copy link
Member

I have also found this annoying for locally understanding a function

let a = ~1;
foo(a)

Does that function call move a or does it not? Turns out it depends on the declaration of foo.

I think that an interesting exercise for this issue would be removing autoref/autoderef for function arguments. I have a feeling that the change would be incredibly painful and probably not worth it, but this is probably something that should at least be considered.

The alternative would be

let a = ~1;
foo(&*a)

Which does look a bit silly, but it is clear to all who read it that the argument is not moved, but rather borrowed. I'm not personally a big fan of this syntax.

Another idea of how to go about this would be to very clearly document this in the tutorial/manual. This may be done already, but I'm not sure how well autoref/autoderef is covered overall.

@tomdale
Copy link

tomdale commented Nov 15, 2013

👍

@wycats
Copy link
Contributor Author

wycats commented Nov 15, 2013

@alexcrichton I would be ok with either always requiring & or never requiring it, but not the current mix, even if covered better in the docs.

The problem is that in order to explain the current behavior (which is fundamental, even at an early stage), you have to explain borrowing, then the &T syntax, and then explain that the compiler automatically coerces ~T, so you don't have to do &~T.

In contrast, if the behavior symmetrically required &, you would teach borrowing, and then say that you need to do &T to pass any pointer to the function that wants to borrow it.

If the behavior symmetrically did not require &, you would teach borrowing, and then say that you can pass any pointer to a function that wants to borrow it.

Either of those are better than the current sometimes-coerces approach, I think.

@nikomatsakis
Copy link
Contributor

I would like to remove argument coercion -- well, specifically "autoborrowing." Nominating.

@nikomatsakis
Copy link
Contributor

Reading the comments and discussing with @wycats, I hadn't considered the possibility of extending the autoborrow to encompass T -> &T (what we call "autoref" in a method call). Hmm. I think I'd rather err on the side of less magic at the moment, but that is an interesting possibility that wouldn't be too hard to do, and maybe it means less annoying little &.

Still, what I dislike most about the autoborrowing is that I can't tell from a call site when I am passing by value and when I am not. That is, I dislike seeing this:

let x = ~[1,2,3];
foo(x);

because it looks to me like x has been given away. But in fact then I look at foo and find:

fn foo(x: &[int]) { ... }

and so I see that in fact x was only borrowed. I think I find it easy to understand the flow of ownership if I don't have to look at the declarations of the fns being called. YMMV.

@nikomatsakis
Copy link
Contributor

Something else to consider if we extended autoref: what about &mut?

It seems odd to me that foo(x) might mutate x.

And would we allow foo(rvalue())? We currently do permit rvalues to be implicitly autoref'd in a mutable way because of iterators. That implies one could call inc(3) which would allocate a temporary, put 3 in it, call inc which mutates the temporary (to no visible effect). Maybe that's ok.

@alexcrichton
Copy link
Member

I've added this to the meeting agenda.

@wycats
Copy link
Contributor Author

wycats commented Nov 17, 2013

Here's another similar issue I ran into today when trying to explain things to someone:

use std::io::stdio::println;

struct Person{
  first_name: ~str,
  last_name: ~str
}

impl<'self> Person {
  pub fn new(first: ~str, last: ~str) -> ~Person {
    ~Person{ first_name: first, last_name: last }
  }

  pub fn full_name(&self) -> ~str {
    self.first_name + " " + self.last_name
  }

  pub fn get_first_name(&'self self) -> &'self str {
    self.first_name
  }
}

fn main() {
  let person = Person::new(~"Tom", ~"Dale");
  println(person.get_first_name());
  println(person.full_name());
}

In this case, the use of self.first_name inside of get_first_name was a compiler error:

hello.rs:18:4: 18:19 error: mismatched types: expected `&'self str` but found `~str` (str storage differs: expected &'self  but found ~)
hello.rs:18     self.first_name
                ^~~~~~~~~~~~~~~

The solution was to do:

pub fn get_first_name(&'self self) -> &'self str {
  self.first_name.as_slice()
}

Since I have a ~str here and I need a &str, which has strictly more restrictions, I would expect Rust to automatically demote the ~str to a &str.

As a side note, orthogonal to this point, as_slice() is a somewhat leaky abstraction, because it exposes details about the implementation of ~str and &str that aren't necessary to reason about the ownership semantics (and the ownership semantics tell you everything you need to know about the relative cost of &str vs ~str).

@huonw
Copy link
Member

huonw commented Nov 17, 2013

I imagine it's just a typo, but

this is because the first example automatically coerces the ~World into a &~World, but the second example does not.

in the original bug is incorrect, ~World is being borrowed to a &World (i.e. it's pointer -> pointer, not auto-ref).

@nikomatsakis
Copy link
Contributor

Currently we coerce for arguments and struct field initializers. We ought to coerce in return expressions too. I'm not sure if there are other missing places. Worth opening a separate bug for return expression coercion, if one doesn't already exist.

@asb
Copy link

asb commented Nov 19, 2013

That implies one could call inc(3) which would allocate a temporary, put 3 in it, call inc which mutates the temporary (to no visible effect). Maybe that's ok.

@nikomatsakis I rather liked the idea of the invariant you stated in a blog post a while back. Namely "you don’t have allocation unless you see a sigil". The example you gave, and the automatic coercion of arguments in general seems to break this which is unfortunate, and I think a good argument against the coercion.

@huonw
Copy link
Member

huonw commented Nov 19, 2013

@asb I assume @nikomatsakis is only talking about coercion to a reference (&), which is never an allocation (the quoted example would be putting 3 on the stack).

@lilac
Copy link
Contributor

lilac commented Nov 20, 2013

@alexcrichton @nikomatsakis The problem you raised, whether a function call moves a unique pointer or not is not explicit, is annoying. But I think the source of this problem is the implicit move semantic, auto-borrowing just intensifies it. If you want easy understandings of the flow of ownerships, then maybe we should make the move semantic explicit, just like C++'s std::move.

I dislike the &* way, since it just makes the language more ugly, and borrowing a heavy burden.

@lilac
Copy link
Contributor

lilac commented Nov 20, 2013

@wycats I try to explain why ~T can be automatically cast to &T but not T. Rust distinguishes between values and pointers, thus it is reasonable to cast a ~T (unique pointer) to a &T (borrowed pointer). In contrast, the cast from a T (value) to a pointer (&T) isn't that consistent in design, although we could consider this.

In C++, a &T actually is an alias, so it makes sense to accept a T when requires a &T.

@thestinger
Copy link
Contributor

Rust doesn't have copy constructors, so the alternative to not moving by default is not having the ability to copy types with destructors.

@nikomatsakis
Copy link
Contributor

On Tue, Nov 19, 2013 at 05:10:27PM -0800, lilac wrote:

If you want easy understandings of the flow of ownerships, then maybe we should make the move semantic explicit, just like C++'s std::move.

We tried it and found it quite painful in practice. This blog post
describes the rationale for removing it:
http://smallcultfollowing.com/babysteps/blog/2012/10/01/moves-based-on-type/

@lilac
Copy link
Contributor

lilac commented Nov 20, 2013

@nikomatsakis Right. I've followed that blog for a long time, thus have read the post you mentioned.

The latest post http://smallcultfollowing.com/babysteps/blog/2013/11/20/parameter-coercion-in-rust/ is really good work, covering many aspects of this issue. I can't agree with "if we remove autoborrow the notational overhead will be too large" and "it’s not actually possible to reason about a function independently of its callees" any more.

@pnkfelix
Copy link
Member

We need to decide what we're going to do for 1.0. P-backcompat-lang.

@pcwalton
Copy link
Contributor

pcwalton commented Jun 9, 2014

I propose taking RFC rust-lang/rfcs#112 and doing nothing else.

pcwalton added a commit to pcwalton/rust that referenced this issue Jun 25, 2014
This will break code like:

    fn f(x: &mut int) {}

    let mut a = box 1i;
    f(a);

Change it to:

    fn f(x: &mut int) {}

    let mut a = box 1i;
    f(&mut *a);

RFC 33; issue rust-lang#10504.

[breaking-change]
@pcwalton
Copy link
Contributor

PR #15171 removes the mutable Box to &mut borrow. But I believe, based on @huonw's statistics, that we should remove the borrow from Box to & as well, as it will break little code. So I would like to propose another RFC before we mark this as closed.

bors added a commit that referenced this issue Jun 25, 2014
This will break code like:

    fn f(x: &mut int) {}

    let mut a = box 1i;
    f(a);

Change it to:

    fn f(x: &mut int) {}

    let mut a = box 1i;
    f(&mut *a);

RFC 33; issue #10504.

[breaking-change]

r? @brson
@pcwalton
Copy link
Contributor

New RFC: rust-lang/rfcs#139

@pcwalton
Copy link
Contributor

pcwalton commented Jul 2, 2014

Nominating for closure in favor of #15349, which I believe adequately captures what we'd like to do here for 1.0.

@pnkfelix
Copy link
Member

pnkfelix commented Jul 3, 2014

Closing in favor of #15349 as suggested by @pcwalton

@pnkfelix pnkfelix closed this as completed Jul 3, 2014
flip1995 pushed a commit to flip1995/rust that referenced this issue Jun 30, 2023
flip1995 pushed a commit to flip1995/rust that referenced this issue Jun 30, 2023
…blyxyas,xFrednet

[`type_repetition_in_bounds`]: Don't lint on derived code

fixes rust-lang#10504.

changelog: [`type_repetition_in_bounds`]: Don't lint on derived code
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

No branches or pull requests

10 participants