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

std: Deprecate a number of unstable features #26914

Merged
merged 1 commit into from
Jul 28, 2015

Conversation

alexcrichton
Copy link
Member

Many of these have long since reached their stage of being obsolete, so this
commit starts the removal process for all of them. The unstable features that
were deprecated are:

  • box_heap
  • cmp_partial
  • fs_time
  • hash_default
  • int_slice
  • iter_min_max
  • iter_reset_fuse
  • iter_to_vec
  • map_in_place
  • move_from
  • owned_ascii_ext
  • page_size
  • read_and_zero
  • scan_state
  • slice_chars
  • slice_position_elem
  • subslice_offset

@rust-highfive
Copy link
Collaborator

r? @huonw

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

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jul 9, 2015
@alexcrichton
Copy link
Member Author

r? @aturon

cc @rust-lang/libs

@rust-highfive rust-highfive assigned aturon and unassigned huonw Jul 9, 2015
@@ -2448,6 +2459,8 @@ impl<I> Fuse<I> {
/// previously returned `None`.
#[inline]
#[unstable(feature = "iter_reset_fuse", reason = "seems marginal")]
#[deprecated(since = "1.3.0",
reason = "unusual for adaptors to have one-off methods")]
Copy link
Member

Choose a reason for hiding this comment

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

This is an unusual comment :-) I don't disagree with the deprecation, but I'd like to add a method to fuse, to query its fused/nonfused state, it would be quite useful.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I agree: the problem here isn't the one-off-ness per se, but rather that this method doesn't seem terribly useful in practice.

@@ -401,6 +401,7 @@ pub fn max<T: Ord>(v1: T, v2: T) -> T {
/// ```
#[inline]
#[unstable(feature = "cmp_partial")]
#[deprecated(since = "1.3.0", reason = "has not proven itself worthwhile")]
Copy link
Member

Choose a reason for hiding this comment

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

This only deprecates partial_min, but not partial_max. Are you sure that’s not an oversight?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, thanks!

@nagisa
Copy link
Member

nagisa commented Jul 10, 2015

I have no complaints about scope of this PR 😊

@bors
Copy link
Contributor

bors commented Jul 24, 2015

☔ The latest upstream changes (presumably #27215) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Jul 26, 2015

☔ The latest upstream changes (presumably #26870) made this pull request unmergeable. Please resolve the merge conflicts.

@alexcrichton alexcrichton force-pushed the deprecate-easy branch 2 times, most recently from 53e662e to a1227e8 Compare July 26, 2015 17:19
@bluss
Copy link
Member

bluss commented Jul 26, 2015

I'd like to keep position elem. We want this functionality in some form (I imagine something like .find<P>(p: P) where P: SlicePattern in the end, inspired by str's patterns). Can we rename it to .find(elem: &T) for now, and wait for an RFC and design to generalize its signature in the future?

@@ -86,10 +86,13 @@ use core::raw::{TraitObject};
#[lang = "exchange_heap"]
#[unstable(feature = "box_heap",
reason = "may be renamed; uncertain about custom allocator design")]
#[allow(deprecated)]
Copy link
Member

Choose a reason for hiding this comment

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

cc @pnkfelix -- can you comment on whether we may still want this constant with the upcoming box/allocator system?

@aturon
Copy link
Member

aturon commented Jul 27, 2015

r=me modulo nits.

@alexcrichton
Copy link
Member Author

@bluss perhaps, yeah, but I'd prefer to keep the scope of this PR to just deprecate items instead of adding and/or renaming new APIs.

@alexcrichton
Copy link
Member Author

@bors: r=aturon a1227e87c96c3e083467b8c5fb373b09c0f38e6c

@bors
Copy link
Contributor

bors commented Jul 27, 2015

⌛ Testing commit a1227e8 with merge be324ca...

@bors
Copy link
Contributor

bors commented Jul 27, 2015

💔 Test failed - auto-win-gnu-32-nopt-t

Many of these have long since reached their stage of being obsolete, so this
commit starts the removal process for all of them. The unstable features that
were deprecated are:

* cmp_partial
* fs_time
* hash_default
* int_slice
* iter_min_max
* iter_reset_fuse
* iter_to_vec
* map_in_place
* move_from
* owned_ascii_ext
* page_size
* read_and_zero
* scan_state
* slice_chars
* slice_position_elem
* subslice_offset
@alexcrichton
Copy link
Member Author

@bors: r=aturon

On Mon, Jul 27, 2015 at 4:38 PM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-win-gnu-32-nopt-t
http://buildbot.rust-lang.org/builders/auto-win-gnu-32-nopt-t/builds/842


Reply to this email directly or view it on GitHub
#26914 (comment).

@bors
Copy link
Contributor

bors commented Jul 27, 2015

📌 Commit b3aa1a6 has been approved by aturon

@@ -1627,6 +1630,7 @@ impl<T> IntoIter<T> {
#[inline]
/// Drops all items that have not yet been moved and returns the empty vector.
#[unstable(feature = "iter_to_vec")]
#[deprecated(since = "1.3.0", reason = "replaced by drain()")]
Copy link
Member

Choose a reason for hiding this comment

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

I don't see how this is replaced by drain, it may in fact be very useful (I'd use it in itertools if it were stable).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes this isn't replaced by Drain -- in particular IntoIter allows you to return the Vec up and transport it around -- Drain doesn't. Still, this seems fairly marginal for today's std.

Copy link
Member Author

Choose a reason for hiding this comment

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

True, but I believe that this was originally added to get back the original allocation, in which case drain suffices for this.

Copy link
Member

Choose a reason for hiding this comment

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

Not for every use case, for example not for GroupByLazy, which would like to recycle allocations of the IntoIters it uses.

@bluss
Copy link
Member

bluss commented Jul 27, 2015

@alexcrichton You don't need to rename it, I just suggested not deprecating position_elem yet.

@alexcrichton
Copy link
Member Author

Possibly, yeah, but it's slated to be deprecated soon anyway so it doesn't seem too bad to just go ahead and do so.

@bors
Copy link
Contributor

bors commented Jul 28, 2015

⌛ Testing commit b3aa1a6 with merge 9ca511c...

bors added a commit that referenced this pull request Jul 28, 2015
Many of these have long since reached their stage of being obsolete, so this
commit starts the removal process for all of them. The unstable features that
were deprecated are:

* box_heap
* cmp_partial
* fs_time
* hash_default
* int_slice
* iter_min_max
* iter_reset_fuse
* iter_to_vec
* map_in_place
* move_from
* owned_ascii_ext
* page_size
* read_and_zero
* scan_state
* slice_chars
* slice_position_elem
* subslice_offset
@bors bors merged commit b3aa1a6 into rust-lang:master Jul 28, 2015
@alexcrichton alexcrichton deleted the deprecate-easy branch August 4, 2015 15:52
@@ -590,6 +590,8 @@ impl ExactSizeIterator for ArgsOs {

/// Returns the page size of the current architecture in bytes.
#[unstable(feature = "page_size", reason = "naming and/or location may change")]
#[deprecated(since = "1.3.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is used by coroutine-rs, would be nice to provide this somewhere since it's os specific and non-trivial to obtain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-needs-decision Issue: In need of a decision. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.