Conversation
- only compare where predicates to trait bounds when generating where clause specific message to fix rust-lang#9151 - use comparable_trait_ref to account for trait bound generics to fix rust-lang#8757
…repr(C)` type is compatible with the other type
This enables more thorough checking of types to avoid triggering on custom Some and None enum variants
It is passed an argument that is never used.
…jgillot Simplify visitors By removing some unused arguments. r? `@cjgillot`
Update Clippy r? `@Manishearth`
Extend `if_then_some_else_none` to also suggest `bool::then_some` Closes rust-lang#9094. changelog: Extend `if_then_some_else_none` to also suggest `bool::then_some`
1.63 Changelog r? `@xFrednet` changelog: none [Rendered](https://github.com/flip1995/rust-clippy/blob/changelog/CHANGELOG.md)
Alexendoo
reviewed
Aug 29, 2022
Member
There was a problem hiding this comment.
For #101000 it'll be fine to tack it on here as | ...CountIsStar(_), can sort it out fully later in the clippy repo if there's anywhere that can use the info
Contributor
Author
There was a problem hiding this comment.
There's an error in the parsing somewhere. All the tests with * as the index aren't working.
Member
There was a problem hiding this comment.
Ah that'll be https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/write.rs#L543-L564, can apply the same treatment there
Fix missing parens in `suboptimal_flops` suggestion Fixes rust-lang#9391. The problem is simple enough, I didn't check if the same problem occurs elsewhere, though. changelog: fix missing parenthesis in `suboptimal_flops` suggestion
This comment has been minimized.
This comment has been minimized.
* Emit the lint in source order * Make suggestions with multiple traits be in source order rather than alphabetical
Initial implementation `result_large_err` This is a shot at rust-lang#6560, rust-lang#4652, and rust-lang#3884. The lint checks for `Result` being returned from functions/methods where the `Err` variant is larger than a configurable threshold (the default of which is 128 bytes). There has been some discussion around this, which I'll try to quickly summarize: * A large `Err`-variant may force an equally large `Result` if `Err` is actually bigger than `Ok`. * There is a cost involved in large `Result`, as LLVM may choose to `memcpy` them around above a certain size. * We usually expect the `Err` variant to be seldomly used, but pay the cost every time. * `Result` returned from library code has a high chance of bubbling up the call stack, getting stuffed into `MyLibError { IoError(std::io::Error), ParseError(parselib::Error), ...}`, exacerbating the problem. This PR deliberately does not take into account comparing the `Ok` to the `Err` variant (e.g. a ratio, or one being larger than the other). Rather we choose an absolute threshold for `Err`'s size, above which we warn. The reason for this is that `Err`s probably get `map_err`'ed further up the call stack, and we can't draw conclusions from the ratio at the point where the `Result` is returned. A relative threshold would also be less predictable, while not accounting for the cost of LLVM being forced to generate less efficient code if the `Err`-variant is _large_ in absolute terms. We lint private functions as well as public functions, as the perf-cost applies to in-crate code as well. In order to account for type-parameters, I conjured up `fn approx_ty_size`. The function relies on `LateContext::layout_of` to compute the actual size, and in case of failure (e.g. due to generics) tries to come up with an "at least size". In the latter case, the size of obviously wrong, but the inspected size certainly can't be smaller than that. Please give the approach a heavy dose of review, as I'm not actually familiar with the type-system at all (read: I have no idea what I'm doing). The approach does, however flimsy it is, allow us to successfully lint situations like ```rust pub union UnionError<T: Copy> { _maybe: T, _or_perhaps_even: (T, [u8; 512]), } // We know `UnionError<T>` will be at least 512 bytes, no matter what `T` is pub fn param_large_union<T: Copy>() -> Result<(), UnionError<T>> { Ok(()) } ``` I've given some refactoring to `functions/result_unit_err.rs` to re-use some bits. This is also the groundwork for rust-lang#6409 The default threshold is 128 because of rust-lang/rust-clippy#4652 (comment) `lintcheck` does not trigger this lint for a threshold of 128. It does warn for 64, though. The suggestion currently is the following, which is just a placeholder for discussion to be had. I did have the computed size in a `span_label`. However, that might cause both ui-tests here and lints elsewhere to become flaky wrt to their output (as the size is platform dependent). ``` error: the `Err`-variant returned via this `Result` is very large --> $DIR/result_large_err.rs:36:34 | LL | pub fn param_large_error<R>() -> Result<(), (u128, R, FullyDefinedLargeError)> { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ The `Err` variant is unusually large, at least 128 bytes ``` changelog: Add [`result_large_err`] lint
Fix the emission order of `trait_duplication_in_bounds` Makes the lint emit in source order rather than whatever order the hash map happens to be in. This is currently blocking the sync into rustc. changelog: None
Contributor
Author
|
@bors r+ |
Collaborator
bors
added
S-waiting-on-bors
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Aug 31, 2022
Update Clippy r? `@Manishearth`
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Aug 31, 2022
Update Clippy r? ``@Manishearth``
Member
|
@bors p=1 bitrotty |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 31, 2022
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#100787 (Pretty printing give proper error message without panic) - rust-lang#100838 (Suggest moving redundant generic args of an assoc fn to its trait) - rust-lang#100844 (migrate rustc_query_system to use SessionDiagnostic) - rust-lang#101140 (Update Clippy) - rust-lang#101161 (Fix uintended diagnostic caused by `drain(..)`) - rust-lang#101165 (Use more `into_iter` rather than `drain(..)`) - rust-lang#101229 (Link “? operator” to relevant chapter in The Book) - rust-lang#101230 (lint: avoid linting diag functions with diag lints) - rust-lang#101236 (Avoid needless buffer zeroing in `std::sys::windows::fs`) - rust-lang#101240 (Fix a typo on `wasm64-unknown-unknown` doc) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Collaborator
flip1995
pushed a commit
to flip1995/rust
that referenced
this pull request
Sep 9, 2022
…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#100787 (Pretty printing give proper error message without panic) - rust-lang#100838 (Suggest moving redundant generic args of an assoc fn to its trait) - rust-lang#100844 (migrate rustc_query_system to use SessionDiagnostic) - rust-lang#101140 (Update Clippy) - rust-lang#101161 (Fix uintended diagnostic caused by `drain(..)`) - rust-lang#101165 (Use more `into_iter` rather than `drain(..)`) - rust-lang#101229 (Link “? operator” to relevant chapter in The Book) - rust-lang#101230 (lint: avoid linting diag functions with diag lints) - rust-lang#101236 (Avoid needless buffer zeroing in `std::sys::windows::fs`) - rust-lang#101240 (Fix a typo on `wasm64-unknown-unknown` doc) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
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 free
to 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.
r? @Manishearth