Skip to content

Commit

Permalink
Moved let-chain warning/error to ast_validation and changed to uncond…
Browse files Browse the repository at this point in the history
…itional error.
  • Loading branch information
davidtwco committed Sep 2, 2018
1 parent 2ce56c5 commit 8c92cbe
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 96 deletions.
48 changes: 48 additions & 0 deletions src/librustc_passes/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use syntax::ast::*;
use syntax::attr;
use syntax::source_map::Spanned;
use syntax::symbol::keywords;
use syntax::ptr::P;
use syntax::visit::{self, Visitor};
use syntax_pos::Span;
use errors;
Expand Down Expand Up @@ -167,11 +168,58 @@ impl<'a> AstValidator<'a> {
"only lifetime parameters can be used in this context");
}
}

/// With eRFC 2497, we need to check whether an expression is ambigious and warn or error
/// depending on the edition, this function handles that.
fn while_if_let_ambiguity(&self, expr: &P<Expr>) {
if let Some((span, op_kind)) = self.while_if_let_expr_ambiguity(&expr) {
let mut err = self.err_handler().struct_span_err(
span, &format!("ambigious use of `{}`", op_kind.to_string())
);

err.note(
"this will be a error until the `let_chains` feature is stabilized"
);

if let Ok(snippet) = self.session.source_map().span_to_snippet(span) {
err.span_suggestion(
span, "consider adding parentheses", format!("({})", snippet),
);
}

err.emit();
}
}

/// With eRFC 2497 adding if-let chains, there is a requirement that the parsing of
/// `&&` and `||` in a if-let statement be unambigious. This function returns a span and
/// a `BinOpKind` (either `&&` or `||` depending on what was ambigious) if it is determined
/// that the current expression parsed is ambigious and will break in future.
fn while_if_let_expr_ambiguity(&self, expr: &P<Expr>) -> Option<(Span, BinOpKind)> {
debug!("while_if_let_expr_ambiguity: expr.node: {:?}", expr.node);
match &expr.node {
ExprKind::Binary(op, _, _) if op.node == BinOpKind::And || op.node == BinOpKind::Or => {
Some((expr.span, op.node))
},
ExprKind::Range(ref lhs, ref rhs, _) => {
let lhs_ambigious = lhs.as_ref()
.and_then(|lhs| self.while_if_let_expr_ambiguity(lhs));
let rhs_ambigious = rhs.as_ref()
.and_then(|rhs| self.while_if_let_expr_ambiguity(rhs));

lhs_ambigious.or(rhs_ambigious)
}
_ => None,
}
}

}

impl<'a> Visitor<'a> for AstValidator<'a> {
fn visit_expr(&mut self, expr: &'a Expr) {
match expr.node {
ExprKind::IfLet(_, ref expr, _, _) | ExprKind::WhileLet(_, ref expr, _, _) =>
self.while_if_let_ambiguity(&expr),
ExprKind::InlineAsm(..) if !self.session.target.target.options.allow_asm => {
span_err!(self.session, expr.span, E0472, "asm! is unsupported on this target");
}
Expand Down
53 changes: 0 additions & 53 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3327,8 +3327,6 @@ impl<'a> Parser<'a> {
let pats = self.parse_pats()?;
self.expect(&token::Eq)?;
let expr = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?;
self.while_if_let_ambiguity(&expr);

let thn = self.parse_block()?;
let (hi, els) = if self.eat_keyword(keywords::Else) {
let expr = self.parse_else_expr()?;
Expand All @@ -3339,56 +3337,6 @@ impl<'a> Parser<'a> {
Ok(self.mk_expr(lo.to(hi), ExprKind::IfLet(pats, expr, thn, els), attrs))
}

/// With eRFC 2497, we need to check whether an expression is ambigious and warn or error
/// depending on the edition, this function handles that.
fn while_if_let_ambiguity(&self, expr: &P<Expr>) {
if let Some((span, op_kind)) = self.while_if_let_expr_ambiguity(&expr) {
let message = format!("ambigious use of `{}`", op_kind.to_string());
let mut err = if self.span.edition() >= Edition::Edition2018 {
self.diagnostic().struct_span_err(span, &message)
} else {
self.diagnostic().struct_span_warn(span, &message)
};

let note = if self.span.edition() >= Edition::Edition2018 {
"This will be a error until the `let_chains` feature is stabilized."
} else {
"This will be a error in Rust 2018 until the `let_chains` feature is stabilized."
};
err.note(note);

if let Ok(snippet) = self.sess.source_map().span_to_snippet(span) {
err.span_suggestion(
span, "consider adding parenthesis", format!("({})", snippet),
);
}

err.emit();
}
}

/// With eRFC 2497 adding if-let chains, there is a requirement that the parsing of
/// `&&` and `||` in a if-let statement be unambigious. This function returns a span and
/// a `BinOpKind` (either `&&` or `||` depending on what was ambigious) if it is determined
/// that the current expression parsed is ambigious and will break in future.
fn while_if_let_expr_ambiguity(&self, expr: &P<Expr>) -> Option<(Span, BinOpKind)> {
debug!("while_if_let_expr_ambiguity: expr.node: {:?}", expr.node);
match &expr.node {
ExprKind::Binary(op, _, _) if op.node == BinOpKind::And || op.node == BinOpKind::Or => {
Some((expr.span, op.node))
},
ExprKind::Range(ref lhs, ref rhs, _) => {
let lhs_ambigious = lhs.as_ref()
.and_then(|lhs| self.while_if_let_expr_ambiguity(lhs));
let rhs_ambigious = rhs.as_ref()
.and_then(|rhs| self.while_if_let_expr_ambiguity(rhs));

lhs_ambigious.or(rhs_ambigious)
}
_ => None,
}
}

// `move |args| expr`
fn parse_lambda_expr(&mut self,
attrs: ThinVec<Attribute>)
Expand Down Expand Up @@ -3489,7 +3437,6 @@ impl<'a> Parser<'a> {
let pats = self.parse_pats()?;
self.expect(&token::Eq)?;
let expr = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?;
self.while_if_let_ambiguity(&expr);
let (iattrs, body) = self.parse_inner_attrs_and_block()?;
attrs.extend(iattrs);
let span = span_lo.to(body.span);
Expand Down
13 changes: 6 additions & 7 deletions src/test/ui/rfc-2497-if-let-chains/syntax-ambiguity-2015.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
// except according to those terms.

// edition:2015
// compile-pass

// Enabling `ireffutable_let_patterns` isn't necessary for what this tests, but it makes coming up
// with examples easier.
Expand All @@ -20,22 +19,22 @@ fn main() {
use std::ops::Range;

if let Range { start: _, end: _ } = true..true && false { }
//~^ WARN error in 2018
//~^ ERROR ambigious use of `&&`

if let Range { start: _, end: _ } = true..true || false { }
//~^ WARN error in 2018
//~^ ERROR ambigious use of `||`

while let Range { start: _, end: _ } = true..true && false { }
//~^ WARN error in 2018
//~^ ERROR ambigious use of `&&`

while let Range { start: _, end: _ } = true..true || false { }
//~^ WARN error in 2018
//~^ ERROR ambigious use of `||`

if let true = false && false { }
//~^ WARN error in 2018
//~^ ERROR ambigious use of `&&`

while let true = (1 == 2) && false { }
//~^ WARN error in 2018
//~^ ERROR ambigious use of `&&`

// The following cases are not an error as parenthesis are used to
// clarify intent:
Expand Down
50 changes: 26 additions & 24 deletions src/test/ui/rfc-2497-if-let-chains/syntax-ambiguity-2015.stderr
Original file line number Diff line number Diff line change
@@ -1,48 +1,50 @@
warning: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2015.rs:22:47
error: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2015.rs:21:47
|
LL | if let Range { start: _, end: _ } = true..true && false { }
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true && false)`
| ^^^^^^^^^^^^^ help: consider adding parentheses: `(true && false)`
|
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.
= note: this will be a error until the `let_chains` feature is stabilized

warning: ambigious use of `||`
--> $DIR/syntax-ambiguity-2015.rs:25:47
error: ambigious use of `||`
--> $DIR/syntax-ambiguity-2015.rs:24:47
|
LL | if let Range { start: _, end: _ } = true..true || false { }
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true || false)`
| ^^^^^^^^^^^^^ help: consider adding parentheses: `(true || false)`
|
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.
= note: this will be a error until the `let_chains` feature is stabilized

warning: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2015.rs:28:50
error: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2015.rs:27:50
|
LL | while let Range { start: _, end: _ } = true..true && false { }
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true && false)`
| ^^^^^^^^^^^^^ help: consider adding parentheses: `(true && false)`
|
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.
= note: this will be a error until the `let_chains` feature is stabilized

warning: ambigious use of `||`
--> $DIR/syntax-ambiguity-2015.rs:31:50
error: ambigious use of `||`
--> $DIR/syntax-ambiguity-2015.rs:30:50
|
LL | while let Range { start: _, end: _ } = true..true || false { }
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true || false)`
| ^^^^^^^^^^^^^ help: consider adding parentheses: `(true || false)`
|
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.
= note: this will be a error until the `let_chains` feature is stabilized

warning: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2015.rs:34:19
error: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2015.rs:33:19
|
LL | if let true = false && false { }
| ^^^^^^^^^^^^^^ help: consider adding parenthesis: `(false && false)`
| ^^^^^^^^^^^^^^ help: consider adding parentheses: `(false && false)`
|
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.
= note: this will be a error until the `let_chains` feature is stabilized

warning: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2015.rs:37:22
error: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2015.rs:36:22
|
LL | while let true = (1 == 2) && false { }
| ^^^^^^^^^^^^^^^^^ help: consider adding parenthesis: `((1 == 2) && false)`
| ^^^^^^^^^^^^^^^^^ help: consider adding parentheses: `((1 == 2) && false)`
|
= note: This will be a error in Rust 2018 until the `let_chains` feature is stabilized.
= note: this will be a error until the `let_chains` feature is stabilized

error: aborting due to 6 previous errors

24 changes: 12 additions & 12 deletions src/test/ui/rfc-2497-if-let-chains/syntax-ambiguity-2018.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,49 +2,49 @@ error: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2018.rs:21:47
|
LL | if let Range { start: _, end: _ } = true..true && false { }
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true && false)`
| ^^^^^^^^^^^^^ help: consider adding parentheses: `(true && false)`
|
= note: This will be a error until the `let_chains` feature is stabilized.
= note: this will be a error until the `let_chains` feature is stabilized

error: ambigious use of `||`
--> $DIR/syntax-ambiguity-2018.rs:24:47
|
LL | if let Range { start: _, end: _ } = true..true || false { }
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true || false)`
| ^^^^^^^^^^^^^ help: consider adding parentheses: `(true || false)`
|
= note: This will be a error until the `let_chains` feature is stabilized.
= note: this will be a error until the `let_chains` feature is stabilized

error: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2018.rs:27:50
|
LL | while let Range { start: _, end: _ } = true..true && false { }
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true && false)`
| ^^^^^^^^^^^^^ help: consider adding parentheses: `(true && false)`
|
= note: This will be a error until the `let_chains` feature is stabilized.
= note: this will be a error until the `let_chains` feature is stabilized

error: ambigious use of `||`
--> $DIR/syntax-ambiguity-2018.rs:30:50
|
LL | while let Range { start: _, end: _ } = true..true || false { }
| ^^^^^^^^^^^^^ help: consider adding parenthesis: `(true || false)`
| ^^^^^^^^^^^^^ help: consider adding parentheses: `(true || false)`
|
= note: This will be a error until the `let_chains` feature is stabilized.
= note: this will be a error until the `let_chains` feature is stabilized

error: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2018.rs:33:19
|
LL | if let true = false && false { }
| ^^^^^^^^^^^^^^ help: consider adding parenthesis: `(false && false)`
| ^^^^^^^^^^^^^^ help: consider adding parentheses: `(false && false)`
|
= note: This will be a error until the `let_chains` feature is stabilized.
= note: this will be a error until the `let_chains` feature is stabilized

error: ambigious use of `&&`
--> $DIR/syntax-ambiguity-2018.rs:36:22
|
LL | while let true = (1 == 2) && false { }
| ^^^^^^^^^^^^^^^^^ help: consider adding parenthesis: `((1 == 2) && false)`
| ^^^^^^^^^^^^^^^^^ help: consider adding parentheses: `((1 == 2) && false)`
|
= note: This will be a error until the `let_chains` feature is stabilized.
= note: this will be a error until the `let_chains` feature is stabilized

error: aborting due to 6 previous errors

0 comments on commit 8c92cbe

Please sign in to comment.