Skip to content

Commit

Permalink
Move PatKind::Lit checking from ast_validation to ast lowering
Browse files Browse the repository at this point in the history
Fixes #92074

This allows us to insert an `ExprKind::Err` when an invalid expression
is used in a literal pattern, preventing later stages of compilation
from seeing an unexpected literal pattern.
  • Loading branch information
Aaron1011 committed Jan 1, 2022
1 parent 4f49627 commit 137c374
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 54 deletions.
37 changes: 34 additions & 3 deletions compiler/rustc_ast_lowering/src/pat.rs
Expand Up @@ -24,7 +24,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let lower_sub = |this: &mut Self| sub.as_ref().map(|s| this.lower_pat(&*s));
break self.lower_pat_ident(pattern, binding_mode, ident, lower_sub);
}
PatKind::Lit(ref e) => break hir::PatKind::Lit(self.lower_expr(e)),
PatKind::Lit(ref e) => {
break hir::PatKind::Lit(self.lower_expr_within_pat(e, false));
}
PatKind::TupleStruct(ref qself, ref path, ref pats) => {
let qpath = self.lower_qpath(
pattern.id,
Expand Down Expand Up @@ -81,8 +83,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}
PatKind::Range(ref e1, ref e2, Spanned { node: ref end, .. }) => {
break hir::PatKind::Range(
e1.as_deref().map(|e| self.lower_expr(e)),
e2.as_deref().map(|e| self.lower_expr(e)),
e1.as_deref().map(|e| self.lower_expr_within_pat(e, true)),
e2.as_deref().map(|e| self.lower_expr_within_pat(e, true)),
self.lower_range_end(end, e2.is_some()),
);
}
Expand Down Expand Up @@ -314,4 +316,33 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
RangeEnd::Excluded | RangeEnd::Included(_) => hir::RangeEnd::Included,
}
}

/// Matches `'-' lit | lit (cf. parser::Parser::parse_literal_maybe_minus)`,
/// or paths for ranges.
//
// FIXME: do we want to allow `expr -> pattern` conversion to create path expressions?
// That means making this work:
//
// ```rust,ignore (FIXME)
// struct S;
// macro_rules! m {
// ($a:expr) => {
// let $a = S;
// }
// }
// 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::Err => {}
ExprKind::Path(..) if allow_paths => {}
ExprKind::Unary(UnOp::Neg, ref inner) if matches!(inner.kind, ExprKind::Lit(_)) => {}
_ => {
self.diagnostic()
.span_err(expr.span, "arbitrary expressions aren't allowed in patterns");
return self.arena.alloc(self.expr_err(expr.span));
}
}
self.lower_expr(expr)
}
}
47 changes: 0 additions & 47 deletions compiler/rustc_ast_passes/src/ast_validation.rs
Expand Up @@ -302,34 +302,6 @@ impl<'a> AstValidator<'a> {
}
}

/// Matches `'-' lit | lit (cf. parser::Parser::parse_literal_maybe_minus)`,
/// or paths for ranges.
//
// FIXME: do we want to allow `expr -> pattern` conversion to create path expressions?
// That means making this work:
//
// ```rust,ignore (FIXME)
// struct S;
// macro_rules! m {
// ($a:expr) => {
// let $a = S;
// }
// }
// m!(S);
// ```
fn check_expr_within_pat(&self, expr: &Expr, allow_paths: bool) {
match expr.kind {
ExprKind::Lit(..) | ExprKind::ConstBlock(..) | ExprKind::Err => {}
ExprKind::Path(..) if allow_paths => {}
ExprKind::Unary(UnOp::Neg, ref inner) if matches!(inner.kind, ExprKind::Lit(_)) => {}
_ => self.err_handler().span_err(
expr.span,
"arbitrary expressions aren't allowed \
in patterns",
),
}
}

fn check_late_bound_lifetime_defs(&self, params: &[GenericParam]) {
// Check only lifetime parameters are present and that the lifetime
// parameters that are present have no bounds.
Expand Down Expand Up @@ -1426,25 +1398,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
visit::walk_param_bound(self, bound)
}

fn visit_pat(&mut self, pat: &'a Pat) {
match &pat.kind {
PatKind::Lit(expr) => {
self.check_expr_within_pat(expr, false);
}
PatKind::Range(start, end, _) => {
if let Some(expr) = start {
self.check_expr_within_pat(expr, true);
}
if let Some(expr) = end {
self.check_expr_within_pat(expr, true);
}
}
_ => {}
}

visit::walk_pat(self, pat)
}

fn visit_poly_trait_ref(&mut self, t: &'a PolyTraitRef, m: &'a TraitBoundModifier) {
self.check_late_bound_lifetime_defs(&t.bound_generic_params);
visit::walk_poly_trait_ref(self, t, m);
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/match/expr_before_ident_pat.stderr
@@ -1,14 +1,14 @@
error: arbitrary expressions aren't allowed in patterns
error[E0425]: cannot find value `a` in this scope
--> $DIR/expr_before_ident_pat.rs:12:12
|
LL | funny!(a, a);
| ^
| ^ not found in this scope

error[E0425]: cannot find value `a` in this scope
error: arbitrary expressions aren't allowed in patterns
--> $DIR/expr_before_ident_pat.rs:12:12
|
LL | funny!(a, a);
| ^ not found in this scope
| ^

error: aborting due to 2 previous errors

Expand Down
36 changes: 36 additions & 0 deletions src/test/ui/pattern/issue-92074-macro-ice.rs
@@ -0,0 +1,36 @@
pub enum En {
A(Vec<u8>)
}

fn get_usize() -> usize {
0
}

macro_rules! force_expr {
($e:expr) => { $e }
}

macro_rules! force_pat {
($a:expr, $b:expr) => { $a..=$b }
}

macro_rules! make_vec {
() => { force_expr!(Vec::new()) } //~ ERROR arbitrary expressions aren't allowed
}

macro_rules! make_pat {
() => { force_pat!(get_usize(), get_usize()) }
//~^ ERROR arbitrary expressions aren't allowed
//~| ERROR arbitrary expressions aren't allowed
}

#[allow(unreachable_code)]
fn f() -> Result<(), impl core::fmt::Debug> {
let x: En = loop {};

assert!(matches!(x, En::A(make_vec!())));
assert!(matches!(5, make_pat!()));
Ok::<(), &'static str>(())
}

fn main() {}
35 changes: 35 additions & 0 deletions src/test/ui/pattern/issue-92074-macro-ice.stderr
@@ -0,0 +1,35 @@
error: arbitrary expressions aren't allowed in patterns
--> $DIR/issue-92074-macro-ice.rs:18:25
|
LL | () => { force_expr!(Vec::new()) }
| ^^^^^^^^^^
...
LL | assert!(matches!(x, En::A(make_vec!())));
| ----------- in this macro invocation
|
= note: this error originates in the macro `make_vec` (in Nightly builds, run with -Z macro-backtrace for more info)

error: arbitrary expressions aren't allowed in patterns
--> $DIR/issue-92074-macro-ice.rs:22:24
|
LL | () => { force_pat!(get_usize(), get_usize()) }
| ^^^^^^^^^^^
...
LL | assert!(matches!(5, make_pat!()));
| ----------- in this macro invocation
|
= note: this error originates in the macro `make_pat` (in Nightly builds, run with -Z macro-backtrace for more info)

error: arbitrary expressions aren't allowed in patterns
--> $DIR/issue-92074-macro-ice.rs:22:37
|
LL | () => { force_pat!(get_usize(), get_usize()) }
| ^^^^^^^^^^^
...
LL | assert!(matches!(5, make_pat!()));
| ----------- in this macro invocation
|
= note: this error originates in the macro `make_pat` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 3 previous errors

0 comments on commit 137c374

Please sign in to comment.