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

Uplift clippy::for_loops_over_fallibles lint into rustc #99696

Merged
merged 19 commits into from
Oct 10, 2022

Conversation

WaffleLapkin
Copy link
Member

This PR, as the title suggests, uplifts clippy::for_loops_over_fallibles lint into rustc. This lint warns for code like this:

for _ in Some(1) {}
for _ in Ok::<_, ()>(1) {}

i.e. directly iterating over Option and Result using for loop.

There are a number of suggestions that this PR adds (on top of what clippy suggested):

  1. If the argument (? is there a better name for that expression) of a for loop is a .next() call, then we can suggest removing it (or rather replacing with .by_ref() to allow iterator being used later)
     for _ in iter.next() {}
     // turns into
     for _ in iter.by_ref() {}
  2. (otherwise) We can suggest using while let, this is useful for non-iterator, iterator-like things like [async] channels
    for _ in rx.recv() {}
    // turns into
    while let Some(_) = rx.recv() {}
  3. If the argument type is Result<impl IntoIterator, _> and the body has a Result<_, _> type, we can suggest using ?
    for _ in f() {}
    // turns into
    for _ in f()? {}
  4. To preserve the original behavior and clear intent, we can suggest using if let
    for _ in f() {}
    // turns into
    if let Some(_) = f() {}

(P.S. Some and Ok are interchangeable depending on the type)

I still feel that the lint wording/look is somewhat off, so I'll be happy to hear suggestions (on how to improve suggestions :D)!

Resolves #99272

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 24, 2022
@rust-highfive
Copy link
Collaborator

r? @fee1-dead

(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 Jul 24, 2022
@Mark-Simulacrum
Copy link
Member

To avoid stalling out on discussing improvements, I'd personally suggest dropping those from this PR and landing them in the future.

For example, the first one:

If the argument (? is there a better name for that expression) of a for loop is a .next() call, then we can suggest removing it (or rather replacing with .by_ref() to allow iterator being used later)

for _ in iter.next() {}
// turns into
for _ in iter.by_ref() {}

This looks pretty unfortunate to me -- those two lines have pretty different semantics. One consumes a single element from the iterator, while the second consumes all remaining elements of the iterator. I don't think rustc should be making suggestions that change the behavior of the code, in general, unless we're 100% confident that the user couldn't have wanted the original behavior, and I don't think that's the case here.

@rust-log-analyzer

This comment has been minimized.

@WaffleLapkin
Copy link
Member Author

@Mark-Simulacrum note that the if let suggestion (that does not change behavior) is always present. But, I don't think that people often write loops when wanting to check a single element, and so I think that behavior changing, but more sensible suggestion are useful.

@WaffleLapkin
Copy link
Member Author

Huh, turns out the compiler itself has places with for loops over options and that's why CI fails 😄

@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Jul 26, 2022

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jul 26, 2022

☔ The latest upstream changes (presumably #99764) made this pull request unmergeable. Please resolve the merge conflicts.

@fee1-dead
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 2, 2022
@rust-log-analyzer

This comment has been minimized.

@WaffleLapkin
Copy link
Member Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 18, 2022
Comment on lines +8 to +11
help: to check pattern in a loop use `while let`
|
LL | while let Some(_) = Some(1) {}
| ~~~~~~~~~~~~~~~ ~~~
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the late review. Could this be less noisy such that it would only suggest a while let if the rhs contains calls? Otherwise LGTM unless @Mark-Simulacrum has any concerns regarding the suggestions.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's not trivial? Basically we need to know if an expression can change when repeatedly evaluated... Conservative way of matching and asking "can this ExprKind potentially change?" makes almost every expression potentially changing (even Some(1), since it's a function), so unless you write something like for _ in (Some { 0: () }) {} you still get noisy suggestions...

@fee1-dead
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 23, 2022

📌 Commit 71b8c89 has been approved by fee1-dead

It is now in the queue for this repository.

@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 Aug 23, 2022
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Aug 24, 2022
Uplift `clippy::for_loops_over_fallibles` lint into rustc

This PR, as the title suggests, uplifts [`clippy::for_loops_over_fallibles`] lint into rustc. This lint warns for code like this:
```rust
for _ in Some(1) {}
for _ in Ok::<_, ()>(1) {}
```
i.e. directly iterating over `Option` and `Result` using `for` loop.

There are a number of suggestions that this PR adds (on top of what clippy suggested):
1. If the argument (? is there a better name for that expression) of a `for` loop is a `.next()` call, then we can suggest removing it (or rather replacing with `.by_ref()` to allow iterator being used later)
   ```rust
    for _ in iter.next() {}
    // turns into
    for _ in iter.by_ref() {}
    ```
2. (otherwise) We can suggest using `while let`, this is useful for non-iterator, iterator-like things like [async] channels
   ```rust
   for _ in rx.recv() {}
   // turns into
   while let Some(_) = rx.recv() {}
   ```
3. If the argument type is `Result<impl IntoIterator, _>` and the body has a `Result<_, _>` type, we can suggest using `?`
   ```rust
   for _ in f() {}
   // turns into
   for _ in f()? {}
   ```
4. To preserve the original behavior and clear intent, we can suggest using `if let`
   ```rust
   for _ in f() {}
   // turns into
   if let Some(_) = f() {}
   ```
(P.S. `Some` and `Ok` are interchangeable depending on the type)

I still feel that the lint wording/look is somewhat off, so I'll be happy to hear suggestions (on how to improve suggestions :D)!

Resolves rust-lang#99272

[`clippy::for_loops_over_fallibles`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loops_over_fallibles
@JohnTitor
Copy link
Member

Failed in rollup: #100974 (comment)

Seems we need some work on clippy test.

@bors r-

Noratrieb added a commit to Noratrieb/rust that referenced this pull request May 30, 2023
…cked, r=WaffleLapkin

Uplift `clippy::invalid_utf8_in_unchecked` lint

This PR aims at uplifting the `clippy::invalid_utf8_in_unchecked` lint into two lints.

## `invalid_from_utf8_unchecked`

(deny-by-default)

The `invalid_from_utf8_unchecked` lint checks for calls to `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut` with an invalid UTF-8 literal.

### Example

```rust
unsafe {
    std::str::from_utf8_unchecked(b"cl\x82ippy");
}
```

### Explanation

Creating such a `str` would result in undefined behavior as per documentation for `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut`.

## `invalid_from_utf8`

(warn-by-default)

The `invalid_from_utf8` lint checks for calls to `std::str::from_utf8` and `std::str::from_utf8_mut` with an invalid UTF-8 literal.

### Example

```rust
std::str::from_utf8(b"ru\x82st");
```

### Explanation

Trying to create such a `str` would always return an error as per documentation for `std::str::from_utf8` and `std::str::from_utf8_mut`.

-----

Mostly followed the instructions for uplifting a clippy lint described here: rust-lang#99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::invalid_utf8_in_unchecked` into rustc
Noratrieb added a commit to Noratrieb/rust that referenced this pull request May 30, 2023
…cked, r=WaffleLapkin

Uplift `clippy::invalid_utf8_in_unchecked` lint

This PR aims at uplifting the `clippy::invalid_utf8_in_unchecked` lint into two lints.

## `invalid_from_utf8_unchecked`

(deny-by-default)

The `invalid_from_utf8_unchecked` lint checks for calls to `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut` with an invalid UTF-8 literal.

### Example

```rust
unsafe {
    std::str::from_utf8_unchecked(b"cl\x82ippy");
}
```

### Explanation

Creating such a `str` would result in undefined behavior as per documentation for `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut`.

## `invalid_from_utf8`

(warn-by-default)

The `invalid_from_utf8` lint checks for calls to `std::str::from_utf8` and `std::str::from_utf8_mut` with an invalid UTF-8 literal.

### Example

```rust
std::str::from_utf8(b"ru\x82st");
```

### Explanation

Trying to create such a `str` would always return an error as per documentation for `std::str::from_utf8` and `std::str::from_utf8_mut`.

-----

Mostly followed the instructions for uplifting a clippy lint described here: rust-lang#99696 (review)

``@rustbot`` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::invalid_utf8_in_unchecked` into rustc
Noratrieb added a commit to Noratrieb/rust that referenced this pull request May 30, 2023
…cked, r=WaffleLapkin

Uplift `clippy::invalid_utf8_in_unchecked` lint

This PR aims at uplifting the `clippy::invalid_utf8_in_unchecked` lint into two lints.

## `invalid_from_utf8_unchecked`

(deny-by-default)

The `invalid_from_utf8_unchecked` lint checks for calls to `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut` with an invalid UTF-8 literal.

### Example

```rust
unsafe {
    std::str::from_utf8_unchecked(b"cl\x82ippy");
}
```

### Explanation

Creating such a `str` would result in undefined behavior as per documentation for `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut`.

## `invalid_from_utf8`

(warn-by-default)

The `invalid_from_utf8` lint checks for calls to `std::str::from_utf8` and `std::str::from_utf8_mut` with an invalid UTF-8 literal.

### Example

```rust
std::str::from_utf8(b"ru\x82st");
```

### Explanation

Trying to create such a `str` would always return an error as per documentation for `std::str::from_utf8` and `std::str::from_utf8_mut`.

-----

Mostly followed the instructions for uplifting a clippy lint described here: rust-lang#99696 (review)

```@rustbot``` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::invalid_utf8_in_unchecked` into rustc
Noratrieb added a commit to Noratrieb/rust that referenced this pull request May 30, 2023
…cked, r=WaffleLapkin

Uplift `clippy::invalid_utf8_in_unchecked` lint

This PR aims at uplifting the `clippy::invalid_utf8_in_unchecked` lint into two lints.

## `invalid_from_utf8_unchecked`

(deny-by-default)

The `invalid_from_utf8_unchecked` lint checks for calls to `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut` with an invalid UTF-8 literal.

### Example

```rust
unsafe {
    std::str::from_utf8_unchecked(b"cl\x82ippy");
}
```

### Explanation

Creating such a `str` would result in undefined behavior as per documentation for `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut`.

## `invalid_from_utf8`

(warn-by-default)

The `invalid_from_utf8` lint checks for calls to `std::str::from_utf8` and `std::str::from_utf8_mut` with an invalid UTF-8 literal.

### Example

```rust
std::str::from_utf8(b"ru\x82st");
```

### Explanation

Trying to create such a `str` would always return an error as per documentation for `std::str::from_utf8` and `std::str::from_utf8_mut`.

-----

Mostly followed the instructions for uplifting a clippy lint described here: rust-lang#99696 (review)

````@rustbot```` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::invalid_utf8_in_unchecked` into rustc
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 1, 2023
Uplift `clippy::cast_ref_to_mut` lint

This PR aims at uplifting the `clippy::cast_ref_to_mut` lint into rustc.

## `cast_ref_to_mut`

(deny-by-default)

The `cast_ref_to_mut` lint checks for casts of `&T` to `&mut T` without using interior mutability.

### Example

```rust,compile_fail
fn x(r: &i32) {
    unsafe {
        *(r as *const i32 as *mut i32) += 1;
    }
}
```

### Explanation

Casting `&T` to `&mut T` without interior mutability is undefined behavior, as it's a violation of Rust reference aliasing requirements.

-----

Mostly followed the instructions for uplifting a clippy lint described here: rust-lang#99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::cast_ref_to_mut` into rustc
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Jun 2, 2023
Uplift `clippy::cast_ref_to_mut` lint

This PR aims at uplifting the `clippy::cast_ref_to_mut` lint into rustc.

## `cast_ref_to_mut`

(deny-by-default)

The `cast_ref_to_mut` lint checks for casts of `&T` to `&mut T` without using interior mutability.

### Example

```rust,compile_fail
fn x(r: &i32) {
    unsafe {
        *(r as *const i32 as *mut i32) += 1;
    }
}
```

### Explanation

Casting `&T` to `&mut T` without interior mutability is undefined behavior, as it's a violation of Rust reference aliasing requirements.

-----

Mostly followed the instructions for uplifting a clippy lint described here: rust-lang/rust#99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::cast_ref_to_mut` into rustc
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Jun 2, 2023
…affleLapkin

Uplift `clippy::invalid_utf8_in_unchecked` lint

This PR aims at uplifting the `clippy::invalid_utf8_in_unchecked` lint into two lints.

## `invalid_from_utf8_unchecked`

(deny-by-default)

The `invalid_from_utf8_unchecked` lint checks for calls to `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut` with an invalid UTF-8 literal.

### Example

```rust
unsafe {
    std::str::from_utf8_unchecked(b"cl\x82ippy");
}
```

### Explanation

Creating such a `str` would result in undefined behavior as per documentation for `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut`.

## `invalid_from_utf8`

(warn-by-default)

The `invalid_from_utf8` lint checks for calls to `std::str::from_utf8` and `std::str::from_utf8_mut` with an invalid UTF-8 literal.

### Example

```rust
std::str::from_utf8(b"ru\x82st");
```

### Explanation

Trying to create such a `str` would always return an error as per documentation for `std::str::from_utf8` and `std::str::from_utf8_mut`.

-----

Mostly followed the instructions for uplifting a clippy lint described here: rust-lang/rust#99696 (review)

````@rustbot```` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::invalid_utf8_in_unchecked` into rustc
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 9, 2023
…s, r=compiler-errors

Uplift `clippy::undropped_manually_drops` lint

This PR aims at uplifting the `clippy::undropped_manually_drops` lint.

## `undropped_manually_drops`

(warn-by-default)

The `undropped_manually_drops` lint check for calls to `std::mem::drop` with a value of `std::mem::ManuallyDrop` which doesn't drop.

### Example

```rust
struct S;
drop(std::mem::ManuallyDrop::new(S));
```

### Explanation

`ManuallyDrop` does not drop it's inner value so calling `std::mem::drop` will not drop the inner value of the `ManuallyDrop` either.

-----

Mostly followed the instructions for uplifting an clippy lint described here: rust-lang#99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::undropped_manually_drops` into rustc
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 10, 2023
Uplift `clippy::cmp_nan` lint

This PR aims at uplifting the `clippy::cmp_nan` lint into rustc.

## `invalid_nan_comparisons`

~~(deny-by-default)~~ (warn-by-default)

The `invalid_nan_comparisons` lint checks comparison with `f32::NAN` or `f64::NAN` as one of the operand.

### Example

```rust,compile_fail
let a = 2.3f32;
if a == f32::NAN {}
```

### Explanation

NaN does not compare meaningfully to anything – not even itself – so those comparisons are always false.

-----

Mostly followed the instructions for uplifting a clippy lint described here: rust-lang#99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Jun 11, 2023
…iler-errors

Uplift `clippy::undropped_manually_drops` lint

This PR aims at uplifting the `clippy::undropped_manually_drops` lint.

## `undropped_manually_drops`

(warn-by-default)

The `undropped_manually_drops` lint check for calls to `std::mem::drop` with a value of `std::mem::ManuallyDrop` which doesn't drop.

### Example

```rust
struct S;
drop(std::mem::ManuallyDrop::new(S));
```

### Explanation

`ManuallyDrop` does not drop it's inner value so calling `std::mem::drop` will not drop the inner value of the `ManuallyDrop` either.

-----

Mostly followed the instructions for uplifting an clippy lint described here: rust-lang/rust#99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::undropped_manually_drops` into rustc
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Jun 29, 2023
…iler-errors

Uplift `clippy::undropped_manually_drops` lint

This PR aims at uplifting the `clippy::undropped_manually_drops` lint.

## `undropped_manually_drops`

(warn-by-default)

The `undropped_manually_drops` lint check for calls to `std::mem::drop` with a value of `std::mem::ManuallyDrop` which doesn't drop.

### Example

```rust
struct S;
drop(std::mem::ManuallyDrop::new(S));
```

### Explanation

`ManuallyDrop` does not drop it's inner value so calling `std::mem::drop` will not drop the inner value of the `ManuallyDrop` either.

-----

Mostly followed the instructions for uplifting an clippy lint described here: rust-lang/rust#99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::undropped_manually_drops` into rustc
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 11, 2023
Uplift `clippy::fn_null_check` lint

This PR aims at uplifting the `clippy::fn_null_check` lint into rustc.

## `incorrect_fn_null_checks`

(warn-by-default)

The `incorrect_fn_null_checks` lint checks for expression that checks if a function pointer is null.

### Example

```rust
let fn_ptr: fn() = /* somehow obtained nullable function pointer */

if (fn_ptr as *const ()).is_null() { /* ... */ }
```

### Explanation

Function pointers are assumed to be non-null, checking for their nullity is incorrect.

-----

Mostly followed the instructions for uplifting a clippy lint described here: rust-lang#99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Jul 12, 2023
Uplift `clippy::fn_null_check` lint

This PR aims at uplifting the `clippy::fn_null_check` lint into rustc.

## `incorrect_fn_null_checks`

(warn-by-default)

The `incorrect_fn_null_checks` lint checks for expression that checks if a function pointer is null.

### Example

```rust
let fn_ptr: fn() = /* somehow obtained nullable function pointer */

if (fn_ptr as *const ()).is_null() { /* ... */ }
```

### Explanation

Function pointers are assumed to be non-null, checking for their nullity is incorrect.

-----

Mostly followed the instructions for uplifting a clippy lint described here: rust-lang/rust#99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Jul 14, 2023
Uplift `clippy::fn_null_check` lint

This PR aims at uplifting the `clippy::fn_null_check` lint into rustc.

## `incorrect_fn_null_checks`

(warn-by-default)

The `incorrect_fn_null_checks` lint checks for expression that checks if a function pointer is null.

### Example

```rust
let fn_ptr: fn() = /* somehow obtained nullable function pointer */

if (fn_ptr as *const ()).is_null() { /* ... */ }
```

### Explanation

Function pointers are assumed to be non-null, checking for their nullity is incorrect.

-----

Mostly followed the instructions for uplifting a clippy lint described here: rust-lang/rust#99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler
thomcc pushed a commit to tcdi/postgrestd that referenced this pull request Jul 18, 2023
Uplift `clippy::{drop,forget}_{ref,copy}` lints

This PR aims at uplifting the `clippy::drop_ref`, `clippy::drop_copy`, `clippy::forget_ref` and `clippy::forget_copy` lints.

Those lints are/were declared in the correctness category of clippy because they lint on useless and most probably is not what the developer wanted.

## `drop_ref` and `forget_ref`

The `drop_ref` and `forget_ref` lint checks for calls to `std::mem::drop` or `std::mem::forget` with a reference instead of an owned value.

### Example

```rust
let mut lock_guard = mutex.lock();
std::mem::drop(&lock_guard) // Should have been drop(lock_guard), mutex
// still locked
operation_that_requires_mutex_to_be_unlocked();
```

### Explanation

Calling `drop` or `forget` on a reference will only drop the reference itself, which is a no-op. It will not call the `drop` or `forget` method on the underlying referenced value, which is likely what was intended.

## `drop_copy` and `forget_copy`

The `drop_copy` and `forget_copy` lint checks for calls to `std::mem::forget` or `std::mem::drop` with a value that derives the Copy trait.

### Example

```rust
let x: i32 = 42; // i32 implements Copy
std::mem::forget(x) // A copy of x is passed to the function, leaving the
                    // original unaffected
```

### Explanation

Calling `std::mem::forget` [does nothing for types that implement Copy](https://doc.rust-lang.org/std/mem/fn.drop.html) since the value will be copied and moved into the function on invocation.

-----

Followed the instructions for uplift a clippy describe here: rust-lang/rust#99696 (review)

cc `@m-ou-se` (as T-libs-api leader because the uplifting was discussed in a recent meeting)
thomcc pushed a commit to tcdi/postgrestd that referenced this pull request Aug 24, 2023
…affleLapkin

Uplift `clippy::invalid_utf8_in_unchecked` lint

This PR aims at uplifting the `clippy::invalid_utf8_in_unchecked` lint into two lints.

## `invalid_from_utf8_unchecked`

(deny-by-default)

The `invalid_from_utf8_unchecked` lint checks for calls to `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut` with an invalid UTF-8 literal.

### Example

```rust
unsafe {
    std::str::from_utf8_unchecked(b"cl\x82ippy");
}
```

### Explanation

Creating such a `str` would result in undefined behavior as per documentation for `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut`.

## `invalid_from_utf8`

(warn-by-default)

The `invalid_from_utf8` lint checks for calls to `std::str::from_utf8` and `std::str::from_utf8_mut` with an invalid UTF-8 literal.

### Example

```rust
std::str::from_utf8(b"ru\x82st");
```

### Explanation

Trying to create such a `str` would always return an error as per documentation for `std::str::from_utf8` and `std::str::from_utf8_mut`.

-----

Mostly followed the instructions for uplifting a clippy lint described here: rust-lang/rust#99696 (review)

````@rustbot```` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::invalid_utf8_in_unchecked` into rustc
thomcc pushed a commit to tcdi/postgrestd that referenced this pull request Aug 24, 2023
Uplift `clippy::cast_ref_to_mut` lint

This PR aims at uplifting the `clippy::cast_ref_to_mut` lint into rustc.

## `cast_ref_to_mut`

(deny-by-default)

The `cast_ref_to_mut` lint checks for casts of `&T` to `&mut T` without using interior mutability.

### Example

```rust,compile_fail
fn x(r: &i32) {
    unsafe {
        *(r as *const i32 as *mut i32) += 1;
    }
}
```

### Explanation

Casting `&T` to `&mut T` without interior mutability is undefined behavior, as it's a violation of Rust reference aliasing requirements.

-----

Mostly followed the instructions for uplifting a clippy lint described here: rust-lang/rust#99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::cast_ref_to_mut` into rustc
thomcc pushed a commit to tcdi/postgrestd that referenced this pull request Aug 24, 2023
…iler-errors

Uplift `clippy::undropped_manually_drops` lint

This PR aims at uplifting the `clippy::undropped_manually_drops` lint.

## `undropped_manually_drops`

(warn-by-default)

The `undropped_manually_drops` lint check for calls to `std::mem::drop` with a value of `std::mem::ManuallyDrop` which doesn't drop.

### Example

```rust
struct S;
drop(std::mem::ManuallyDrop::new(S));
```

### Explanation

`ManuallyDrop` does not drop it's inner value so calling `std::mem::drop` will not drop the inner value of the `ManuallyDrop` either.

-----

Mostly followed the instructions for uplifting an clippy lint described here: rust-lang/rust#99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::undropped_manually_drops` into rustc
thomcc pushed a commit to tcdi/postgrestd that referenced this pull request Aug 24, 2023
Uplift `clippy::cmp_nan` lint

This PR aims at uplifting the `clippy::cmp_nan` lint into rustc.

## `invalid_nan_comparisons`

~~(deny-by-default)~~ (warn-by-default)

The `invalid_nan_comparisons` lint checks comparison with `f32::NAN` or `f64::NAN` as one of the operand.

### Example

```rust,compile_fail
let a = 2.3f32;
if a == f32::NAN {}
```

### Explanation

NaN does not compare meaningfully to anything – not even itself – so those comparisons are always false.

-----

Mostly followed the instructions for uplifting a clippy lint described here: rust-lang/rust#99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler
thomcc pushed a commit to tcdi/postgrestd that referenced this pull request Oct 17, 2023
Uplift `clippy::fn_null_check` lint

This PR aims at uplifting the `clippy::fn_null_check` lint into rustc.

## `incorrect_fn_null_checks`

(warn-by-default)

The `incorrect_fn_null_checks` lint checks for expression that checks if a function pointer is null.

### Example

```rust
let fn_ptr: fn() = /* somehow obtained nullable function pointer */

if (fn_ptr as *const ()).is_null() { /* ... */ }
```

### Explanation

Function pointers are assumed to be non-null, checking for their nullity is incorrect.

-----

Mostly followed the instructions for uplifting a clippy lint described here: rust-lang/rust#99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024
…iler-errors

Uplift `clippy::undropped_manually_drops` lint

This PR aims at uplifting the `clippy::undropped_manually_drops` lint.

## `undropped_manually_drops`

(warn-by-default)

The `undropped_manually_drops` lint check for calls to `std::mem::drop` with a value of `std::mem::ManuallyDrop` which doesn't drop.

### Example

```rust
struct S;
drop(std::mem::ManuallyDrop::new(S));
```

### Explanation

`ManuallyDrop` does not drop it's inner value so calling `std::mem::drop` will not drop the inner value of the `ManuallyDrop` either.

-----

Mostly followed the instructions for uplifting an clippy lint described here: rust-lang/rust#99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::undropped_manually_drops` into rustc
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
…iler-errors

Uplift `clippy::undropped_manually_drops` lint

This PR aims at uplifting the `clippy::undropped_manually_drops` lint.

## `undropped_manually_drops`

(warn-by-default)

The `undropped_manually_drops` lint check for calls to `std::mem::drop` with a value of `std::mem::ManuallyDrop` which doesn't drop.

### Example

```rust
struct S;
drop(std::mem::ManuallyDrop::new(S));
```

### Explanation

`ManuallyDrop` does not drop it's inner value so calling `std::mem::drop` will not drop the inner value of the `ManuallyDrop` either.

-----

Mostly followed the instructions for uplifting an clippy lint described here: rust-lang/rust#99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::undropped_manually_drops` into rustc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uplift clippy::for_loops_over_fallibles to rustc