Skip to content

Fix pattern_from_macro_note for bit-or expr#154369

Open
ver-nyan wants to merge 1 commit intorust-lang:mainfrom
ver-nyan:fix-bit_or-pat_macro-msg
Open

Fix pattern_from_macro_note for bit-or expr#154369
ver-nyan wants to merge 1 commit intorust-lang:mainfrom
ver-nyan:fix-bit_or-pat_macro-msg

Conversation

@ver-nyan
Copy link
Contributor

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 25, 2026
@rustbot
Copy link
Collaborator

rustbot commented Mar 25, 2026

r? @JohnTitor

rustbot has assigned @JohnTitor.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 69 candidates
  • Random selection from 13 candidates

@rustbot

This comment has been minimized.

@ver-nyan ver-nyan changed the title Fix pattern_macro_note detection for bit-or expr Fix pattern_from_macro_note for bit-or expr Mar 25, 2026
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 ver-nyan force-pushed the fix-bit_or-pat_macro-msg branch from 27848c4 to 2f46855 Compare March 25, 2026 13:21
@ver-nyan
Copy link
Contributor Author

⚠️ Warning ⚠️

* There are issue links (such as `#123`) in the commit messages of the following commits.
  _Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree._

mb, it's been a while since i last contributed. Maybe this check can also be added in the git hook?

Comment on lines +422 to +425
|| matches!(
expr.peel_parens().kind,
ExprKind::Binary(Spanned { node: BinOpKind::BitOr, .. }, ..)
);
Copy link
Member

@fmease fmease Mar 25, 2026

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

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

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"

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Confusing "arbitrary expressions aren't allowed in patterns" error

4 participants