Fix pattern_from_macro_note for bit-or expr#154369
Fix pattern_from_macro_note for bit-or expr#154369rust-bors[bot] merged 1 commit intorust-lang:mainfrom
pattern_from_macro_note for bit-or expr#154369Conversation
rustbot
added
S-waiting-on-review
|
r? @JohnTitor rustbot has assigned @JohnTitor. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
ver-nyan
changed the title
pattern_from_macro_note for bit-or expr
Add bit-or detection in lower expr within pattern. Previously `pat1 | pat2` in `$e:expr` failed to trigger `.pattern_from_macro_note` diagnostic bc `ast::Expr::is_approximately_pattern()` did not cover bit-or.
ver-nyan
force-pushed
the
fix-bit_or-pat_macro-msg
branch
from
27848c4 to
2f46855
Compare
mb, it's been a while since i last contributed. Maybe this check can also be added in the git hook? |
| || matches!( | ||
| expr.peel_parens().kind, | ||
| ExprKind::Binary(Spanned { node: BinOpKind::BitOr, .. }, ..) | ||
| ); |
There was a problem hiding this comment.
I'm curious, is there a specific reason why you didn't move this check into is_approximately_pattern? Did it negatively impact other diagnostics that use this method?
Edit: Moving it into that function would also allow you to recurse into the operands to ensure that they're approximately patterns, too.
There was a problem hiding this comment.
I initially had it isolated in lower_expr_within_pat() bc it ended up doing an incorrect suggest_pattern_match_with_let()
e.g.
if (i + 2) = 3 {}
help: you might have meant to use pattern matching
|
LL | if let (i | 2) = 3 {}
| +++
Recursively lhs.is_approximately_pattern() || rhs.is_approximately_pattern() unfortunately didn't work since it would end up matching ExprKind::Lit(_) down the line.
There was a problem hiding this comment.
I just learned that undeclared variables are ExprKind::Path, so both Lit and Path will cause false positive let suggestion :c
A more formal example extending tests/ui/expr/if/bad-if-let-suggestion.rs:
if (i + 2) = 3 {} //~ ERROR cannot find value `i` in this scope
error[E0425]: cannot find value `i` in this scope
--> $DIR/bad-if-let-suggestion.rs:12:9
|
LL | if (i | 2) = 3 {}
| ^ not found in this scope
|
help: you might have meant to use pattern matching
|
LL | if let (i | 2) = 3 {}
| +++
Edit: also i think a recursive approach would lose it's "first-order'ness"
rust/compiler/rustc_ast/src/ast.rs
Lines 1638 to 1639 in 64d5cb6
There was a problem hiding this comment.
Thanks for investigating, given that, I think it's better to go ahead with the current impl.
rust-bors
Bot
added
S-waiting-on-bors
…uwer Rollup of 8 pull requests Successful merges: - #155047 (Always exhaustively match on typing mode) - #155080 (Simplify `try_load_from_disk_fn`.) - #152384 (Restrict EII declarations to functions at lowering time) - #153796 (Fix ICE when combining #[eii] with #[core::contracts::ensures]) - #154369 (Fix `pattern_from_macro_note` for bit-or expr) - #155027 ( Rename some more of our internal `#[rustc_*]` TEST attributes) - #155031 (delegation: fix unelided lifetime ICE, refactoring of GenericArgPosition) - #155040 (Fix code block whitespace handling in Markdown)
Rollup merge of #154369 - ver-nyan:fix-bit_or-pat_macro-msg, r=JohnTitor Fix `pattern_from_macro_note` for bit-or expr This is a continuation of issue #99380 (and pr #124488) but covers bit-or pattern. Essentially a `pat1 | pat2`, or any bit-or pattern used in a `$e:expr` was not firing the `.pattern_from_macro_note` diagnostic bc `ast::Expr::is_approximately_pattern()` did not cover bit-or. The cover for bit-or is only added in `lower_expr_within_pat()`, otherwise doing it in `is_approximately_pattern()` would result in the suggestion `if (i + 2) = 2 => if let (i + 2) = 2` from `suggest_pattern_match_with_let()` (which is the only other place `ast::Expr::is_approximately_pattern()` is used from what i see). resolves #99380 refs #124488
…uwer Rollup of 8 pull requests Successful merges: - rust-lang/rust#155047 (Always exhaustively match on typing mode) - rust-lang/rust#155080 (Simplify `try_load_from_disk_fn`.) - rust-lang/rust#152384 (Restrict EII declarations to functions at lowering time) - rust-lang/rust#153796 (Fix ICE when combining #[eii] with #[core::contracts::ensures]) - rust-lang/rust#154369 (Fix `pattern_from_macro_note` for bit-or expr) - rust-lang/rust#155027 ( Rename some more of our internal `#[rustc_*]` TEST attributes) - rust-lang/rust#155031 (delegation: fix unelided lifetime ICE, refactoring of GenericArgPosition) - rust-lang/rust#155040 (Fix code block whitespace handling in Markdown)
4 participants
This is a continuation of issue #99380 (and pr #124488) but covers bit-or pattern.
Essentially a
pat1 | pat2, or any bit-or pattern used in a$e:exprwas not firing the.pattern_from_macro_notediagnostic bcast::Expr::is_approximately_pattern()did not cover bit-or.The cover for bit-or is only added in
lower_expr_within_pat(), otherwise doing it inis_approximately_pattern()would result in the suggestionif (i + 2) = 2 => if let (i + 2) = 2fromsuggest_pattern_match_with_let()(which is the only other placeast::Expr::is_approximately_pattern()is used from what i see).resolves #99380
refs #124488