Skip to content

Exhaustively handle expressions in patterns #134228

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

Merged
merged 4 commits into from
Jan 9, 2025
Merged
Changes from all commits
Commits
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: 3 additions & 3 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
@@ -623,7 +623,7 @@ impl Pat {
PatKind::Wild
| PatKind::Rest
| PatKind::Never
| PatKind::Lit(_)
| PatKind::Expr(_)
| PatKind::Range(..)
| PatKind::Ident(..)
| PatKind::Path(..)
@@ -801,8 +801,8 @@ pub enum PatKind {
/// A reference pattern (e.g., `&mut (a, b)`).
Ref(P<Pat>, Mutability),

/// A literal.
Lit(P<Expr>),
/// A literal, const block or path.
Expr(P<Expr>),
Comment on lines +804 to +805
Copy link
Contributor

Choose a reason for hiding this comment

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

@oli-obk so the P<Expr> doesn't actually contain const blocks or paths just yet. You only renamed the variant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it always contained these. I'm working towards making that explicit. I wasn't going to touch the ast immediately, but considering that non-lit variants slipped into rustfmt without notice, I should probably do it sooner.

Until then we can just make rustfmt match exhaustively on lit/neg(lit)/constblock/path and panic in the other cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

I had no idea that was the case. Well, thanks for clearing that up.


/// A range pattern (e.g., `1...2`, `1..2`, `1..`, `..2`, `1..=2`, `..=2`).
Range(Option<P<Expr>>, Option<P<Expr>>, Spanned<RangeEnd>),
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
@@ -1512,7 +1512,7 @@ pub fn walk_pat<T: MutVisitor>(vis: &mut T, pat: &mut P<Pat>) {
vis.visit_ident(ident);
visit_opt(sub, |sub| vis.visit_pat(sub));
}
PatKind::Lit(e) => vis.visit_expr(e),
PatKind::Expr(e) => vis.visit_expr(e),
PatKind::TupleStruct(qself, path, elems) => {
vis.visit_qself(qself);
vis.visit_path(path);
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/visit.rs
Original file line number Diff line number Diff line change
@@ -680,7 +680,7 @@ pub fn walk_pat<'a, V: Visitor<'a>>(visitor: &mut V, pattern: &'a Pat) -> V::Res
try_visit!(visitor.visit_ident(ident));
visit_opt!(visitor, visit_pat, optional_subpattern);
}
PatKind::Lit(expression) => try_visit!(visitor.visit_expr(expression)),
PatKind::Expr(expression) => try_visit!(visitor.visit_expr(expression)),
PatKind::Range(lower_bound, upper_bound, _end) => {
visit_opt!(visitor, visit_expr, lower_bound);
visit_opt!(visitor, visit_expr, upper_bound);
51 changes: 28 additions & 23 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
@@ -102,17 +102,7 @@ impl<'hir> LoweringContext<'_, 'hir> {

let kind = match &e.kind {
ExprKind::Array(exprs) => hir::ExprKind::Array(self.lower_exprs(exprs)),
ExprKind::ConstBlock(c) => {
let c = self.with_new_scopes(c.value.span, |this| {
let def_id = this.local_def_id(c.id);
hir::ConstBlock {
def_id,
hir_id: this.lower_node_id(c.id),
body: this.lower_const_body(c.value.span, Some(&c.value)),
}
});
hir::ExprKind::ConstBlock(c)
}
ExprKind::ConstBlock(c) => hir::ExprKind::ConstBlock(self.lower_const_block(c)),
ExprKind::Repeat(expr, count) => {
let expr = self.lower_expr(expr);
let count = self.lower_array_length_to_const_arg(count);
@@ -153,18 +143,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let ohs = self.lower_expr(ohs);
hir::ExprKind::Unary(op, ohs)
}
ExprKind::Lit(token_lit) => {
let lit_kind = match LitKind::from_token_lit(*token_lit) {
Ok(lit_kind) => lit_kind,
Err(err) => {
let guar =
report_lit_error(&self.tcx.sess.psess, err, *token_lit, e.span);
LitKind::Err(guar)
}
};
let lit = self.arena.alloc(respan(self.lower_span(e.span), lit_kind));
hir::ExprKind::Lit(lit)
}
ExprKind::Lit(token_lit) => hir::ExprKind::Lit(self.lower_lit(token_lit, e.span)),
ExprKind::IncludedBytes(bytes) => {
let lit = self.arena.alloc(respan(
self.lower_span(e.span),
@@ -403,6 +382,32 @@ impl<'hir> LoweringContext<'_, 'hir> {
})
}

pub(crate) fn lower_const_block(&mut self, c: &AnonConst) -> hir::ConstBlock {
self.with_new_scopes(c.value.span, |this| {
let def_id = this.local_def_id(c.id);
hir::ConstBlock {
def_id,
hir_id: this.lower_node_id(c.id),
body: this.lower_const_body(c.value.span, Some(&c.value)),
}
})
}

pub(crate) fn lower_lit(
&mut self,
token_lit: &token::Lit,
span: Span,
) -> &'hir Spanned<LitKind> {
let lit_kind = match LitKind::from_token_lit(*token_lit) {
Ok(lit_kind) => lit_kind,
Err(err) => {
let guar = report_lit_error(&self.tcx.sess.psess, err, *token_lit, span);
LitKind::Err(guar)
}
};
self.arena.alloc(respan(self.lower_span(span), lit_kind))
}

fn lower_unop(&mut self, u: UnOp) -> hir::UnOp {
match u {
UnOp::Deref => hir::UnOp::Deref,
8 changes: 8 additions & 0 deletions compiler/rustc_ast_lowering/src/index.rs
Original file line number Diff line number Diff line change
@@ -209,6 +209,14 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
});
}

fn visit_pat_expr(&mut self, expr: &'hir PatExpr<'hir>) {
self.insert(expr.span, expr.hir_id, Node::PatExpr(expr));

self.with_parent(expr.hir_id, |this| {
intravisit::walk_pat_expr(this, expr);
});
}

fn visit_pat_field(&mut self, field: &'hir PatField<'hir>) {
self.insert(field.span, field.hir_id, Node::PatField(field));
self.with_parent(field.hir_id, |this| {
1 change: 1 addition & 0 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
@@ -35,6 +35,7 @@
#![doc(rust_logo)]
#![feature(assert_matches)]
#![feature(box_patterns)]
#![feature(if_let_guard)]
#![feature(let_chains)]
#![feature(rustdoc_internals)]
#![warn(unreachable_pub)]
63 changes: 48 additions & 15 deletions compiler/rustc_ast_lowering/src/pat.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
use std::sync::Arc;

use rustc_ast::ptr::P;
use rustc_ast::*;
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_span::source_map::Spanned;
use rustc_middle::span_bug;
use rustc_span::source_map::{Spanned, respan};
use rustc_span::{Ident, Span};

use super::errors::{
@@ -35,8 +38,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
lower_sub,
);
}
PatKind::Lit(e) => {
break hir::PatKind::Lit(self.lower_expr_within_pat(e, false));
PatKind::Expr(e) => {
break hir::PatKind::Expr(self.lower_expr_within_pat(e, false));
}
PatKind::TupleStruct(qself, path, pats) => {
let qpath = self.lower_qpath(
@@ -367,24 +370,54 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
// }
// m!(S);
// ```
fn lower_expr_within_pat(&mut self, expr: &Expr, allow_paths: bool) -> &'hir hir::Expr<'hir> {
match &expr.kind {
ExprKind::Lit(..)
| ExprKind::ConstBlock(..)
| ExprKind::IncludedBytes(..)
| ExprKind::Err(_)
| ExprKind::Dummy => {}
ExprKind::Path(..) if allow_paths => {}
ExprKind::Unary(UnOp::Neg, inner) if matches!(inner.kind, ExprKind::Lit(_)) => {}
fn lower_expr_within_pat(
&mut self,
expr: &Expr,
allow_paths: bool,
) -> &'hir hir::PatExpr<'hir> {
let err = |guar| hir::PatExprKind::Lit {
lit: self.arena.alloc(respan(self.lower_span(expr.span), LitKind::Err(guar))),
negated: false,
};
let kind = match &expr.kind {
ExprKind::Lit(lit) => {
hir::PatExprKind::Lit { lit: self.lower_lit(lit, expr.span), negated: false }
}
ExprKind::ConstBlock(c) => hir::PatExprKind::ConstBlock(self.lower_const_block(c)),
ExprKind::IncludedBytes(bytes) => hir::PatExprKind::Lit {
lit: self.arena.alloc(respan(
self.lower_span(expr.span),
LitKind::ByteStr(Arc::clone(bytes), StrStyle::Cooked),
)),
negated: false,
},
ExprKind::Err(guar) => err(*guar),
ExprKind::Dummy => span_bug!(expr.span, "lowered ExprKind::Dummy"),
ExprKind::Path(qself, path) if allow_paths => hir::PatExprKind::Path(self.lower_qpath(
expr.id,
qself,
path,
ParamMode::Optional,
AllowReturnTypeNotation::No,
ImplTraitContext::Disallowed(ImplTraitPosition::Path),
None,
)),
ExprKind::Unary(UnOp::Neg, inner) if let ExprKind::Lit(lit) = &inner.kind => {
hir::PatExprKind::Lit { lit: self.lower_lit(lit, expr.span), negated: true }
}
_ => {
let pattern_from_macro = expr.is_approximately_pattern();
let guar = self.dcx().emit_err(ArbitraryExpressionInPattern {
span: expr.span,
pattern_from_macro_note: pattern_from_macro,
});
return self.arena.alloc(self.expr_err(expr.span, guar));
err(guar)
}
}
self.lower_expr(expr)
};
self.arena.alloc(hir::PatExpr {
hir_id: self.lower_node_id(expr.id),
span: expr.span,
kind,
})
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_ast_pretty/src/pprust/state.rs
Original file line number Diff line number Diff line change
@@ -1701,7 +1701,7 @@ impl<'a> State<'a> {
self.print_pat(inner);
}
}
PatKind::Lit(e) => self.print_expr(e, FixupContext::default()),
PatKind::Expr(e) => self.print_expr(e, FixupContext::default()),
PatKind::Range(begin, end, Spanned { node: end_kind, .. }) => {
if let Some(e) = begin {
self.print_expr(e, FixupContext::default());
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
@@ -522,7 +522,7 @@ impl MacResult for MacEager {
return Some(P(ast::Pat {
id: ast::DUMMY_NODE_ID,
span: e.span,
kind: PatKind::Lit(e),
kind: PatKind::Expr(e),
tokens: None,
}));
}
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/build.rs
Original file line number Diff line number Diff line change
@@ -486,7 +486,7 @@ impl<'a> ExtCtxt<'a> {
self.pat(span, PatKind::Wild)
}
pub fn pat_lit(&self, span: Span, expr: P<ast::Expr>) -> P<ast::Pat> {
self.pat(span, PatKind::Lit(expr))
self.pat(span, PatKind::Expr(expr))
}
pub fn pat_ident(&self, span: Span, ident: Ident) -> P<ast::Pat> {
self.pat_ident_binding_mode(span, ident, ast::BindingMode::NONE)
35 changes: 30 additions & 5 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
@@ -1386,7 +1386,7 @@ impl<'hir> Pat<'hir> {

use PatKind::*;
match self.kind {
Wild | Never | Lit(_) | Range(..) | Binding(.., None) | Path(_) | Err(_) => true,
Wild | Never | Expr(_) | Range(..) | Binding(.., None) | Path(_) | Err(_) => true,
Box(s) | Deref(s) | Ref(s, _) | Binding(.., Some(s)) | Guard(s, _) => s.walk_short_(it),
Struct(_, fields, _) => fields.iter().all(|field| field.pat.walk_short_(it)),
TupleStruct(_, s, _) | Tuple(s, _) | Or(s) => s.iter().all(|p| p.walk_short_(it)),
@@ -1413,7 +1413,7 @@ impl<'hir> Pat<'hir> {

use PatKind::*;
match self.kind {
Wild | Never | Lit(_) | Range(..) | Binding(.., None) | Path(_) | Err(_) => {}
Wild | Never | Expr(_) | Range(..) | Binding(.., None) | Path(_) | Err(_) => {}
Box(s) | Deref(s) | Ref(s, _) | Binding(.., Some(s)) | Guard(s, _) => s.walk_(it),
Struct(_, fields, _) => fields.iter().for_each(|field| field.pat.walk_(it)),
TupleStruct(_, s, _) | Tuple(s, _) | Or(s) => s.iter().for_each(|p| p.walk_(it)),
@@ -1519,6 +1519,26 @@ impl fmt::Debug for DotDotPos {
}
}

#[derive(Debug, Clone, Copy, HashStable_Generic)]
pub struct PatExpr<'hir> {
pub hir_id: HirId,
pub span: Span,
pub kind: PatExprKind<'hir>,
}

#[derive(Debug, Clone, Copy, HashStable_Generic)]
pub enum PatExprKind<'hir> {
Lit {
lit: &'hir Lit,
// FIXME: move this into `Lit` and handle negated literal expressions
// once instead of matching on unop neg expressions everywhere.
negated: bool,
},
ConstBlock(ConstBlock),
/// A path pattern for a unit struct/variant or a (maybe-associated) constant.
Path(QPath<'hir>),
}

#[derive(Debug, Clone, Copy, HashStable_Generic)]
pub enum PatKind<'hir> {
/// Represents a wildcard pattern (i.e., `_`).
@@ -1563,14 +1583,14 @@ pub enum PatKind<'hir> {
/// A reference pattern (e.g., `&mut (a, b)`).
Ref(&'hir Pat<'hir>, Mutability),

/// A literal.
Lit(&'hir Expr<'hir>),
/// A literal, const block or path.
Expr(&'hir PatExpr<'hir>),

/// A guard pattern (e.g., `x if guard(x)`).
Guard(&'hir Pat<'hir>, &'hir Expr<'hir>),

/// A range pattern (e.g., `1..=2` or `1..2`).
Range(Option<&'hir Expr<'hir>>, Option<&'hir Expr<'hir>>, RangeEnd),
Range(Option<&'hir PatExpr<'hir>>, Option<&'hir PatExpr<'hir>>, RangeEnd),

/// A slice pattern, `[before_0, ..., before_n, (slice, after_0, ..., after_n)?]`.
///
@@ -4144,6 +4164,10 @@ pub enum Node<'hir> {
OpaqueTy(&'hir OpaqueTy<'hir>),
Pat(&'hir Pat<'hir>),
PatField(&'hir PatField<'hir>),
/// Needed as its own node with its own HirId for tracking
/// the unadjusted type of literals within patterns
/// (e.g. byte str literals not being of slice type).
PatExpr(&'hir PatExpr<'hir>),
Arm(&'hir Arm<'hir>),
Block(&'hir Block<'hir>),
LetStmt(&'hir LetStmt<'hir>),
@@ -4200,6 +4224,7 @@ impl<'hir> Node<'hir> {
| Node::Block(..)
| Node::Ctor(..)
| Node::Pat(..)
| Node::PatExpr(..)
| Node::Arm(..)
| Node::LetStmt(..)
| Node::Crate(..)
18 changes: 15 additions & 3 deletions compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
@@ -342,6 +342,9 @@ pub trait Visitor<'v>: Sized {
fn visit_pat_field(&mut self, f: &'v PatField<'v>) -> Self::Result {
walk_pat_field(self, f)
}
fn visit_pat_expr(&mut self, expr: &'v PatExpr<'v>) -> Self::Result {
walk_pat_expr(self, expr)
}
fn visit_anon_const(&mut self, c: &'v AnonConst) -> Self::Result {
walk_anon_const(self, c)
}
@@ -685,10 +688,10 @@ pub fn walk_pat<'v, V: Visitor<'v>>(visitor: &mut V, pattern: &'v Pat<'v>) -> V:
try_visit!(visitor.visit_ident(ident));
visit_opt!(visitor, visit_pat, optional_subpattern);
}
PatKind::Lit(ref expression) => try_visit!(visitor.visit_expr(expression)),
PatKind::Expr(ref expression) => try_visit!(visitor.visit_pat_expr(expression)),
PatKind::Range(ref lower_bound, ref upper_bound, _) => {
visit_opt!(visitor, visit_expr, lower_bound);
visit_opt!(visitor, visit_expr, upper_bound);
visit_opt!(visitor, visit_pat_expr, lower_bound);
visit_opt!(visitor, visit_pat_expr, upper_bound);
}
PatKind::Never | PatKind::Wild | PatKind::Err(_) => (),
PatKind::Slice(prepatterns, ref slice_pattern, postpatterns) => {
@@ -710,6 +713,15 @@ pub fn walk_pat_field<'v, V: Visitor<'v>>(visitor: &mut V, field: &'v PatField<'
visitor.visit_pat(field.pat)
}

pub fn walk_pat_expr<'v, V: Visitor<'v>>(visitor: &mut V, expr: &'v PatExpr<'v>) -> V::Result {
try_visit!(visitor.visit_id(expr.hir_id));
match &expr.kind {
PatExprKind::Lit { .. } => V::Result::output(),
PatExprKind::ConstBlock(c) => visitor.visit_inline_const(c),
PatExprKind::Path(qpath) => visitor.visit_qpath(qpath, expr.hir_id, expr.span),
}
}

pub fn walk_anon_const<'v, V: Visitor<'v>>(visitor: &mut V, constant: &'v AnonConst) -> V::Result {
try_visit!(visitor.visit_id(constant.hir_id));
visitor.visit_nested_body(constant.body)
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/region.rs
Original file line number Diff line number Diff line change
@@ -704,7 +704,7 @@ fn resolve_local<'tcx>(
| PatKind::Wild
| PatKind::Never
| PatKind::Path(_)
| PatKind::Lit(_)
| PatKind::Expr(_)
| PatKind::Range(_, _, _)
| PatKind::Err(_) => false,
}
Loading