Skip to content

Fix pattern_from_macro_note for bit-or expr #154369

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.

Sign up for GitHub

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

Open
ver-nyan wants to merge 1 commit into
base: main
Choose a base branch
from
+26 −3
Open

Fix pattern_from_macro_note for bit-or expr #154369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion compiler/rustc_ast_lowering/src/pat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,11 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
}
_ => {
let is_const_block = matches!(expr.kind, ExprKind::ConstBlock(_));
let pattern_from_macro = expr.is_approximately_pattern();
let pattern_from_macro = expr.is_approximately_pattern()
|| matches!(
expr.peel_parens().kind,
ExprKind::Binary(Spanned { node: BinOpKind::BitOr, .. }, ..)
);
Comment on lines +422 to +425
Copy link
Member

@fmease fmease Mar 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@ver-nyan ver-nyan Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@ver-nyan ver-nyan Mar 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

/// To a first-order approximation, is this a pattern?
pub fn is_approximately_pattern(&self) -> bool {

let guar = self.dcx().emit_err(ArbitraryExpressionInPattern {
span,
pattern_from_macro_note: pattern_from_macro,
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/lowering/expr-in-pat-issue-99380.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,17 @@ macro_rules! foo {
};
}

macro_rules! custom_matches {
($e:expr, $p:expr) => {
match $e {
$p => true,
_ => false,
}
};
}

fn main() {
foo!(Some(3)); //~ ERROR arbitrary expressions aren't allowed in patterns

let _ = custom_matches!(67, 6 | 7); //~ ERROR arbitrary expressions aren't allowed in patterns
}
12 changes: 10 additions & 2 deletions tests/ui/lowering/expr-in-pat-issue-99380.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
error: arbitrary expressions aren't allowed in patterns
--> $DIR/expr-in-pat-issue-99380.rs:10:10
--> $DIR/expr-in-pat-issue-99380.rs:19:10
|
LL | foo!(Some(3));
| ^^^^^^^
|
= note: the `expr` fragment specifier forces the metavariable's content to be an expression

error: aborting due to 1 previous error
error: arbitrary expressions aren't allowed in patterns
--> $DIR/expr-in-pat-issue-99380.rs:21:33
|
LL | let _ = custom_matches!(67, 6 | 7);
| ^^^^^
|
= note: the `expr` fragment specifier forces the metavariable's content to be an expression

error: aborting due to 2 previous errors

Loading