[comparison_chain] #4827 Check core::cmp::Ord is implemented
#4842
Conversation
This file contains 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
tesuji
reviewed
Nov 24, 2019
clippy_lints/src/comparison_chain.rs
Outdated
|
|
||
| // Check that the type being compared implements `core::cmp::Ord` | ||
| let ty = cx.tables.expr_ty(lhs1); | ||
| let ord_id = get_trait_def_id(cx, &paths::ORD).unwrap(); |
There was a problem hiding this comment.
Suggested change
| let ord_id = get_trait_def_id(cx, &paths::ORD).unwrap(); | |
| if let Some(ord_id) = get_trait_def_id(cx, &paths::ORD); |
There was a problem hiding this comment.
There is no if_chain here, that would make this syntax work. Using map_or and a default value of false instead.
Only emit lint, if `cmp` is actually available on the type being compared. Don't emit lint in cases where only `PartialOrd` is implemented.
timbodeit
force-pushed
the
comparison-chain-false-positive-4827
branch
from
3708b8a to
fff9a8e
Compare
|
Getting rid of |
flip1995
added
the
S-waiting-on-bors
|
@bors r=flip1995 rollup |
|
📌 Commit fff9a8e has been approved by |
phansch
added a commit
to phansch/rust-clippy
that referenced
this pull request
Nov 28, 2019
…itive-4827, r=flip1995 [comparison_chain] rust-lang#4827 Check `core::cmp::Ord` is implemented Only emit `comparison_chain` lint, if `cmp` is actually available on the type being compared. Don't emit lint in cases where only `PartialOrd` is implemented. I haven't yet fully understood [Adjustments](https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ty/adjustment/struct.Adjustment.html). I would appreciate, if someone could double check whether my usage of [expr_ty](https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ty/struct.TypeckTables.html#method.expr_ty) in `clippy_lints/src/comparison_chain.rs:91` is correct or if there are cases where using [expr_ty_adjusted](https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ty/struct.TypeckTables.html#method.expr_ty_adjusted) would lead to a different result when used with `utils::implements_trait`. --- fixes rust-lang#4827 changelog: [comparison_chain] Check `core::cmp::Ord` is implemented
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Only emit
comparison_chainlint, ifcmpis actually available on the type being compared. Don't emit lint in cases where onlyPartialOrdis implemented.I haven't yet fully understood Adjustments. I would appreciate, if someone could double check whether my usage of expr_ty in
clippy_lints/src/comparison_chain.rs:91is correct or if there are cases where using expr_ty_adjusted would lead to a different result when used withutils::implements_trait.fixes #4827
changelog: [comparison_chain] Check
core::cmp::Ordis implemented