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

Rollup of 7 pull requests #128850

Closed
wants to merge 136 commits into from
Closed

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

meithecatte and others added 30 commits June 6, 2024 17:04
This commit fixes a bug introduced in rust-lang#12706, where the behavior of the
lint has been changed, to avoid suggestions that introduce a move. The
motivation in the commit message is quite poor (if the detection for
significant drops is not sufficient because it's not transitive, the
proper fix would be to make it transitive). However, rust-lang#12454, the linked
issue, provides a good reason for the change — if the value being
borrowed is bound to a variable, then moving it will only introduce
friction into future refactorings.

Thus rust-lang#12706 changes the logic so that the lint triggers if the value
being borrowed is Copy, or is the result of a function call, simplifying
the logic to the point where analysing "is this the only use of this
value" isn't necessary.

However, said PR also introduces an undocumented carveout, where
referents that themselves are mutable references are treated as Copy,
to catch some cases that we do want to lint against. However, that is
not sound — it's possible to consume a mutable reference by moving it.

To avoid emitting false suggestions, this PR reintroduces the
referent_used_exactly_once logic and runs that check for referents that
are themselves mutable references.

Thinking about the code shape of &mut x, where x: &mut T, raises the
point that while removing the &mut outright won't work, the extra
indirection is still undesirable, and perhaps instead we should suggest
reborrowing: &mut *x. That, however, is left as possible future work.

Fixes rust-lang#12856
TODO: Should we move `ty::peel_mid_ty_refs_is_mutable` to super module too?
Fixes rust-lang#12751

changelog: Fix [`redundant_slicing`] when the slice is behind a mutable reference
Clean up a few minor refs in `format!` macro, as it has a performance cost. Apparently the compiler is unable to inline `format!("{}", &variable)`, and does a run-time double-reference instead (format macro already does one level referencing).

Inlining format args prevents accidental `&` misuse.
Co-authored-by: Fridtjof Stoldt <xFrednet@gmail.com>
…-errors

Implement lint against ambiguous negative literals

This PR implements a lint against ambiguous negative literals with a literal and method calls right after it.

## `ambiguous_negative_literals`

(deny-by-default)

The `ambiguous_negative_literals` lint checks for cases that are confusing between a negative literal and a negation that's not part of the literal.

### Example

```rust,compile_fail
-1i32.abs(); // equals -1, while `(-1i32).abs()` equals 1
```

### Explanation

Method calls take precedence over unary precedence. Setting the precedence explicitly makes the code clearer and avoid potential bugs.

<details>
<summary>Old proposed lint</summary>

## `ambiguous_unary_precedence`

(deny-by-default)

The `ambiguous_unary_precedence` lint checks for use the negative unary operator with a literal and method calls.

### Example

```rust
-1i32.abs(); // equals -1, while `(-1i32).abs()` equals 1
```

### Explanation

Unary operations take precedence on binary operations and method calls take precedence over unary precedence. Setting the precedence explicitly makes the code clearer and avoid potential bugs.

</details>

-----

Note: This is a strip down version of rust-lang#117161, without the binary op precedence.

Fixes rust-lang#117155
`@rustbot` labels +I-lang-nominated
cc `@scottmcm`
r? compiler
Also get the receiver T in `T.method(|| f())`.
…=xFrednet

Remove unnecessary `res` field in `for_each_expr` visitors

Small refactor in the `for_each_expr*` visitors. This should not change anything functionally.

Instead of storing the final value `Option<B>` in the visitor and setting it to `Some` when we get a `ControlFlow::Break(B)` from the closure, we can just directly return it from the visitor itself now that visitors support that.

cc rust-lang#12829 and rust-lang/rust-clippy#12830 (comment)

changelog: none
… r=xFrednet

needless_borrows_for_generic_args: Fix for &mut

This commit fixes a bug introduced in rust-lang#12706, where the behavior of the lint has been changed, to avoid suggestions that introduce a move. The motivation in the commit message is quite poor (if the detection for significant drops is not sufficient because it's not transitive, the proper fix would be to make it transitive). However, rust-lang#12454, the linked issue, provides a good reason for the change — if the value being borrowed is bound to a variable, then moving it will only introduce friction into future refactorings.

Thus rust-lang#12706 changes the logic so that the lint triggers if the value being borrowed is Copy, or is the result of a function call, simplifying the logic to the point where analysing "is this the only use of this value" isn't necessary.

However, said PR also introduces an undocumented carveout, where referents that themselves are mutable references are treated as Copy, to catch some cases that we do want to lint against. However, that is not sound — it's possible to consume a mutable reference by moving it.

To avoid emitting false suggestions, this PR reintroduces the referent_used_exactly_once logic and runs that check for referents that are themselves mutable references.

Thinking about the code shape of &mut x, where x: &mut T, raises the point that while removing the &mut outright won't work, the extra indirection is still undesirable, and perhaps instead we should suggest reborrowing: &mut *x. That, however, is left as possible future work.

Fixes rust-lang#12856

changelog: none
…endoo

Fix handling of `Deref` in `assigning_clones`

The `assigning_clones` lint had a special case for producing a bit nicer code for mutable references:
```rust
fn clone_function_lhs_mut_ref(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
    *mut_thing = Clone::clone(ref_thing);
}
//v
fn clone_function_lhs_mut_ref(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
    Clone::clone_from(mut_thing, ref_thing);
}
```
However, this could [break](rust-lang/rust-clippy#12437) when combined with `Deref`.

This PR removes the special case, so that the generated code should work more generally. Later we can improve the detection of `Deref` and put the special case back in a way that does not break code.

Fixes: rust-lang/rust-clippy#12437

r? `@blyxyas`

changelog: [`assigning_clones`]: change applicability to `Unspecified` and fix a problem with `Deref`.
…tthiaskrgr

Clippy subtree update

r? `@Manishearth`

Updates Cargo.lock due to the Clippy version update and the ui_test bump to v0.24
…hat, r=y21

Make restriction lint's use `span_lint_and_then` (i -> p)

This migrates a few restriction lints to use `span_lint_and_then`. This change is motivated by rust-lang/rust-clippy#7797.

I've also cleaned up some lint message. Mostly minor stuff. For example: suggestions with a longer message than `"try"` now use `SuggestionStyle::ShowAlways`

---

cc: rust-lang/rust-clippy#7797

brother PR of: rust-lang/rust-clippy#13136

changelog: none
Signed-off-by: riyueguang <rustruby@outlook.com>
bors and others added 17 commits August 8, 2024 14:42
Don't use `LateContext` in the constant evaluator

This also changes the interface to require explicitly creating the context. `constant` could be added back in, but the others are probably not worth it.

A couple of bugs have been fixed. The wrong `TypeckResults` was used once when evaluating a constant, and the wrong `ParamEnv` was used by some callers (there wasn't a way to use the correct one).

changelog: none
modules are now stripped based on the same logic that's used to strip other item kinds
Rustup

r? `@ghost`

changelog: ICE fix [`uninit_vec`] through rust-lang#128720
rwlock: disable 'frob' test in Miri on macOS

Due to rust-lang#121950, Miri will sometimes complain about this test on macOS. Better disable the test, as otherwise it can fail for unrelated PRs.

r? `@joboet`
…=lcnr

Don't implement `AsyncFn` for `FnDef`/`FnPtr` that wouldnt implement `Fn`

Due to unsafety, ABI, or the presence of target features, some `FnDef`/`FnPtr` types don't implement `Fn*`. Do the same for `AsyncFn*`.

Noticed this due to rust-lang#128764, but this isn't really related to that ICE, which is fixed in rust-lang#128792.
Split `ColorConfig` off of `HumanReadableErrorType`

The previous setup tied two unrelated things together. Splitting these two is a better model.

Identified by https://github.com/rust-lang/rust/pull/126597/files#r1667800754
std float tests: special-case Miri in feature detection

Quick work-around to fix miri-test-libstd failures.

r? `@tgross35`
…nTheVoid

rustdoc: strip unreachable modules

Modules are now stripped based on the same logic that's used to strip other item kinds
Fixes rust-lang#101105
…, r=aDotInTheVoid

rustdoc-json: add a test for impls on private & hidden types

Fixes rust-lang#107278 (or rather just ensures it won't resurface)
r? `@aDotInTheVoid`
…Manishearth

Clippy subtree update

r? `@Manishearth`

Updates Cargo.lock due to uitest bump
@rustbot rustbot added A-rustdoc-json Area: Rustdoc JSON backend S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) rollup A PR which is a rollup labels Aug 8, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=7

@bors
Copy link
Contributor

bors commented Aug 8, 2024

📌 Commit 89ea6b2 has been approved by matthiaskrgr

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 8, 2024
@matthiaskrgr matthiaskrgr deleted the rollup-t2qpodp branch September 1, 2024 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-json Area: Rustdoc JSON backend rollup A PR which is a rollup 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.