Skip to content

Conversation

@aturon
Copy link
Contributor

@aturon aturon commented Mar 3, 2017

No description provided.

@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@erickt
Copy link
Contributor

erickt commented Mar 3, 2017

Rendered

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left a few nits, love it

vital. However:

- The vast majority of methods borrow `self`, rather than taking it by value.
- Usually the name of a method makes obvious whether the `self` borrow will be
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: "makes it obvious"


- The vast majority of methods borrow `self`, rather than taking it by value.
- Usually the name of a method makes obvious whether the `self` borrow will be
mutable or not (e.g. `push` versus `len`).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: push(), len()

- Usually the name of a method makes obvious whether the `self` borrow will be
mutable or not (e.g. `push` versus `len`).
- The borrow checker will prevent any incorrect borrowing, though if borrowing
were too implicit, it could make debugging borrow check errors harder.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: "borrow-check errors"

```

But we could easily allow you to write `read_config(path)`, implicitly borrowing
`path` for `read_config` and then *dropping it* immediately afterwards. That
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still feel like it might be helpful to say, effectively desugaring into something like:

let config = {
    let tmp = path;
    read_config(&tmp)
};

but perhaps the syntax is too fancy

@aturon aturon merged commit c29ba63 into rust-lang:gh-pages Mar 3, 2017
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 this pull request may close these issues.

5 participants