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

mark std::string::String::new() and std::vec::Vec::new() as #[must_use]. #50766

Closed
wants to merge 1 commit into from

Conversation

matthiaskrgr
Copy link
Member

No description provided.

@rust-highfive
Copy link
Collaborator

r? @TimNN

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 15, 2018
@hanna-kruppe
Copy link
Contributor

Could you say a bit about the motivation? With many other methods where must_use is being proposed, I can see an argument about not using the result hiding a specific, subtle bug (e.g., calling String::trim and expecting it to work in-place) or seriously wasting resources (e.g., anything that allocates). But neither of those reasons applies to {Vec,String}::new().

@leoyvens
Copy link
Contributor

I suppose that the motivation is that if the return value is dead code then the constructing call is always dead code.

@scottmcm
Copy link
Member

In #48926 (comment), @est31 proposed a new lint that would obviate the need for changes like this (since both methods are const) which I think is a better approach than manually annotating all these.

@TimNN
Copy link
Contributor

TimNN commented May 19, 2018

I think we'll need some more consensus on general guidelines which std function should be marked #[must_use], so I'm marking this PR as blocked on #48926 for the time being.

@TimNN TimNN added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 19, 2018
@TimNN TimNN added A-allocators Area: Custom and system allocators and removed A-allocators Area: Custom and system allocators labels May 29, 2018
@TimNN TimNN added A-allocators Area: Custom and system allocators and removed A-allocators Area: Custom and system allocators labels Jun 12, 2018
@TimNN TimNN added A-allocators Area: Custom and system allocators and removed A-allocators Area: Custom and system allocators labels Jun 26, 2018
@TimNN
Copy link
Contributor

TimNN commented Aug 3, 2018

Thank you for your PR! It doesn't look like there will be a decision about how to handle #[must_use] functions in the standard library any time soon, so I'm going to close this PR for now.

@TimNN TimNN closed this Aug 3, 2018
@TimNN TimNN added S-blocked-closed and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Aug 3, 2018
@matthiaskrgr matthiaskrgr deleted the str_vec_must_use branch August 28, 2018 17:35
@jyn514 jyn514 added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-blocked-closed labels Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked Status: Blocked on something else such as an RFC or other implementation work.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants