Auto merge of #32267 - durka:inclusive-range-error, r=nrc

melt the ICE when lowering an impossible range

Emit a fatal error instead of panicking when HIR lowering encounters a range with no `end` point.

This involved adding a method to wire up `LoweringContext::span_fatal`.

Fixes #32245 (cc @nodakai).

r? @nrc

6 files changed, 88 insertions, 32 deletions

diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs
index 19123f0f15e..20ca6213254 100644
--- a/src/librustc/session/mod.rs
+++ b/src/librustc/session/mod.rs
@@ -348,6 +348,10 @@ impl NodeIdAssigner for Session {
     fn peek_node_id(&self) -> NodeId {
         self.next_node_id.get().checked_add(1).unwrap()
     }
+
+    fn diagnostic(&self) -> &errors::Handler {
+        self.diagnostic()
+    }
 }
 
 fn split_msg_into_multilines(msg: &str) -> Option<String> {
diff --git a/src/librustc_front/lowering.rs b/src/librustc_front/lowering.rs
index 8aac6356f9d..be184179451 100644
--- a/src/librustc_front/lowering.rs
+++ b/src/librustc_front/lowering.rs
@@ -68,6 +68,7 @@ use std::collections::HashMap;
 use std::iter;
 use syntax::ast::*;
 use syntax::attr::{ThinAttributes, ThinAttributesExt};
+use syntax::errors::Handler;
 use syntax::ext::mtwt;
 use syntax::ptr::P;
 use syntax::codemap::{respan, Spanned, Span};
@@ -140,6 +141,11 @@ impl<'a, 'hir> LoweringContext<'a> {
             result
         }
     }
+
+    // Panics if this LoweringContext's NodeIdAssigner is not able to emit diagnostics.
+    fn diagnostic(&self) -> &Handler {
+        self.id_assigner.diagnostic()
+    }
 }
 
 // Utility fn for setting and unsetting the cached id.
@@ -1289,7 +1295,8 @@ pub fn lower_expr(lctx: &LoweringContext, e: &Expr) -> P<hir::Expr> {
                             make_struct(lctx, e, &["RangeInclusive", "NonEmpty"],
                                                  &[("start", e1), ("end", e2)]),
 
-                        _ => panic!("impossible range in AST"),
+                        _ => panic!(lctx.diagnostic().span_fatal(e.span,
+                                                                 "inclusive range with no end"))
                     }
                 });
             }
diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
index ac1a07d1cb5..e096aa99024 100644
--- a/src/libsyntax/ast.rs
+++ b/src/libsyntax/ast.rs
@@ -19,6 +19,7 @@ pub use self::PathParameters::*;
 use attr::ThinAttributes;
 use codemap::{Span, Spanned, DUMMY_SP, ExpnId};
 use abi::Abi;
+use errors;
 use ext::base;
 use ext::tt::macro_parser;
 use parse::token::InternedString;
@@ -344,6 +345,10 @@ pub const DUMMY_NODE_ID: NodeId = !0;
 pub trait NodeIdAssigner {
     fn next_node_id(&self) -> NodeId;
     fn peek_node_id(&self) -> NodeId;
+
+    fn diagnostic(&self) -> &errors::Handler {
+        panic!("this ID assigner cannot emit diagnostics")
+    }
 }
 
 /// The AST represents all type param bounds as types.
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 82715f263c9..dd133d74b4f 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -2072,8 +2072,15 @@ impl<'a> Parser<'a> {
                     start: Option<P<Expr>>,
                     end: Option<P<Expr>>,
                     limits: RangeLimits)
-                    -> ast::ExprKind {
-        ExprKind::Range(start, end, limits)
+                    -> PResult<'a, ast::ExprKind> {
+        if end.is_none() && limits == RangeLimits::Closed {
+            Err(self.span_fatal_help(self.span,
+                                     "inclusive range with no end",
+                                     "inclusive ranges must be bounded at the end \
+                                      (`...b` or `a...b`)"))
+        } else {
+            Ok(ExprKind::Range(start, end, limits))
+        }
     }
 
     pub fn mk_field(&mut self, expr: P<Expr>, ident: ast::SpannedIdent) -> ast::ExprKind {
@@ -2999,12 +3006,12 @@ impl<'a> Parser<'a> {
                 lhs = self.mk_expr(lhs_span.lo, rhs.span.hi,
                                    ExprKind::Type(lhs, rhs), None);
                 continue
-            } else if op == AssocOp::DotDot {
-                // If we didn’t have to handle `x..`, it would be pretty easy to generalise
-                // it to the Fixity::None code.
+            } else if op == AssocOp::DotDot || op == AssocOp::DotDotDot {
+                // If we didn’t have to handle `x..`/`x...`, it would be pretty easy to
+                // generalise it to the Fixity::None code.
                 //
-                // We have 2 alternatives here: `x..y` and `x..` The other two variants are
-                // handled with `parse_prefix_range_expr` call above.
+                // 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.
                 let rhs = if self.is_at_start_of_range_notation_rhs() {
                     let rhs = self.parse_assoc_expr_with(op.precedence() + 1,
                                                          LhsExpr::NotYetParsed);
@@ -3023,7 +3030,13 @@ impl<'a> Parser<'a> {
                 } else {
                     cur_op_span
                 });
-                let r = self.mk_range(Some(lhs), rhs, RangeLimits::HalfOpen);
+                let limits = if op == AssocOp::DotDot {
+                    RangeLimits::HalfOpen
+                } else {
+                    RangeLimits::Closed
+                };
+
+                let r = try!(self.mk_range(Some(lhs), rhs, limits));
                 lhs = self.mk_expr(lhs_span.lo, rhs_span.hi, r, None);
                 break
             }
@@ -3041,8 +3054,8 @@ impl<'a> Parser<'a> {
                         this.parse_assoc_expr_with(op.precedence() + 1,
                             LhsExpr::NotYetParsed)
                 }),
-                // the only operator handled here is `...` (the other non-associative operators are
-                // special-cased above)
+                // We currently have no non-associative operators that are not handled above by
+                // the special cases. The code is here only for future convenience.
                 Fixity::None => self.with_res(
                     restrictions - Restrictions::RESTRICTION_STMT_EXPR,
                     |this| {
@@ -3083,13 +3096,8 @@ impl<'a> Parser<'a> {
                     let aopexpr = self.mk_assign_op(codemap::respan(cur_op_span, aop), lhs, rhs);
                     self.mk_expr(lhs_span.lo, rhs_span.hi, aopexpr, None)
                 }
-                AssocOp::DotDotDot => {
-                    let (lhs_span, rhs_span) = (lhs.span, rhs.span);
-                    let r = self.mk_range(Some(lhs), Some(rhs), RangeLimits::Closed);
-                    self.mk_expr(lhs_span.lo, rhs_span.hi, r, None)
-                }
-                AssocOp::As | AssocOp::Colon | AssocOp::DotDot => {
-                    self.bug("As, Colon or DotDot branch reached")
+                AssocOp::As | AssocOp::Colon | AssocOp::DotDot | AssocOp::DotDotDot => {
+                    self.bug("As, Colon, DotDot or DotDotDot branch reached")
                 }
             };
 
@@ -3133,21 +3141,23 @@ impl<'a> Parser<'a> {
             // RHS must be parsed with more associativity than the dots.
             let next_prec = AssocOp::from_token(&tok).unwrap().precedence() + 1;
             Some(self.parse_assoc_expr_with(next_prec,
-                                                 LhsExpr::NotYetParsed)
-            .map(|x|{
-                hi = x.span.hi;
-                x
-            })?)
+                                            LhsExpr::NotYetParsed)
+                .map(|x|{
+                    hi = x.span.hi;
+                    x
+                })?)
          } else {
             None
         };
-        let r = self.mk_range(None,
-                              opt_end,
-                              if tok == token::DotDot {
-                                  RangeLimits::HalfOpen
-                              } else {
-                                  RangeLimits::Closed
-                              });
+        let limits = if tok == token::DotDot {
+            RangeLimits::HalfOpen
+        } else {
+            RangeLimits::Closed
+        };
+
+        let r = try!(self.mk_range(None,
+                                   opt_end,
+                                   limits));
         Ok(self.mk_expr(lo, hi, r, attrs))
     }
 
diff --git a/src/test/compile-fail/impossible_range.rs b/src/test/compile-fail/impossible_range.rs
new file mode 100644
index 00000000000..94e048fed65
--- /dev/null
+++ b/src/test/compile-fail/impossible_range.rs
@@ -0,0 +1,29 @@
+// Copyright 2016 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.
+
+// Make sure that invalid ranges generate an error during HIR lowering, not an ICE
+
+#![feature(inclusive_range_syntax)]
+
+pub fn main() {
+    ..;
+    0..;
+    ..1;
+    0..1;
+
+    ...; //~ERROR inclusive range with no end
+         //~^HELP bounded at the end
+    0...; //~ERROR inclusive range with no end
+          //~^HELP bounded at the end
+    ...1;
+    0...1;
+}
+
+
diff --git a/src/test/parse-fail/range_inclusive.rs b/src/test/parse-fail/range_inclusive.rs
index 5fd6f1834e0..ce97372c668 100644
--- a/src/test/parse-fail/range_inclusive.rs
+++ b/src/test/parse-fail/range_inclusive.rs
@@ -13,6 +13,7 @@
 #![feature(inclusive_range_syntax, inclusive_range)]
 
 pub fn main() {
-    for _ in 1... {}
-} //~ ERROR expected one of
+    for _ in 1... {} //~ERROR inclusive range with no end
+                     //~^HELP bounded at the end
+}