Uh oh!
There was an error while loading. Please reload this page.
Rollup of 8 pull requests - #128853
Merged
Merged
Conversation
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. Fixesrust-lang#12856
TODO: Should we move `ty::peel_mid_ty_refs_is_mutable` to super module too?
Fixesrust-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. Fixesrust-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. Fixesrust-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>
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 Fixesrust-lang#101105
…, r=aDotInTheVoid rustdoc-json: add a test for impls on private & hidden types Fixesrust-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
…ir, r=matthiaskrgr Add comment that bors did not see pushed before it merged In rust-lang#128612, bors merged 470ada2 instead of 1e07c19. This means it dropped a useful comment I added, and a stage rename that is more descriptive.
matthiaskrgr
commented
Aug 8, 2024
MemberAuthor
@bors r+ rollup=never p=8 |
bors
commented
Aug 8, 2024
Collaborator
bors
commented
Aug 9, 2024
Collaborator
bors
commented
Aug 9, 2024
Collaborator
☀️ Test successful - checks-actions |
rust-timer
commented
Aug 9, 2024
Collaborator
📌 Perf builds for each rolled up PR:
previous master: c7b0d4e81f In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
rust-timer
commented
Aug 9, 2024
Collaborator
Finished benchmarking commit (fac7753): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary 3.3%, secondary 3.3%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 762.109s -> 761.187s (-0.12%) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Successful merges:
AsyncFnforFnDef/FnPtrthat wouldnt implementFn#128791 (Don't implementAsyncFnforFnDef/FnPtrthat wouldnt implementFn)ColorConfigoff ofHumanReadableErrorType#128806 (SplitColorConfigoff ofHumanReadableErrorType)r? @ghost
@rustbot modify labels: rollup
Create a similar rollup