Skip to content

Add error for ... in expressions #45773

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions src/libcore/slice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@

#![stable(feature = "rust1", since = "1.0.0")]

// FIXME: after next stage0, change RangeInclusive { ... } back to ..=
use ops::RangeInclusive;

// How this module is organized.
//
// The library infrastructure for slices is fairly messy. There's
Expand Down Expand Up @@ -1047,32 +1044,32 @@ impl<T> SliceIndex<[T]> for ops::RangeToInclusive<usize> {

#[inline]
fn get(self, slice: &[T]) -> Option<&[T]> {
(RangeInclusive { start: 0, end: self.end }).get(slice)
(0..=self.end).get(slice)
}

#[inline]
fn get_mut(self, slice: &mut [T]) -> Option<&mut [T]> {
(RangeInclusive { start: 0, end: self.end }).get_mut(slice)
(0..=self.end).get_mut(slice)
}

#[inline]
unsafe fn get_unchecked(self, slice: &[T]) -> &[T] {
(RangeInclusive { start: 0, end: self.end }).get_unchecked(slice)
(0..=self.end).get_unchecked(slice)
}

#[inline]
unsafe fn get_unchecked_mut(self, slice: &mut [T]) -> &mut [T] {
(RangeInclusive { start: 0, end: self.end }).get_unchecked_mut(slice)
(0..=self.end).get_unchecked_mut(slice)
}

#[inline]
fn index(self, slice: &[T]) -> &[T] {
(RangeInclusive { start: 0, end: self.end }).index(slice)
(0..=self.end).index(slice)
}

#[inline]
fn index_mut(self, slice: &mut [T]) -> &mut [T] {
(RangeInclusive { start: 0, end: self.end }).index_mut(slice)
(0..=self.end).index_mut(slice)
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/maps/on_disk_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ impl<'a> CacheDecoder<'a> {
fn find_filemap_prev_bytepos(&self,
prev_bytepos: BytePos)
-> Option<(BytePos, StableFilemapId)> {
for (start, id) in self.prev_filemap_starts.range(BytePos(0) ... prev_bytepos).rev() {
for (start, id) in self.prev_filemap_starts.range(BytePos(0) ..= prev_bytepos).rev() {
return Some((*start, *id))
}

Expand Down
32 changes: 18 additions & 14 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2784,10 +2784,11 @@ impl<'a> Parser<'a> {
if op.precedence() < min_prec {
break;
}
// Warn about deprecated ... syntax (until SNAP)
if self.token == token::DotDotDot {
self.warn_dotdoteq(self.span);
// Check for deprecated `...` syntax
if self.token == token::DotDotDot && op == AssocOp::DotDotEq {
self.err_dotdotdot_syntax(self.span);
}

self.bump();
if op.is_comparison() {
self.check_no_chained_comparison(&lhs, &op);
Expand Down Expand Up @@ -2820,7 +2821,6 @@ impl<'a> Parser<'a> {
//
// We have 2 alternatives here: `x..y`/`x..=y` and `x..`/`x..=` The other
// two variants are handled with `parse_prefix_range_expr` call above.
// (and `x...y`/`x...` until SNAP)
let rhs = if self.is_at_start_of_range_notation_rhs() {
Some(self.parse_assoc_expr_with(op.precedence() + 1,
LhsExpr::NotYetParsed)?)
Expand Down Expand Up @@ -3008,22 +3008,22 @@ impl<'a> Parser<'a> {
}
}

/// Parse prefix-forms of range notation: `..expr`, `..`, `..=expr` (and `...expr` until SNAP)
/// Parse prefix-forms of range notation: `..expr`, `..`, `..=expr`
fn parse_prefix_range_expr(&mut self,
already_parsed_attrs: Option<ThinVec<Attribute>>)
-> PResult<'a, P<Expr>> {
// SNAP remove DotDotDot
// Check for deprecated `...` syntax
if self.token == token::DotDotDot {
self.err_dotdotdot_syntax(self.span);
}

debug_assert!([token::DotDot, token::DotDotDot, token::DotDotEq].contains(&self.token),
"parse_prefix_range_expr: token {:?} is not DotDot/DotDotDot/DotDotEq",
"parse_prefix_range_expr: token {:?} is not DotDot/DotDotEq",
self.token);
let tok = self.token.clone();
let attrs = self.parse_or_use_outer_attributes(already_parsed_attrs)?;
let lo = self.span;
let mut hi = self.span;
// Warn about deprecated ... syntax (until SNAP)
if tok == token::DotDotDot {
self.warn_dotdoteq(self.span);
}
self.bump();
let opt_end = if self.is_at_start_of_range_notation_rhs() {
// RHS must be parsed with more associativity than the dots.
Expand Down Expand Up @@ -4271,9 +4271,13 @@ impl<'a> Parser<'a> {
}).emit();
}

fn warn_dotdoteq(&self, span: Span) {
self.diagnostic().struct_span_warn(span, {
"`...` is being replaced by `..=`"
fn err_dotdotdot_syntax(&self, span: Span) {
self.diagnostic().struct_span_err(span, {
"`...` syntax cannot be used in expressions"
}).help({
"Use `..` if you need an exclusive range (a < b)"
}).help({
"or `..=` if you need an inclusive range (a <= b)"
}).emit();
}

Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/parse/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,8 @@ impl Token {
BinOp(Or) | OrOr | // closure
BinOp(And) | // reference
AndAnd | // double reference
// DotDotDot is no longer supported, but we need some way to display the error
DotDot | DotDotDot | DotDotEq | // range notation
// SNAP remove DotDotDot
Lt | BinOp(Shl) | // associated path
ModSep | // global path
Pound => true, // expression attributes
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/print/pprust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2203,7 +2203,7 @@ impl<'a> State<'a> {
if limits == ast::RangeLimits::HalfOpen {
self.s.word("..")?;
} else {
self.s.word("...")?;
self.s.word("..=")?;
}
if let Some(ref e) = *end {
self.print_expr_maybe_paren(e, fake_prec)?;
Expand Down
3 changes: 2 additions & 1 deletion src/libsyntax/util/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ impl AssocOp {
Token::OrOr => Some(LOr),
Token::DotDot => Some(DotDot),
Token::DotDotEq => Some(DotDotEq),
Token::DotDotDot => Some(DotDotEq), // remove this after SNAP
// DotDotDot is no longer supported, but we need some way to display the error
Token::DotDotDot => Some(DotDotEq),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main concern is that in order to show the error we need to parse the expression, so here I'm parsing Token::DotDotDot as AssocOp::DotDotEq, and in src/libsyntax/parse/parser.rs I check for Token::DotDotDot and AssocOp::DotDotEq to show the error. An alternative would be returning None here but then the expression would be incomplete (adding the generic "expected one of" error).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm parsing Token::DotDotDot as AssocOp::DotDotEq

That's exactly what is needed for good error recovery.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

Token::Colon => Some(Colon),
_ if t.is_keyword(keywords::As) => Some(As),
_ => None
Expand Down
38 changes: 38 additions & 0 deletions src/test/parse-fail/range_inclusive_dotdotdot.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright 2017 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.

// compile-flags: -Z parse-only -Z continue-parse-after-error

// Make sure that inclusive ranges with `...` syntax don't parse.

#![feature(inclusive_range_syntax, inclusive_range)]

use std::ops::RangeToInclusive;

fn return_range_to() -> RangeToInclusive<i32> {
return ...1; //~ERROR `...` syntax cannot be used in expressions
//~^HELP Use `..` if you need an exclusive range (a < b)
//~^^HELP or `..=` if you need an inclusive range (a <= b)
}

pub fn main() {
let x = ...0; //~ERROR `...` syntax cannot be used in expressions
//~^HELP Use `..` if you need an exclusive range (a < b)
//~^^HELP or `..=` if you need an inclusive range (a <= b)

let x = 5...5; //~ERROR `...` syntax cannot be used in expressions
//~^HELP Use `..` if you need an exclusive range (a < b)
//~^^HELP or `..=` if you need an inclusive range (a <= b)

for _ in 0...1 {} //~ERROR `...` syntax cannot be used in expressions
//~^HELP Use `..` if you need an exclusive range (a < b)
//~^^HELP or `..=` if you need an inclusive range (a <= b)
}

4 changes: 2 additions & 2 deletions src/test/run-pass-fulldeps/issue-35829.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ fn main() {
let raw_byte_string_lit_kind = LitKind::ByteStr(Rc::new(b"#\"two\"#".to_vec()));
assert_eq!(raw_byte_string.node, ExprKind::Lit(P(dummy_spanned(raw_byte_string_lit_kind))));

// check dotdotdot
let closed_range = quote_expr!(&cx, 0 ... 1);
// check dotdoteq
let closed_range = quote_expr!(&cx, 0 ..= 1);
assert_eq!(closed_range.node, ExprKind::Range(
Some(quote_expr!(&cx, 0)),
Some(quote_expr!(&cx, 1)),
Expand Down