Skip to content

Add #[must_use] to a few standard library methods - #49533

Merged
alexcrichton merged 1 commit into
rust-lang:masterfrom
scottmcm:more-must-use
Apr 5, 2018
Merged

Add #[must_use] to a few standard library methods#49533
alexcrichton merged 1 commit into
rust-lang:masterfrom
scottmcm:more-must-use

Conversation

@scottmcm

Copy link
Copy Markdown
Member

Chosen to start a precedent of using it on ones that are potentially-expensive and where using it for side effects is particularly discouraged.

Discuss :)

warning: unused return value of `std::iter::Iterator::collect` which must be used:if you really need to exhaust the iterator, consider `.for_each(drop)` instead
--> $DIR/fn_must_use_stdlib.rs:19:5
|
LL | "1 2 3".split_whitespace().collect::<Vec<_>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning:unusedreturn value of `std::borrow::ToOwned::to_owned` which must be used: cloning is often expensive and is not expected to have side effects
--> $DIR/fn_must_use_stdlib.rs:21:5
|
LL | "hello".to_owned();
| ^^^^^^^^^^^^^^^^^^^
warning:unusedreturn value of `std::clone::Clone::clone` which must be used: cloning is often expensive and is not expected to have side effects
--> $DIR/fn_must_use_stdlib.rs:23:5
|
LL | String::from("world").clone();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

cc #48926

Chosen to start a precedent of using it on ones that are potentially-expensive and where using it for side effects is particularly discouraged.
Discuss :)
@scottmcm

Copy link
Copy Markdown
MemberAuthor

Looks like highfive was asleep for this one, so picking, from the must_use tracking issue,

r? @nikomatsakis

@nikomatsakis

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Apr 2, 2018

Copy link
Copy Markdown
Collaborator

📌 Commit fb7deda has been approved by nikomatsakis

@borsbors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 2, 2018
/// [`Result`]: ../../std/result/enum.Result.html
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
#[must_use = "if you really need to exhaust the iterator, consider `.for_each(drop)` instead"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is going to be annoying for the "collect to unit" use-case. I don't know how much we want to support that, but I thought it worth pointing out.

@ExpHPExpHPApr 2, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AFAICT Unit-returning functions never trigger the warning, even if they are the result of monomorphising a function generic in its output type. Also, there's a use case for that FromIterator implementation besides just making collect::<Result<()>> work?

kennytm added a commit to kennytm/rust that referenced this pull request Apr 3, 2018
Add #[must_use] to a few standard library methods
Chosen to start a precedent of using it on ones that are potentially-expensive and where using it for side effects is particularly discouraged.
Discuss :)
```rust
warning: unused return value of `std::iter::Iterator::collect` which must be used: if you really need to exhaust the iterator, consider `.for_each(drop)` instead
--> $DIR/fn_must_use_stdlib.rs:19:5
|
LL | "1 2 3".split_whitespace().collect::<Vec<_>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: unused return value of `std::borrow::ToOwned::to_owned` which must be used: cloning is often expensive and is not expected to have side effects
--> $DIR/fn_must_use_stdlib.rs:21:5
|
LL | "hello".to_owned();
| ^^^^^^^^^^^^^^^^^^^
warning: unused return value of `std::clone::Clone::clone` which must be used: cloning is often expensive and is not expected to have side effects
--> $DIR/fn_must_use_stdlib.rs:23:5
|
LL | String::from("world").clone();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```
cc rust-lang#48926
@kennytmkennytm mentioned this pull request Apr 3, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Apr 3, 2018
Add #[must_use] to a few standard library methods
Chosen to start a precedent of using it on ones that are potentially-expensive and where using it for side effects is particularly discouraged.
Discuss :)
```rust
warning: unused return value of `std::iter::Iterator::collect` which must be used: if you really need to exhaust the iterator, consider `.for_each(drop)` instead
--> $DIR/fn_must_use_stdlib.rs:19:5
|
LL | "1 2 3".split_whitespace().collect::<Vec<_>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: unused return value of `std::borrow::ToOwned::to_owned` which must be used: cloning is often expensive and is not expected to have side effects
--> $DIR/fn_must_use_stdlib.rs:21:5
|
LL | "hello".to_owned();
| ^^^^^^^^^^^^^^^^^^^
warning: unused return value of `std::clone::Clone::clone` which must be used: cloning is often expensive and is not expected to have side effects
--> $DIR/fn_must_use_stdlib.rs:23:5
|
LL | String::from("world").clone();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```
cc rust-lang#48926
@kennytmkennytm mentioned this pull request Apr 3, 2018
bors added a commit that referenced this pull request Apr 3, 2018
Rollup of 14 pull requests
Successful merges:
- #49179 (Handle future deprecation annotations )
- #49512 (Add support for variant and types fields for intra links)
- #49516 (Add missing anchor for union type fields)
- #49532 (Add test for rustdoc ignore test)
- #49533 (Add #[must_use] to a few standard library methods)
- #49570 (avoid IdxSets containing garbage above the universe length)
- #49594 (Add some performance guidance to std::fs and std::io docs)
- #49599 (Fix typo)
- #49603 (Fix url for intra link provided method)
- #49609 (run-pass/attr-stmt-expr: expand test cases)
- #49612 (Fix "since" version for getpid feature.)
- #49618 (Fix build error when compiling libcore for 16bit targets)
- #49619 (tweak core::fmt docs)
- #49623 (update mdbook)
Failed merges:
kennytm added a commit to kennytm/rust that referenced this pull request Apr 4, 2018
Add #[must_use] to a few standard library methods
Chosen to start a precedent of using it on ones that are potentially-expensive and where using it for side effects is particularly discouraged.
Discuss :)
```rust
warning: unused return value of `std::iter::Iterator::collect` which must be used: if you really need to exhaust the iterator, consider `.for_each(drop)` instead
--> $DIR/fn_must_use_stdlib.rs:19:5
|
LL | "1 2 3".split_whitespace().collect::<Vec<_>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
warning: unused return value of `std::borrow::ToOwned::to_owned` which must be used: cloning is often expensive and is not expected to have side effects
--> $DIR/fn_must_use_stdlib.rs:21:5
|
LL | "hello".to_owned();
| ^^^^^^^^^^^^^^^^^^^
warning: unused return value of `std::clone::Clone::clone` which must be used: cloning is often expensive and is not expected to have side effects
--> $DIR/fn_must_use_stdlib.rs:23:5
|
LL | String::from("world").clone();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```
cc rust-lang#48926
@kennytmkennytm mentioned this pull request Apr 4, 2018
bors added a commit that referenced this pull request Apr 4, 2018
Rollup of 25 pull requests
Successful merges:
- #49179 (Handle future deprecation annotations )
- #49512 (Add support for variant and types fields for intra links)
- #49515 (fix targetted value background)
- #49516 (Add missing anchor for union type fields)
- #49532 (Add test for rustdoc ignore test)
- #49533 (Add #[must_use] to a few standard library methods)
- #49540 (Fix miri Discriminant() for non-ADT)
- #49559 (Introduce Vec::resize_with method (see #41758))
- #49570 (avoid IdxSets containing garbage above the universe length)
- #49577 (Stabilize String::replace_range)
- #49599 (Fix typo)
- #49603 (Fix url for intra link provided method)
- #49607 (Stabilize iterator methods in 1.27)
- #49609 (run-pass/attr-stmt-expr: expand test cases)
- #49612 (Fix "since" version for getpid feature.)
- #49618 (Fix build error when compiling libcore for 16bit targets)
- #49619 (tweak core::fmt docs)
- #49637 (Stabilize parent_id())
- #49639 (Update Cargo)
- #49628 (Re-write the documentation index)
- #49594 (Add some performance guidance to std::fs and std::io docs)
- #49625 (miri: add public alloc_kind accessor)
- #49634 (Add a test for the fix to issue #43058)
- #49641 (Regression test for #46314)
- #49547 (Unignore borrowck test)
Failed merges:
@alexcrichton
alexcrichton merged commit fb7deda into rust-lang:masterApr 5, 2018
@scottmcm
scottmcm deleted the more-must-use branch April 5, 2018 00:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@scottmcm@nikomatsakis@bors@ExpHP@cramertj@alexcrichton