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

RFC: conventions for ownership variants #199

Merged
merged 2 commits into from Aug 28, 2014

Conversation

Projects
None yet
10 participants
@aturon
Member

aturon commented Aug 14, 2014

This is a conventions RFC for settling naming conventions when there
are by value, by reference, and by mutable reference variants of an
operation.

Rendered

@kballard

This comment has been minimized.

Show comment
Hide comment
@kballard

kballard Aug 14, 2014

Contributor

I'm generally in favor of this sort of renaming, but I do not like iter_owned(). I disagree that we want to be pushing the idea of "ownership" here. While technically true, I typically do not consider 3i to be an "owned value", it's just a "value", so calling .iter_owned() on a Vec<int> and getting Iterator<int> is quite strange.

I liked move_iter() because the "move" word was the signal that it consumed the receiver and moved values out of it. The closest alternative to that we have is into_iter(), and I'm inclined to say we should just go with that.

If into_iter() simply isn't tenable for some reason, I like iter_val() (or iter_value()) better than iter_owned(). But I think into_iter() is better for two reasons:

  1. It makes it clear that the receiver is consumed.
  2. If the receiver is a container of references (e.g. Vec<&int>), iter_value() might be slightly confusing.
Contributor

kballard commented Aug 14, 2014

I'm generally in favor of this sort of renaming, but I do not like iter_owned(). I disagree that we want to be pushing the idea of "ownership" here. While technically true, I typically do not consider 3i to be an "owned value", it's just a "value", so calling .iter_owned() on a Vec<int> and getting Iterator<int> is quite strange.

I liked move_iter() because the "move" word was the signal that it consumed the receiver and moved values out of it. The closest alternative to that we have is into_iter(), and I'm inclined to say we should just go with that.

If into_iter() simply isn't tenable for some reason, I like iter_val() (or iter_value()) better than iter_owned(). But I think into_iter() is better for two reasons:

  1. It makes it clear that the receiver is consumed.
  2. If the receiver is a container of references (e.g. Vec<&int>), iter_value() might be slightly confusing.
@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Aug 14, 2014

Member

I also agree that I dislike iter_owned. I'm not sure what is best as a replacement. I agree that the name should indicate consumption.

Member

steveklabnik commented Aug 14, 2014

I also agree that I dislike iter_owned. I'm not sure what is best as a replacement. I agree that the name should indicate consumption.

Proposal: generally standardize on `mut` as a shortening of `mut_ref`.

This comment has been minimized.

@steveklabnik

steveklabnik Aug 14, 2014

Member

✂️

@steveklabnik

This comment has been minimized.

@aturon

aturon Aug 14, 2014

Member

Maybe ruining a joke, but I don't know what this means :)

@aturon

aturon Aug 14, 2014

Member

Maybe ruining a joke, but I don't know what this means :)

This comment has been minimized.

@steveklabnik

steveklabnik Aug 14, 2014

Member

Sorry, there's an extra blank line here, so you should snip it 😄

@steveklabnik

steveklabnik Aug 14, 2014

Member

Sorry, there's an extra blank line here, so you should snip it 😄

@metajack

This comment has been minimized.

Show comment
Hide comment
@metajack

metajack Aug 14, 2014

@steveklabnik I think iter_val would work for that.

metajack commented Aug 14, 2014

@steveklabnik I think iter_val would work for that.

@aturon

This comment has been minimized.

Show comment
Hide comment
@aturon

aturon Aug 14, 2014

Member

@steveklabnik Can you elaborate on why you dislike owned? Same reasons as @kballard (i.e. Copy data)?

By the way, in case it's not clear, _move is a viable alternative suffix.

Member

aturon commented Aug 14, 2014

@steveklabnik Can you elaborate on why you dislike owned? Same reasons as @kballard (i.e. Copy data)?

By the way, in case it's not clear, _move is a viable alternative suffix.

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Aug 14, 2014

Member

I'm not fully sure, but

I typically do not consider 3i to be an "owned value", it's just a "value", so calling .iter_owned() on a Vec and getting Iterator<int> is quite strange.

Seems right, or at least part of it. I'll do some reflecting here.

(I think it may be because we've been dropping the term 'owned pointer' in favor of 'box', and so even though this isn't related, that's just hanging on.)

I think _move might be better, as it describes what's actually happening: it's moving the value.

Member

steveklabnik commented Aug 14, 2014

I'm not fully sure, but

I typically do not consider 3i to be an "owned value", it's just a "value", so calling .iter_owned() on a Vec and getting Iterator<int> is quite strange.

Seems right, or at least part of it. I'll do some reflecting here.

(I think it may be because we've been dropping the term 'owned pointer' in favor of 'box', and so even though this isn't related, that's just hanging on.)

I think _move might be better, as it describes what's actually happening: it's moving the value.

@kballard

This comment has been minimized.

Show comment
Hide comment
@kballard

kballard Aug 14, 2014

Contributor

If iter_move is viable, maybe we should just use that. Although I am still partial to into_iter(), because it has a nice symmetry in the types: any collection Foo<T> is transformed into some Iterator<T>, i.e. the element type stays the same and only the container type changes. On the other hand, into_ is not a suffix, and there's some benefit to having all 3 iteration methods all start with iter.

So I guess I'm currently voting for iter_move(), with into_iter() as a close second.

Contributor

kballard commented Aug 14, 2014

If iter_move is viable, maybe we should just use that. Although I am still partial to into_iter(), because it has a nice symmetry in the types: any collection Foo<T> is transformed into some Iterator<T>, i.e. the element type stays the same and only the container type changes. On the other hand, into_ is not a suffix, and there's some benefit to having all 3 iteration methods all start with iter.

So I guess I'm currently voting for iter_move(), with into_iter() as a close second.

@glaebhoerl

This comment has been minimized.

Show comment
Hide comment
@glaebhoerl

glaebhoerl Aug 15, 2014

Contributor

I don't have strong feelings about this at all, but for what it's worth I also prefer iter_{val,value,move} to iter_owned.

Contributor

glaebhoerl commented Aug 15, 2014

I don't have strong feelings about this at all, but for what it's worth I also prefer iter_{val,value,move} to iter_owned.

@Valloric

This comment has been minimized.

Show comment
Hide comment
@Valloric

Valloric Aug 15, 2014

Agreed that iter_owned is worse than iter_move or iter_val. I'd prefer the iter_move option. The reason is that iter_move tells you what is happening: iterate over the range by moving out values. Iteration by "owning" makes far less sense to me. At first glance, I'm not sure how to parse it.

Also don't forget that Rust's biggest potential market is C++ developers and to them "moving" is a familiar concept (or at least should be, if they stayed up to date with C++11). C++ has move constructors, move assignment etc. Not "owned" constructors.

Furthermore, "move" as a verb tells you about the action, while "owned" (since it's an adjective) appears to be talking about the iterator itself and reads to me like "iterate over this owned iterator."

Valloric commented Aug 15, 2014

Agreed that iter_owned is worse than iter_move or iter_val. I'd prefer the iter_move option. The reason is that iter_move tells you what is happening: iterate over the range by moving out values. Iteration by "owning" makes far less sense to me. At first glance, I'm not sure how to parse it.

Also don't forget that Rust's biggest potential market is C++ developers and to them "moving" is a familiar concept (or at least should be, if they stayed up to date with C++11). C++ has move constructors, move assignment etc. Not "owned" constructors.

Furthermore, "move" as a verb tells you about the action, while "owned" (since it's an adjective) appears to be talking about the iterator itself and reads to me like "iterate over this owned iterator."

@nrc

This comment has been minimized.

Show comment
Hide comment
@nrc

nrc Aug 15, 2014

Member

+1 to _move, -1 to _owned. I think iterators are about doing stuff (they are a function encapsulated as an object, in a way) so I prefer a verb to an adjective. In particular, the iterator doesn't own anything, so I find iter_owned misleading, it should be 'iter_transfers_ownershipbut that is kind of a mouthful, which is why I preferiter_move. To me, movement is the dynamic counterpart to ownership, so we are still sticking to the ownership narrative with move (as opposed to sayiter_mutoriter_uniq`).

Member

nrc commented Aug 15, 2014

+1 to _move, -1 to _owned. I think iterators are about doing stuff (they are a function encapsulated as an object, in a way) so I prefer a verb to an adjective. In particular, the iterator doesn't own anything, so I find iter_owned misleading, it should be 'iter_transfers_ownershipbut that is kind of a mouthful, which is why I preferiter_move. To me, movement is the dynamic counterpart to ownership, so we are still sticking to the ownership narrative with move (as opposed to sayiter_mutoriter_uniq`).

@ben0x539

This comment has been minimized.

Show comment
Hide comment
@ben0x539

ben0x539 Aug 17, 2014

If anything it should be iter_owning, just like people complained about owned pointers really being owning pointers. ;)

I'm also in favor of iter_move, though. How does a _ref method yield its result? By referencing something/via a reference. How does a _move method do it? By moving something/via a move.

ben0x539 commented Aug 17, 2014

If anything it should be iter_owning, just like people complained about owned pointers really being owning pointers. ;)

I'm also in favor of iter_move, though. How does a _ref method yield its result? By referencing something/via a reference. How does a _move method do it? By moving something/via a move.

@Kimundi

This comment has been minimized.

Show comment
Hide comment
@Kimundi

Kimundi Aug 25, 2014

Member

Using the word move interchangeable with by-value is a idiosyncrasy that developed over time and stems from the misguided assumption that owned == Box<T>, thus by-value == move.

While for specific types like Box or Vec it might be true, the problem is that in the generic case it is wrong to describe something taking a value as "doing a move", which is what the _move naming convention does.

For example, one could image this in a future Rust:

impl<T, static N: uint> [T, ..N] {
    fn iter_move(self) -> FixedVecIter<T, N> { ... }
}

for which a iter_move() would not move, but copy.

The into prefix has kinda the same issue, but there it could be easier to paper over as a naming convention because its not literally using the word move.
(Sadly, it was my own misconception about equating by-value with move that lead to the coining of the into_ prefix in the first place.)

Better would be value, val or owned (not owning, as this is about the way self is passed, not the resulting type)

Member

Kimundi commented Aug 25, 2014

Using the word move interchangeable with by-value is a idiosyncrasy that developed over time and stems from the misguided assumption that owned == Box<T>, thus by-value == move.

While for specific types like Box or Vec it might be true, the problem is that in the generic case it is wrong to describe something taking a value as "doing a move", which is what the _move naming convention does.

For example, one could image this in a future Rust:

impl<T, static N: uint> [T, ..N] {
    fn iter_move(self) -> FixedVecIter<T, N> { ... }
}

for which a iter_move() would not move, but copy.

The into prefix has kinda the same issue, but there it could be easier to paper over as a naming convention because its not literally using the word move.
(Sadly, it was my own misconception about equating by-value with move that lead to the coining of the into_ prefix in the first place.)

Better would be value, val or owned (not owning, as this is about the way self is passed, not the resulting type)

@ben0x539

This comment has been minimized.

Show comment
Hide comment
@ben0x539

ben0x539 Aug 25, 2014

Types not being Copy is the general case. You can tell because the super-general fn f<T>(t: T) -> T { drop(t); t } doesn't compile, because the value has in fact been moved.

I don't agree that the "move" terminology stems from the behavior of boxes, since it was already prevalent in C++.

I also don't understand your point about "not owning", what does owned describe in this example?

ben0x539 commented Aug 25, 2014

Types not being Copy is the general case. You can tell because the super-general fn f<T>(t: T) -> T { drop(t); t } doesn't compile, because the value has in fact been moved.

I don't agree that the "move" terminology stems from the behavior of boxes, since it was already prevalent in C++.

I also don't understand your point about "not owning", what does owned describe in this example?

@Kimundi

This comment has been minimized.

Show comment
Hide comment
@Kimundi

Kimundi Aug 25, 2014

Member

@ben0x539: Moves are the general case if you don't know anything about the type, like for unbounded generics, yes.

But what I'm saying here is that the naming of these methods should be correct in regard to each specific use of the method, that is where the types are known:

let v = [1, 2, 3];
let iter = v.iter_move(); // This is a lie - a copy happens, not a move
let v2 = v;

iter_move() in this example might be written generically with only using moves internally, but that has nothing todo with the user calling it on a concrete copyable type.

Names are documentation to help the understanding of the code, and thus is should not use terminology that is in contradiction to the language semantic.


Not sure what point I was trying to make with owning tbh, just ignore it.

Member

Kimundi commented Aug 25, 2014

@ben0x539: Moves are the general case if you don't know anything about the type, like for unbounded generics, yes.

But what I'm saying here is that the naming of these methods should be correct in regard to each specific use of the method, that is where the types are known:

let v = [1, 2, 3];
let iter = v.iter_move(); // This is a lie - a copy happens, not a move
let v2 = v;

iter_move() in this example might be written generically with only using moves internally, but that has nothing todo with the user calling it on a concrete copyable type.

Names are documentation to help the understanding of the code, and thus is should not use terminology that is in contradiction to the language semantic.


Not sure what point I was trying to make with owning tbh, just ignore it.

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Aug 28, 2014

Member

This was discussed in this week's meeting, and the decision was to merge with the alteration of iter_owned => iter_move (which has now been made).

As a result, I am going to merge this.

Member

alexcrichton commented Aug 28, 2014

This was discussed in this week's meeting, and the decision was to merge with the alteration of iter_owned => iter_move (which has now been made).

As a result, I am going to merge this.

@alexcrichton alexcrichton merged commit 1851c2f into rust-lang:master Aug 28, 2014

@aturon aturon referenced this pull request Sep 5, 2014

Merged

collections: Stabilize Vec #17029

@chriskrycho chriskrycho referenced this pull request Dec 30, 2016

Closed

Document all features in the reference #38643

0 of 17 tasks complete

withoutboats pushed a commit to withoutboats/rfcs that referenced this pull request Jan 15, 2017

Merge pull request #199 from tmiasko/stream-take
Pass through all errors in both Stream::take and Stream::skip until stream is done.

@chriskrycho chriskrycho referenced this pull request Mar 11, 2017

Closed

Document all features #9

18 of 48 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment