From a12866270e348a8fa2ec4f12aa6cdb779122ef4a Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sun, 11 Jan 2015 17:37:07 +0100 Subject: [PATCH] Fix precedence of '..' range notation and some grammar inconsistencies. Fixes #20811 and #20241. Note: this changes the semantics of parse_more_binops() to parse binops with precedence >= min_prec. Previously, it would parse binops with precedence > min_prec. --- src/doc/reference.md | 6 +- src/libsyntax/ast_util.rs | 12 ++++ src/libsyntax/parse/parser.rs | 96 ++++++++++++++++---------- src/test/compile-fail/range-3.rs | 16 +++++ src/test/run-pass/ranges-precedence.rs | 8 +++ 5 files changed, 97 insertions(+), 41 deletions(-) create mode 100644 src/test/compile-fail/range-3.rs diff --git a/src/doc/reference.md b/src/doc/reference.md index 2486466c8696d..ac61060c2c23b 100644 --- a/src/doc/reference.md +++ b/src/doc/reference.md @@ -3134,15 +3134,15 @@ The precedence of Rust binary operators is ordered as follows, going from strong to weak: ```{.text .precedence} -* / % as +* / % + - << >> & ^ | -< > <= >= -== != +.. +== != < > <= >= && || = diff --git a/src/libsyntax/ast_util.rs b/src/libsyntax/ast_util.rs index bc7fbd46fd8ba..3e8b640c1cf55 100644 --- a/src/libsyntax/ast_util.rs +++ b/src/libsyntax/ast_util.rs @@ -321,6 +321,7 @@ pub fn struct_field_visibility(field: ast::StructField) -> Visibility { /// Maps a binary operator to its precedence pub fn operator_prec(op: ast::BinOp) -> uint { match op { + // prefix expressions sit here with 13 // 'as' sits here with 12 BiMul | BiDiv | BiRem => 11u, BiAdd | BiSub => 10u, @@ -328,17 +329,28 @@ pub fn operator_prec(op: ast::BinOp) -> uint { BiBitAnd => 8u, BiBitXor => 7u, BiBitOr => 6u, + // '..' sits here with 5 BiLt | BiLe | BiGe | BiGt | BiEq | BiNe => 3u, BiAnd => 2u, BiOr => 1u } } +/// Precedence of the prefix operators '!', '-', '&', '*', '~' etc. +/// Does not include the prefix form of '..' +#[allow(non_upper_case_globals)] +pub static prefix_prec: uint = 13u; + /// Precedence of the `as` operator, which is a binary operator /// not appearing in the prior table. #[allow(non_upper_case_globals)] pub static as_prec: uint = 12u; +// Precedence of '..', which exists as a binary operator, +// and as unary operators (prefix and postfix form) +#[allow(non_upper_case_globals)] +pub static range_prec: uint = 5u; + pub fn empty_generics() -> Generics { Generics { lifetimes: Vec::new(), diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 33f9e35d8b7af..0062b07dc1679 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -62,7 +62,7 @@ use ast::{ViewItem, ViewItem_, ViewItemExternCrate, ViewItemUse}; use ast::{ViewPath, ViewPathGlob, ViewPathList, ViewPathSimple}; use ast::{Visibility, WhereClause}; use ast; -use ast_util::{self, as_prec, ident_to_path, operator_prec}; +use ast_util::{self, prefix_prec, as_prec, range_prec, ident_to_path, operator_prec}; use codemap::{self, Span, BytePos, Spanned, spanned, mk_sp}; use diagnostic; use ext::tt::macro_parser; @@ -93,7 +93,6 @@ bitflags! { const RESTRICTION_STMT_EXPR = 0b0001, const RESTRICTION_NO_BAR_OP = 0b0010, const RESTRICTION_NO_STRUCT_LITERAL = 0b0100, - const RESTRICTION_NO_DOTS = 0b1000, } } @@ -2769,7 +2768,8 @@ impl<'a> Parser<'a> { } /// Parse a prefix-operator expr - pub fn parse_prefix_expr(&mut self) -> P { + /// only operators with a precedence >= min_prec will be accepted + pub fn parse_prefix_expr(&mut self, min_prec: uint) -> P { let lo = self.span.lo; let hi; @@ -2777,26 +2777,26 @@ impl<'a> Parser<'a> { match self.token { token::Not => { self.bump(); - let e = self.parse_prefix_expr(); + let e = self.parse_prefix_expr(prefix_prec); hi = e.span.hi; ex = self.mk_unary(UnNot, e); } token::BinOp(token::Minus) => { self.bump(); - let e = self.parse_prefix_expr(); + let e = self.parse_prefix_expr(prefix_prec); hi = e.span.hi; ex = self.mk_unary(UnNeg, e); } token::BinOp(token::Star) => { self.bump(); - let e = self.parse_prefix_expr(); + let e = self.parse_prefix_expr(prefix_prec); hi = e.span.hi; ex = self.mk_unary(UnDeref, e); } token::BinOp(token::And) | token::AndAnd => { self.expect_and(); let m = self.parse_mutability(); - let e = self.parse_prefix_expr(); + let e = self.parse_prefix_expr(prefix_prec); hi = e.span.hi; ex = ExprAddrOf(m, e); } @@ -2810,14 +2810,14 @@ impl<'a> Parser<'a> { _ => self.obsolete(last_span, ObsoleteSyntax::OwnedExpr) } - let e = self.parse_prefix_expr(); + let e = self.parse_prefix_expr(prefix_prec); hi = e.span.hi; ex = self.mk_unary(UnUniq, e); } - token::DotDot if !self.restrictions.contains(RESTRICTION_NO_DOTS) => { + token::DotDot if min_prec <= range_prec => { // A range, closed above: `..expr`. self.bump(); - let e = self.parse_expr(); + let e = self.parse_binops(range_prec + 1); hi = e.span.hi; ex = self.mk_range(None, Some(e)); } @@ -2848,7 +2848,7 @@ impl<'a> Parser<'a> { "perhaps you meant `box() (foo)` instead?"); self.abort_if_errors(); } - let subexpression = self.parse_prefix_expr(); + let subexpression = self.parse_prefix_expr(prefix_prec); hi = subexpression.span.hi; ex = ExprBox(Some(place), subexpression); return self.mk_expr(lo, hi, ex); @@ -2856,7 +2856,7 @@ impl<'a> Parser<'a> { } // Otherwise, we use the unique pointer default. - let subexpression = self.parse_prefix_expr(); + let subexpression = self.parse_prefix_expr(prefix_prec); hi = subexpression.span.hi; // FIXME (pnkfelix): After working out kinks with box // desugaring, should be `ExprBox(None, subexpression)` @@ -2868,10 +2868,10 @@ impl<'a> Parser<'a> { return self.mk_expr(lo, hi, ex); } - /// Parse an expression of binops - pub fn parse_binops(&mut self) -> P { - let prefix_expr = self.parse_prefix_expr(); - self.parse_more_binops(prefix_expr, 0) + /// Parse an expression of binops of at least min_prec precedence + pub fn parse_binops(&mut self, min_prec: uint) -> P { + let prefix_expr = self.parse_prefix_expr(min_prec); + self.parse_more_binops(prefix_expr, min_prec) } /// Parse an expression of binops of at least min_prec precedence @@ -2894,10 +2894,9 @@ impl<'a> Parser<'a> { self.check_no_chained_comparison(&*lhs, cur_op) } let cur_prec = operator_prec(cur_op); - if cur_prec > min_prec { + if cur_prec >= min_prec { self.bump(); - let expr = self.parse_prefix_expr(); - let rhs = self.parse_more_binops(expr, cur_prec); + let rhs = self.parse_binops(cur_prec + 1); let lhs_span = lhs.span; let rhs_span = rhs.span; let binary = self.mk_binary(cur_op, lhs, rhs); @@ -2908,12 +2907,29 @@ impl<'a> Parser<'a> { } } None => { - if as_prec > min_prec && self.eat_keyword(keywords::As) { + if as_prec >= min_prec && self.eat_keyword(keywords::As) { let rhs = self.parse_ty(); let _as = self.mk_expr(lhs.span.lo, rhs.span.hi, ExprCast(lhs, rhs)); self.parse_more_binops(_as, min_prec) + } else if range_prec >= min_prec + && match lhs.node { ExprRange(_, _) => false, _ => true } + && self.eat(&token::DotDot) { + // '..' range notation, infix or postfix form + // Note that we intentionally reject other range expressions on the lhs. + // This makes '..1..2' invalid. + // This is necessary for consistency between the prefix and postfix forms. + let opt_rhs = if self.is_at_start_of_range_notation_rhs() { + Some(self.parse_binops(range_prec + 1)) + } else { + None + }; + let lo = lhs.span.lo; + let hi = self.span.hi; + let range = self.mk_range(Some(lhs), opt_rhs); + let bin = self.mk_expr(lo, hi, range); + self.parse_more_binops(bin, min_prec) } else { lhs } @@ -2921,6 +2937,27 @@ impl<'a> Parser<'a> { } } + fn is_at_start_of_range_notation_rhs(&self) -> bool { + if self.token.can_begin_expr() { + // parse `for i in 1.. { }` as infinite loop, not as `for i in (1..{})`. + if self.token == token::OpenDelim(token::Brace) { + return !self.restrictions.contains(RESTRICTION_NO_STRUCT_LITERAL); + } + + // `1..*i` is ambiguous between `1..(*i)` and `(1..)*(i)`. + // We pick the `1..(*i)` interpretation. + + // `r==1..&&true` is ambiguous between `r==(1..(&&true))` and `(r==(1..))&&true`. + // We pick the latter interpretation. + match self.token.to_binop() { + Some(op) => operator_prec(op) > range_prec, + None => true + } + } else { + false + } + } + /// 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 @@ -2944,7 +2981,7 @@ impl<'a> Parser<'a> { /// actually, this seems to be the main entry point for /// parsing an arbitrary expression. pub fn parse_assign_expr(&mut self) -> P { - let lhs = self.parse_binops(); + let lhs = self.parse_binops(0); self.parse_assign_expr_with(lhs) } @@ -2976,23 +3013,6 @@ impl<'a> Parser<'a> { let assign_op = self.mk_assign_op(aop, lhs, rhs); self.mk_expr(span.lo, rhs_span.hi, assign_op) } - // A range expression, either `expr..expr` or `expr..`. - token::DotDot if !self.restrictions.contains(RESTRICTION_NO_DOTS) => { - self.bump(); - - let opt_end = if self.token.can_begin_expr() { - let end = self.parse_expr_res(RESTRICTION_NO_DOTS); - Some(end) - } else { - None - }; - - let lo = lhs.span.lo; - let hi = self.span.hi; - let range = self.mk_range(Some(lhs), opt_end); - return self.mk_expr(lo, hi, range); - } - _ => { lhs } diff --git a/src/test/compile-fail/range-3.rs b/src/test/compile-fail/range-3.rs new file mode 100644 index 0000000000000..78c575d33bad0 --- /dev/null +++ b/src/test/compile-fail/range-3.rs @@ -0,0 +1,16 @@ +// Copyright 2014 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test range syntax - syntax errors. + +pub fn main() { + let r = 1..2..3; + //~^ ERROR expected one of `.`, `;`, or an operator, found `..` +} diff --git a/src/test/run-pass/ranges-precedence.rs b/src/test/run-pass/ranges-precedence.rs index f678eed8775cd..1151a4a0d66f4 100644 --- a/src/test/run-pass/ranges-precedence.rs +++ b/src/test/run-pass/ranges-precedence.rs @@ -48,5 +48,13 @@ fn main() { assert!(x == &a[3..]); for _i in 2+4..10-3 {} + + let i = 42; + for _ in 1..i {} + for _ in 1.. { break; } + + if 1..2 == 1..2 {} + if 1.. == 1.. {} + if ..1 == ..1 {} }