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

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
9 participants
@scottmcm
Copy link
Member

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

@leonardo-m

This comment has been minimized.

Copy link

leonardo-m commented Jan 16, 2019

What's the point of this change?

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Jan 16, 2019

@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

This comment has been minimized.

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.

@clarcharr

This comment has been minimized.

Copy link
Contributor

clarcharr commented Jan 16, 2019

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

This comment has been minimized.

Copy link

leonardo-m commented Jan 16, 2019

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

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Jan 27, 2019

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

@avl

This comment has been minimized.

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

This comment has been minimized.

Copy link

scottjmaddox commented Jan 31, 2019

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

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Feb 1, 2019

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

@hellow554

This comment has been minimized.

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

This comment has been minimized.

Copy link

rfcbot commented Feb 6, 2019

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

@scottmcm

This comment has been minimized.

Copy link
Member Author

scottmcm commented Feb 12, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment