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

Deprecate the unstable Vec::resize_default #57656

Merged
merged 1 commit into from
Feb 23, 2019

Conversation

scottmcm
Copy link
Member

@scottmcm scottmcm commented Jan 16, 2019

As a way to either get additional feedback to stabilize or help move nightly users off it.

Tracking issue: #41758 (comment)

r? @SimonSapin

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 16, 2019
@scottmcm scottmcm added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jan 16, 2019
@leonardo-m
Copy link

What's the point of this change?

@SimonSapin
Copy link
Contributor

@leonardo-m The point is to emit a deprecation warning where this method is used, to leave some time for users to move to .resize_with(Default::default) instead, before we remove resize_default.

I’m ok with going in this direction, let’s see about the rest of the team:

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Jan 16, 2019

Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jan 16, 2019
@clarfonthey
Copy link
Contributor

I think this is a great idea to encourage people who prefer resize_default to voice their opinions rather than removing it immediately. 👍

@leonardo-m
Copy link

I think I prefer resize_default because it's shorter and more handy. But if you remove it, I'll live without it.

@rfcbot
Copy link

rfcbot commented Jan 27, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Jan 27, 2019
@avl
Copy link

avl commented Jan 31, 2019

I discovered Vec::resize_default yesterday. It was precisely what I wanted. First I found resize_with and it would have worked, but resize_default was exactly what I needed.

Arguments to keep it:
1: It makes for slightly shorter code in cases where it's needed (fewer symbols)
2: It is quite obvious how it works
3: It presumably doesn't cost very much in documentation or implementation to keep it?

That said, it is of course not an essential function.

@scottjmaddox
Copy link

Considering resize_with(Default::default) is not a particularly obvious pattern, especially for new Rust users, and considering the use case is fairly common (e.g. creating a zeroed byte buffer), I support stabilizing, rather than deprecating.

@SimonSapin
Copy link
Contributor

I think that vec.resize(n, 0) would be easier to find than vec.resize_default(n) to create a zeroed byte buffer.

@hellow554
Copy link
Contributor

hellow554 commented Feb 6, 2019

I do like the deprecation. At first I thought it's a bad idea, but removing duplicate code is always-ish okay.
Yes it might be confusing for really new people, but IMHO it really tells you what it does, namely it resizes the vec with Default (values).
It removes the ambiguity between which one to choose, is then identical to the struct construction with Default (..Default::default).
Also a plus point the doc already has the correct example https://doc.rust-lang.org/std/vec/struct.Vec.html#method.resize_with

@rfcbot rfcbot added the finished-final-comment-period The final comment period is finished for this PR / Issue. label Feb 6, 2019
@rfcbot
Copy link

rfcbot commented Feb 6, 2019

The final comment period, with a disposition to merge, as per the review above, is now complete.

@rfcbot rfcbot removed the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Feb 6, 2019
@scottmcm
Copy link
Member Author

@SimonSapin looks like FCP has finished; are you good with the diff here?

@SimonSapin
Copy link
Contributor

Thanks for the PR!

@bors r+

@bors
Copy link
Contributor

bors commented Feb 19, 2019

📌 Commit 81cd1e6 has been approved by SimonSapin

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 19, 2019
@Manishearth
Copy link
Member

@bors rollup

Centril added a commit to Centril/rust that referenced this pull request Feb 20, 2019
…r=SimonSapin

Deprecate the unstable Vec::resize_default

As a way to either get additional feedback to stabilize or help move nightly users off it.

Tracking issue: rust-lang#41758 (comment)

r? @SimonSapin
Centril added a commit to Centril/rust that referenced this pull request Feb 22, 2019
…r=SimonSapin

Deprecate the unstable Vec::resize_default

As a way to either get additional feedback to stabilize or help move nightly users off it.

Tracking issue: rust-lang#41758 (comment)

r? @SimonSapin
Centril added a commit to Centril/rust that referenced this pull request Feb 22, 2019
…r=SimonSapin

Deprecate the unstable Vec::resize_default

As a way to either get additional feedback to stabilize or help move nightly users off it.

Tracking issue: rust-lang#41758 (comment)

r? @SimonSapin
bors added a commit that referenced this pull request Feb 22, 2019
Rollup of 17 pull requests

Successful merges:

 - #57656 (Deprecate the unstable Vec::resize_default)
 - #58059 (deprecate before_exec in favor of unsafe pre_exec)
 - #58064 (override `VecDeque::try_rfold`, also update iterator)
 - #58198 (Suggest removing parentheses surrounding lifetimes)
 - #58431 (fix overlapping references in BTree)
 - #58555 (Add a note about 2018e if someone uses `try {` in 2015e)
 - #58588 (remove a bit of dead code)
 - #58589 (cleanup macro after 2018 transition)
 - #58591 (Dedup a rustdoc diagnostic construction)
 - #58600 (fix small documentation typo)
 - #58601 (Search for target_triple.json only if builtin target not found)
 - #58606 (Docs: put Future trait into spotlight)
 - #58607 (Fixes #58586: Make E0505 erronous example fail for the 2018 edition)
 - #58615 (miri: explain why we use static alignment in ref-to-place conversion)
 - #58620 (introduce benchmarks of BTreeSet.intersection)
 - #58621 (Update miri links)
 - #58632 (Make std feature list sorted)

Failed merges:

r? @ghost
@bors bors merged commit 81cd1e6 into rust-lang:master Feb 23, 2019
@scottmcm scottmcm deleted the deprecate-resize_default branch February 23, 2019 09:38
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 16, 2020
Remove deprecated unstable Vec::resize_default

It's [been deprecated](rust-lang#57656) for 15 releases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

None yet