Skip to content

Commit

Permalink
RFC 558: Require parentheses for chained comparisons
Browse files Browse the repository at this point in the history
Fixes #20724.
  • Loading branch information
dgrunwald committed Jan 8, 2015
1 parent 9e4e524 commit 1cc69c4
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 4 deletions.
10 changes: 8 additions & 2 deletions src/libsyntax/ast_util.rs
Expand Up @@ -85,6 +85,13 @@ pub fn is_shift_binop(b: BinOp) -> bool {
}
}

pub fn is_comparison_binop(b: BinOp) -> bool {
match b {
BiEq | BiLt | BiLe | BiNe | BiGt | BiGe => true,
_ => false
}
}

/// Returns `true` if the binary operator takes its arguments by value
pub fn is_by_value_binop(b: BinOp) -> bool {
match b {
Expand Down Expand Up @@ -317,8 +324,7 @@ pub fn operator_prec(op: ast::BinOp) -> uint {
BiBitAnd => 8u,
BiBitXor => 7u,
BiBitOr => 6u,
BiLt | BiLe | BiGe | BiGt => 4u,
BiEq | BiNe => 3u,
BiLt | BiLe | BiGe | BiGt | BiEq | BiNe => 3u,
BiAnd => 2u,
BiOr => 1u
}
Expand Down
24 changes: 23 additions & 1 deletion src/libsyntax/parse/parser.rs
Expand Up @@ -16,7 +16,7 @@ use ast::{AssociatedType, BareFnTy};
use ast::{RegionTyParamBound, TraitTyParamBound, TraitBoundModifier};
use ast::{ProvidedMethod, Public, Unsafety};
use ast::{Mod, BiAdd, Arg, Arm, Attribute, BindByRef, BindByValue};
use ast::{BiBitAnd, BiBitOr, BiBitXor, BiRem, Block};
use ast::{BiBitAnd, BiBitOr, BiBitXor, BiRem, BiLt, BiGt, Block};
use ast::{BlockCheckMode, CaptureByRef, CaptureByValue, CaptureClause};
use ast::{Crate, CrateConfig, Decl, DeclItem};
use ast::{DeclLocal, DefaultBlock, UnDeref, BiDiv, EMPTY_CTXT, EnumDef, ExplicitSelf};
Expand Down Expand Up @@ -2906,6 +2906,9 @@ impl<'a> Parser<'a> {
let cur_opt = self.token.to_binop();
match cur_opt {
Some(cur_op) => {
if ast_util::is_comparison_binop(cur_op) {
self.check_no_chained_comparison(&*lhs, cur_op)
}
let cur_prec = operator_prec(cur_op);
if cur_prec > min_prec {
self.bump();
Expand Down Expand Up @@ -2934,6 +2937,25 @@ impl<'a> Parser<'a> {
}
}

/// Produce an error if comparison operators are chained (RFC #558).
/// We only need to check lhs, not rhs, because all comparison ops
/// have same precedence and are left-associative
fn check_no_chained_comparison(&mut self, lhs: &Expr, outer_op: ast::BinOp) {
debug_assert!(ast_util::is_comparison_binop(outer_op));
match lhs.node {
ExprBinary(op, _, _) if ast_util::is_comparison_binop(op) => {
let op_span = self.span;
self.span_err(op_span,
"Chained comparison operators require parentheses");
if op == BiLt && outer_op == BiGt {
self.span_help(op_span,
"Use ::< instead of < if you meant to specify type arguments.");
}
}
_ => {}
}
}

/// Parse an assignment expression....
/// actually, this seems to be the main entry point for
/// parsing an arbitrary expression.
Expand Down
23 changes: 23 additions & 0 deletions src/test/compile-fail/require-parens-for-chained-comparison.rs
@@ -0,0 +1,23 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

fn f<T>() {}

fn main() {
false == false == false;
//~^ ERROR: Chained comparison operators require parentheses

false == 0 < 2;
//~^ ERROR: Chained comparison operators require parentheses

f<X>();
//~^ ERROR: Chained comparison operators require parentheses
//~^^ HELP: Use ::< instead of < if you meant to specify type arguments.
}
5 changes: 4 additions & 1 deletion src/test/compile-fail/unsized2.rs
Expand Up @@ -13,5 +13,8 @@
fn f<X>() {}

pub fn main() {
f<type>(); //~ ERROR expected identifier, found keyword `type`
f<type>();
//~^ ERROR expected identifier, found keyword `type`
//~^^ ERROR: Chained comparison operators require parentheses
//~^^^ HELP: Use ::< instead of < if you meant to specify type arguments.
}

0 comments on commit 1cc69c4

Please sign in to comment.