Skip to content
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

Exhaustively handle expressions in patterns #134228

Merged
merged 4 commits into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
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
Expand Up @@ -623,7 +623,7 @@ impl Pat {
PatKind::Wild
| PatKind::Rest
| PatKind::Never
| PatKind::Lit(_)
| PatKind::Expr(_)
| PatKind::Range(..)
| PatKind::Ident(..)
| PatKind::Path(..)
Expand Down Expand Up @@ -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>),
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
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>),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
51 changes: 28 additions & 23 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_ast_lowering/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
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::{
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}));
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_expand/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
35 changes: 30 additions & 5 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand All @@ -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)),
Expand Down Expand Up @@ -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., `_`).
Expand Down Expand Up @@ -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)?]`.
///
Expand Down Expand Up @@ -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>),
Expand Down Expand Up @@ -4200,6 +4224,7 @@ impl<'hir> Node<'hir> {
| Node::Block(..)
| Node::Ctor(..)
| Node::Pat(..)
| Node::PatExpr(..)
| Node::Arm(..)
| Node::LetStmt(..)
| Node::Crate(..)
Expand Down
18 changes: 15 additions & 3 deletions compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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) => {
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,7 @@ fn resolve_local<'tcx>(
| PatKind::Wild
| PatKind::Never
| PatKind::Path(_)
| PatKind::Lit(_)
| PatKind::Expr(_)
| PatKind::Range(_, _, _)
| PatKind::Err(_) => false,
}
Expand Down
Loading
Loading