Rename `push/pop_back` to `push`/`pop`, add to `MutableSeq` #15611

brson · 2014-07-11T18:41:35Z

This fixes naming conventions for push/pop from either end of a structure by partially implementing @erickt's suggestion from #10852 (comment), namely:

push/pop from the 'back' are called push and pop.
push/pop from the 'front' are called push_front and pop_front.
push/pop are declared on the MutableSeq trait.
Implement MutableSeq for Vec, DList, and RingBuf.
Add MutableSeq to the prelude.

I did not make any further refactorings because there is some more extensive thought that needs to be put into the collections traits. This is an easy first step that should close #10852.

I left the push_back and pop_back methods on DList and RingBuf deprecated. Because MutableSeq is in the prelude it shouldn't break many, but it is a breaking change.

brson · 2014-07-11T18:42:20Z

cc @aturon @alexcrichton @kballard

brson · 2014-07-11T18:50:39Z

Hm, probably need to move around some docs here.

lilyball · 2014-07-11T18:51:21Z

src/libstd/prelude.rs

@@ -63,7 +63,7 @@
 #[doc(no_inline)] pub use clone::Clone;
 #[doc(no_inline)] pub use cmp::{PartialEq, PartialOrd, Eq, Ord};
 #[doc(no_inline)] pub use cmp::{Ordering, Less, Equal, Greater, Equiv};
-#[doc(no_inline)] pub use collections::{Collection, Mutable, Map, MutableMap};
+#[doc(no_inline)] pub use collections::{Collection, Mutable, Map, MutableMap, MutableSeq};


Could you update the Vim syntax file to include MutableSeq?

alexcrichton · 2014-07-11T19:23:16Z

Overall I'm not certain if we want these methods to be in a trait or not (yet another prelude trait), but I'm ok with as-is for now. Thinking about a question such as this would require more thought to the design of the collection traits in general.

This is definitely a step forward, and I'm a fan of it. 👍

lilyball · 2014-07-11T19:24:43Z

Oh yes I forgot to say, I'm generally in favor of this as well 👍 though I agree with @alexcrichton that at some point we do need to give some thought to the design of the collection traits and make sure that what we have is appropriate.

aturon · 2014-07-11T20:23:32Z

I'm late to the party, but yes, this moves us in the right direction. We should consider adding insert and remove to MutableSeq (from @erickt's proposal) as part of a separate pass over the collections trait. 👍

One issue, though: I'm not sure this closes #10852, since that issue was at least partly about renaming shift/unshift. Since there seemed to be agreement that we want to do that, we should make sure to keep it on the list somewhere.

brson · 2014-07-12T01:10:29Z

@aturon Oh good point about shift/unshift. I guess those need to be push_front / pop_front and Vec needs to implement Deque?

brson · 2014-07-12T01:11:20Z

I added a commit that makes push_back / pop_back default methods, which addresses most of @kballard's concerns. I'll review why the imports in the macros were needed and see if I can get rid of them.

brson · 2014-07-12T01:23:28Z

Guh, if Vec implements Deque then we have to deal with the inconsistency between Deque::front and ImmutableVector::head, etc.

brson · 2014-07-12T01:23:41Z

I'm not going to deal with that. There will be temporary redundancy.

brson · 2014-07-12T01:25:25Z

#15626 covers the front/back vs head/last issue.

brson · 2014-07-12T01:31:02Z

To deprecate shift/unshift on Vec we should probably put Deque in the prelude.

brson · 2014-07-12T01:51:37Z

Added another commit that implements Deque on Vec, deprecates shift/unshift, and adds Deque to the prelude.

lilyball · 2014-07-12T06:50:54Z

@brson As dcb mentioned on IRC a few minutes ago, implementing Deque for Vec actually seems pretty weird, because Deque will be expected (by the average programmer) to have an efficient push_front()/pop_front() implementation and Vec does not qualify.

danburkert · 2014-07-12T06:54:48Z

I suggest not implementing Deque for Vec. The Deque docs do not say that the implementation must be efficient, but that will be a common assumption. In #10852 @nikomatsakis suggested removing shift and unshift in favor of using insert and remove.

danburkert · 2014-07-12T06:55:07Z

@kballard beat me to it :)

brson · 2014-07-14T19:29:23Z

Ok. I will just deprecate shift and unshift and not implement Deque.

brson · 2014-07-14T21:49:57Z

All comments addressed. Vec does not implement Deque. shift/unshift are deprecated. And the imports are removed from the vec! macro.

alexcrichton · 2014-07-14T21:56:14Z

src/librustc_llvm/llvmdeps.rs

+#[link(name = "dl")]
+#[link(name = "m")]
+#[link(name = "stdc++")]
+extern {}


This seems to have snuck in by accident.

alexcrichton · 2014-07-14T21:59:44Z

I feel like MutableSeq is better named Queue, but I also feel that the collections traits all need better names, so I don't want that to block the renaming of the methods as well as the deprecation of shift/unshift.

r=me with the stray file removed.

alexcrichton · 2014-07-14T22:00:15Z

As in, I don't want to bikeshed the name of a trait at this time that is rarely spoken compared to the methods.

A later PR can handle the collections traits as a whole.

Implement for Vec, DList, RingBuf. Add MutableSeq to the prelude. Since the collections traits are in the prelude most consumers of these methods will continue to work without change. [breaking-change]

Use insert/remove instead.

@erickt

This fixes naming conventions for `push`/`pop` from either end of a structure by partially implementing @erickt's suggestion from #10852 (comment), namely: * push/pop from the 'back' are called `push` and `pop`. * push/pop from the 'front' are called `push_front` and `pop_front`. * `push`/`pop` are declared on the `MutableSeq` trait. * Implement `MutableSeq` for `Vec`, `DList`, and `RingBuf`. * Add `MutableSeq` to the prelude. I did not make any further refactorings because there is some more extensive thought that needs to be put into the collections traits. This is an easy first step that should close #10852. I left the `push_back` and `pop_back` methods on `DList` and `RingBuf` deprecated. Because `MutableSeq` is in the prelude it shouldn't break many, but it is a breaking change.

Prefer stable paths over unstable ones in import path calculation Fixes rust-lang/rust-analyzer#15610

lilyball reviewed Jul 11, 2014
View reviewed changes

alexcrichton reviewed Jul 14, 2014
View reviewed changes

brson added 11 commits July 23, 2014 13:20

collections: Move push/pop to MutableSeq

d36a8f3

Implement for Vec, DList, RingBuf. Add MutableSeq to the prelude. Since the collections traits are in the prelude most consumers of these methods will continue to work without change. [breaking-change]

collections: Deprecate push_back/pop_back

79a9805

Convert some push_back users to push

5599b69

collections: Move push/pop docs to MutableSeq

7c61bb7

vim: Add MutableSeq

2d79bfa

collections: Make push_back/pop_back default methods

94e42c2

collections: Deprecate shift/unshift

9db1d35

Use insert/remove instead.

Remove kludgy imports from vec! macro

054b1ff

collections: Tweak docs for push

63d1137

Remove stray llvmdeps.rs

27e70c5

Just land already

71a75cc

bors closed this Jul 24, 2014

bors merged commit 71a75cc into rust-lang:master Jul 24, 2014

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 18, 2023

Auto merge of rust-lang#15611 - Veykril:stability-import, r=Veykril

12e28c3

Prefer stable paths over unstable ones in import path calculation Fixes rust-lang/rust-analyzer#15610

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename `push/pop_back` to `push`/`pop`, add to `MutableSeq` #15611

Rename `push/pop_back` to `push`/`pop`, add to `MutableSeq` #15611

brson commented Jul 11, 2014

brson commented Jul 11, 2014

brson commented Jul 11, 2014

lilyball Jul 11, 2014

alexcrichton commented Jul 11, 2014

lilyball commented Jul 11, 2014

aturon commented Jul 11, 2014

brson commented Jul 12, 2014

brson commented Jul 12, 2014

brson commented Jul 12, 2014

brson commented Jul 12, 2014

brson commented Jul 12, 2014

brson commented Jul 12, 2014

brson commented Jul 12, 2014

lilyball commented Jul 12, 2014

danburkert commented Jul 12, 2014

danburkert commented Jul 12, 2014

brson commented Jul 14, 2014

brson commented Jul 14, 2014

alexcrichton Jul 14, 2014

alexcrichton commented Jul 14, 2014

alexcrichton commented Jul 14, 2014

Rename push/pop_back to push/pop, add to MutableSeq #15611

Rename push/pop_back to push/pop, add to MutableSeq #15611

Conversation

brson commented Jul 11, 2014

brson commented Jul 11, 2014

brson commented Jul 11, 2014

lilyball Jul 11, 2014

Choose a reason for hiding this comment

alexcrichton commented Jul 11, 2014

lilyball commented Jul 11, 2014

aturon commented Jul 11, 2014

brson commented Jul 12, 2014

brson commented Jul 12, 2014

brson commented Jul 12, 2014

brson commented Jul 12, 2014

brson commented Jul 12, 2014

brson commented Jul 12, 2014

brson commented Jul 12, 2014

lilyball commented Jul 12, 2014

danburkert commented Jul 12, 2014

danburkert commented Jul 12, 2014

brson commented Jul 14, 2014

brson commented Jul 14, 2014

alexcrichton Jul 14, 2014

Choose a reason for hiding this comment

alexcrichton commented Jul 14, 2014

alexcrichton commented Jul 14, 2014

Rename `push/pop_back` to `push`/`pop`, add to `MutableSeq` #15611

Rename `push/pop_back` to `push`/`pop`, add to `MutableSeq` #15611