Skip to content

Commit

Permalink
let_chains: Change AST validation strategy slightly.
Browse files Browse the repository at this point in the history
  • Loading branch information
Centril committed May 18, 2019
1 parent 2178db4 commit f607ff1
Showing 1 changed file with 45 additions and 34 deletions.
79 changes: 45 additions & 34 deletions src/librustc_passes/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,10 @@ impl<'a> AstValidator<'a> {
with(self, outer, |this| &mut this.outer_impl_trait, f)
}

fn with_let_allowed(&mut self, v: bool, f: impl FnOnce(&mut Self)) {
with(self, v, |this| &mut this.is_let_allowed, f)
fn with_let_allowed(&mut self, v: bool, f: impl FnOnce(&mut Self, bool)) {
let old = mem::replace(&mut self.is_let_allowed, v);
f(self, old);
self.is_let_allowed = old;
}

fn visit_assoc_type_binding_from_generic_args(&mut self, type_binding: &'a TypeBinding) {
Expand Down Expand Up @@ -338,34 +340,45 @@ impl<'a> AstValidator<'a> {
/// Visits the `expr` and adjusts whether `let $pat = $expr` is allowed in decendants.
/// Returns whether we walked into `expr` or not.
/// If we did, walking should not happen again.
fn visit_expr_with_let_maybe_allowed(&mut self, expr: &'a Expr) -> bool {
fn visit_expr_with_let_maybe_allowed(&mut self, expr: &'a Expr, let_allowed: bool) -> bool {
match &expr.node {
// Assuming the context permits, `($expr)` does not impose additional constraints.
ExprKind::Paren(_) => visit::walk_expr(self, expr),
ExprKind::Paren(_) => {
self.with_let_allowed(let_allowed, |this, _| visit::walk_expr(this, expr));
}
// Assuming the context permits,
// l && r` allows decendants in `l` and `r` to be `let` expressions.
ExprKind::Binary(op, ..) if op.node == BinOpKind::And => visit::walk_expr(self, expr),
ExprKind::Binary(op, ..) if op.node == BinOpKind::And => {
self.with_let_allowed(let_allowed, |this, _| visit::walk_expr(this, expr));
}
// However, we do allow it in the condition of the `if` expression.
// We do not allow `let` in `then` and `opt_else` directly.
ExprKind::If(ref cond, ref then, ref opt_else) => {
self.with_let_allowed(false, |this| {
this.visit_block(then);
walk_list!(this, visit_expr, opt_else);
});
self.with_let_allowed(true, |this| this.visit_expr(cond));
ExprKind::If(cond, then, opt_else) => {
self.visit_block(then);
walk_list!(self, visit_expr, opt_else);
self.with_let_allowed(true, |this, _| this.visit_expr(cond));
}
// The same logic applies to `While`.
ExprKind::While(ref cond, ref then, ref opt_label) => {
ExprKind::While(cond, then, opt_label) => {
walk_list!(self, visit_label, opt_label);
self.with_let_allowed(false, |this| this.visit_block(then));
self.with_let_allowed(true, |this| this.visit_expr(cond));
self.visit_block(then);
self.with_let_allowed(true, |this, _| this.visit_expr(cond));
}
// Don't walk into `expr` and defer further checks to the caller.
_ => return false,
}

true
}

/// Emits an error banning the `let` expression provided.
fn ban_let_expr(&self, expr: &'a Expr) {
self.err_handler()
.struct_span_err(expr.span, "`let` expressions are not supported here")
.note("only supported directly in conditions of `if`- and `while`-expressions")
.note("as well as when nested within `&&` and parenthesis in those conditions")
.emit();
}
}

enum GenericPosition {
Expand Down Expand Up @@ -470,28 +483,26 @@ fn validate_generics_order<'a>(

impl<'a> Visitor<'a> for AstValidator<'a> {
fn visit_expr(&mut self, expr: &'a Expr) {
match expr.node {
ExprKind::Let(_, _) if !self.is_let_allowed => {
self.err_handler()
.struct_span_err(expr.span, "`let` expressions are not supported here")
.note("only supported directly in conditions of `if`- and `while`-expressions")
.note("as well as when nested within `&&` and parenthesis in those conditions")
.emit();
}
_ if self.visit_expr_with_let_maybe_allowed(&expr) => {
// Prevent `walk_expr` to happen since we've already done that.
return;
}
ExprKind::InlineAsm(..) if !self.session.target.target.options.allow_asm => {
span_err!(self.session, expr.span, E0472, "asm! is unsupported on this target");
}
ExprKind::ObsoleteInPlace(ref place, ref val) => {
self.obsolete_in_place(expr, place, val);
self.with_let_allowed(false, |this, let_allowed| {
match &expr.node {
ExprKind::Let(_, _) if !let_allowed => {
this.ban_let_expr(expr);
}
_ if this.visit_expr_with_let_maybe_allowed(&expr, let_allowed) => {
// Prevent `walk_expr` to happen since we've already done that.
return;
}
ExprKind::InlineAsm(..) if !this.session.target.target.options.allow_asm => {
span_err!(this.session, expr.span, E0472, "asm! is unsupported on this target");
}
ExprKind::ObsoleteInPlace(place, val) => {
this.obsolete_in_place(expr, place, val);
}
_ => {}
}
_ => {}
}

self.with_let_allowed(false, |this| visit::walk_expr(this, expr));
visit::walk_expr(this, expr);
});
}

fn visit_ty(&mut self, ty: &'a Ty) {
Expand Down

0 comments on commit f607ff1

Please sign in to comment.