-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
_match.rs: prune sub-match tree too aggressively #13034
Conversation
let m2 = const_eval::compare_lit_exprs(tcx, a2, b_expr); | ||
match (m1, m2) { | ||
// b is in range [a1, a2] iff a1 <= b and b <= a2 | ||
(Some(val1), Some(val2)) => (val1 <= 0 && 0 <= val2), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, does compare_lit_exprs
return -1
, 0
or 1
? How peculiar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it does that like Java did :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we have an Ordering
enum just for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, val1 <= Equal && Equal <= val2
that'll be more readable.
After reading into the code, I don't think this is the right answer. I wrote details (a lot more details) here: #13027 (comment) |
r? |
(r+ with a comment, in case that was unclear) |
The `_match.rs` takes advantage of passes prior to `trans` and aggressively prunes the sub-match tree based on exact equality. When it comes to literal or range, the strategy may lead to wrong result if there's guard function or multiple patterns inside tuple. Closes rust-lang#12582. Closes rust-lang#13027.
It has been found that rust-lang#13034 was flawed and caused regression rust-lang#13867. This patch reveres the changes made by it except the companion tests.
fix: escape keywords used as names in earlier editions Fixes rust-lang#13030 There are keywords in Rust 2018+ that you can use as names without escaping when your crate is in Rust 2015 e.g. "try". We need to be consistent on how to keep track of the names regardless of how they are actually written in each crate. This patch attempts at it by taking such names into account and storing them uniformly in their escaped form.
…, r=Veykril fix: Search raw identifiers without prefix When we find references/usages of a raw identifier, we should disregard `r#` prefix because there are keywords one can use without the prefix in earlier editions (see rust-lang#13034; this bug is actually fallout from the PR). `name`, the text we're searching for, has already been stripped of the prefix, but the text of nodes we compare it to hasn't been. The second commit is strictly refactoring, I can remove it if it's not much of value.
Add new `trivial_map_over_range` lint This lint checks for code that looks like ```rust let something : Vec<_> = (0..100).map(|_| { 1 + 2 + 3 }).collect(); ``` which is more clear as ```rust let something : Vec<_> = std::iter::repeat_with(|| { 1 + 2 + 3 }).take(100).collect(); ``` That is, a map over a range which does nothing with the parameter passed to it is simply a function (or closure) being called `n` times and could be more semantically expressed using `take`. - [x] Followed [lint naming conventions][lint_naming] - [x] Added passing UI tests (including committed `.stderr` file) - [x] `cargo test` passes locally - [x] Executed `cargo dev update_lints` - [x] Added lint documentation - [x] Run `cargo dev fmt` changelog: new lint: [`trivial_map_over_range`] `restriction`
The
_match.rs
takes advantage of passes prior totrans
andaggressively prunes the sub-match tree based on exact equality. When it
comes to literal or range, the strategy may lead to wrong result if
there's guard function or multiple patterns inside tuple.
Closes #12582.
Closes #13027.