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.

Sign up for GitHub

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

Jump to bottom

Add #[must_use] to remaining core functions #89897

Merged
merged 1 commit into from Oct 31, 2021
Merged

Add #[must_use] to remaining core functions #89897

merged 1 commit into from Oct 31, 2021

Conversation

jkugelman
Copy link
Contributor

@jkugelman jkugelman commented Oct 14, 2021 •
edited

I've run out of compelling reasons to group functions together across crates so I'm just going to go module-by-module. This is everything remaining from the core crate.

Ignored by clippy for reasons unknown:

core::alloc::Layout   unsafe fn for_value_raw<T: ?Sized>(t: *const T) -> Self;
core::any             const fn type_name_of_val<T: ?Sized>(_val: &T) -> &'static str;

Ignored by clippy because of mut:

str   fn split_at_mut(&mut self, mid: usize) -> (&mut str, &mut str);
Ignored by clippy presumably because a caller might want `f` called for side effects. That seems like a bad usage of `map` to me. 
core::cell::Ref<'b, T>   fn map<U: ?Sized, F>(orig: Ref<'b, T>, f: F) -> Ref<'b, T>;
core::cell::Ref<'b, T>   fn map_split<U: ?Sized, V: ?Sized, F>(orig: Ref<'b, T>, f: F) -> (Ref<'b, U>, Ref<'b, V>);

Parent issue: #89692

r? @joshtriplett

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 14, 2021
@jkugelman jkugelman mentioned this pull request Oct 14, 2021
Closed
@apiraino apiraino added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Oct 21, 2021
@joshtriplett
Copy link
Member

I left a comment on two functions that I think should not have must_use. r=me with those two dropped.

@jkugelman
Copy link
Contributor Author

@joshtriplett What does r=me mean, by the way?

@joshtriplett
Copy link
Member

joshtriplett commented Oct 30, 2021 •
edited

@jkugelman If someone says "r=me with xyz fixed", that means "anyone with permission who comes along later is welcome to bors r=myusername if they see that xyz has been fixed; you don't need to wait for me to come back and re-review". So, for instance, if someone had come along after your change, before I did, they could have written (with an at-sign before bors) bors r=@joshtriplett. Given appropriate permissions, you could also do that yourself, on your own change, if you've made the requested trivial fix.

@joshtriplett
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Oct 30, 2021

📌 Commit 68b0d86 has been approved by joshtriplett

@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 Oct 30, 2021
@joshtriplett joshtriplett added the relnotes Marks issues that should be documented in the release notes of the next release. label Oct 30, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 31, 2021
@matthiaskrgr
Rollup merge of rust-lang#89897 - jkugelman:must-use-core, r=joshtrip…
f0a8a1d 
…lett

Add #[must_use] to remaining core functions

I've run out of compelling reasons to group functions together across crates so I'm just going to go module-by-module. This is everything remaining from the `core` crate.

Ignored by clippy for reasons unknown:

```rust
core::alloc::Layout   unsafe fn for_value_raw<T: ?Sized>(t: *const T) -> Self;
core::any             const fn type_name_of_val<T: ?Sized>(_val: &T) -> &'static str;
```

Ignored by clippy because of `mut`:

```rust
str   fn split_at_mut(&mut self, mid: usize) -> (&mut str, &mut str);
```

<del>
Ignored by clippy presumably because a caller might want `f` called for side effects. That seems like a bad usage of `map` to me.

```rust
core::cell::Ref<'b, T>   fn map<U: ?Sized, F>(orig: Ref<'b, T>, f: F) -> Ref<'b, T>;
core::cell::Ref<'b, T>   fn map_split<U: ?Sized, V: ?Sized, F>(orig: Ref<'b, T>, f: F) -> (Ref<'b, U>, Ref<'b, V>);
```
</del>

Parent issue: rust-lang#89692

r? `@joshtriplett`
@matthiaskrgr matthiaskrgr mentioned this pull request Oct 31, 2021
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 31, 2021
@matthiaskrgr
Rollup merge of rust-lang#89897 - jkugelman:must-use-core, r=joshtrip…
2d8b8f1 
…lett

Add #[must_use] to remaining core functions

I've run out of compelling reasons to group functions together across crates so I'm just going to go module-by-module. This is everything remaining from the `core` crate.

Ignored by clippy for reasons unknown:

```rust
core::alloc::Layout   unsafe fn for_value_raw<T: ?Sized>(t: *const T) -> Self;
core::any             const fn type_name_of_val<T: ?Sized>(_val: &T) -> &'static str;
```

Ignored by clippy because of `mut`:

```rust
str   fn split_at_mut(&mut self, mid: usize) -> (&mut str, &mut str);
```

<del>
Ignored by clippy presumably because a caller might want `f` called for side effects. That seems like a bad usage of `map` to me.

```rust
core::cell::Ref<'b, T>   fn map<U: ?Sized, F>(orig: Ref<'b, T>, f: F) -> Ref<'b, T>;
core::cell::Ref<'b, T>   fn map_split<U: ?Sized, V: ?Sized, F>(orig: Ref<'b, T>, f: F) -> (Ref<'b, U>, Ref<'b, V>);
```
</del>

Parent issue: rust-lang#89692

r? ``@joshtriplett``
@matthiaskrgr matthiaskrgr mentioned this pull request Oct 31, 2021
Closed
@matthiaskrgr matthiaskrgr mentioned this pull request Oct 31, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 31, 2021
@bors
Auto merge of rust-lang#90434 - matthiaskrgr:rollup-xbn393a, r=matthi…
58899c4 
…askrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#89446 (Add paragraph to ControlFlow docs to menion it works with the ? operator (rust-lang#88715))
 - rust-lang#89677 (Stabilize `is_symlink()` for `Metadata` and `Path`)
 - rust-lang#89833 (Add #[must_use] to Rc::downgrade)
 - rust-lang#89835 (Add #[must_use] to expensive computations)
 - rust-lang#89839 (Add #[must_use] to mem/ptr functions)
 - rust-lang#89897 (Add #[must_use] to remaining core functions)
 - rust-lang#89951 (Stabilize `option_result_unwrap_unchecked`)
 - rust-lang#90427 (Add #[must_use] to alloc functions that would leak memory)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 95750ae into rust-lang:master Oct 31, 2021
@rustbot rustbot added this to the 1.58.0 milestone Oct 31, 2021
@jkugelman jkugelman deleted the must-use-core branch October 31, 2021 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshtriplett joshtriplett joshtriplett left review comments

Assignees

@joshtriplett joshtriplett

Labels
relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.58.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@jkugelman @joshtriplett @bors @apiraino @rust-highfive @rustbot