auto merge of #17264 : bkoropoff/rust/issue-17252, r=nick29581

Recursive items are currently detected in the `check_const` pass which runs after type checking.  This means a recursive static item used as an array length will cause type checking to blow the stack.  This PR separates the recursion check out into a separate pass which is run before type checking.

Closes issue #17252

r? @nick29581

src/librustc/driver/driver.rs

@@ -414,6 +414,9 @@ pub fn phase_3_run_analysis_passes<'tcx>(sess: Session,
     let stability_index = time(time_passes, "stability index", (), |_|
                                stability::Index::build(krate));
 
+    time(time_passes, "static item recursion checking", (), |_|
+         middle::check_static_recursion::check_crate(&sess, krate, &def_map, &ast_map));
+
     let ty_cx = ty::mk_ctxt(sess,
                             type_arena,
                             def_map,

src/librustc/lib.rs

@@ -80,6 +80,7 @@ pub mod middle {
     pub mod borrowck;
     pub mod cfg;
     pub mod check_const;
+    pub mod check_static_recursion;
     pub mod check_loop;
     pub mod check_match;
     pub mod check_rvalues;

src/librustc/middle/check_const.rs

@@ -9,15 +9,13 @@
 // except according to those terms.
 
 
-use driver::session::Session;
 use middle::def::*;
-use middle::resolve;
 use middle::ty;
 use middle::typeck;
 use util::ppaux;
 
 use syntax::ast::*;
-use syntax::{ast_util, ast_map};
+use syntax::ast_util;
 use syntax::visit::Visitor;
 use syntax::visit;
 
@@ -63,7 +61,6 @@ fn check_item(v: &mut CheckCrateVisitor, it: &Item) {
     match it.node {
         ItemStatic(_, _, ref ex) => {
             v.inside_const(|v| v.visit_expr(&**ex));
-            check_item_recursion(&v.tcx.sess, &v.tcx.map, &v.tcx.def_map, it);
         }
         ItemEnum(ref enum_definition, _) => {
             for var in (*enum_definition).variants.iter() {
@@ -214,55 +211,3 @@ fn check_expr(v: &mut CheckCrateVisitor, e: &Expr) {
     }
     visit::walk_expr(v, e);
 }
-
-struct CheckItemRecursionVisitor<'a, 'ast: 'a> {
-    root_it: &'a Item,
-    sess: &'a Session,
-    ast_map: &'a ast_map::Map<'ast>,
-    def_map: &'a resolve::DefMap,
-    idstack: Vec<NodeId>
-}
-
-// Make sure a const item doesn't recursively refer to itself
-// FIXME: Should use the dependency graph when it's available (#1356)
-pub fn check_item_recursion<'a>(sess: &'a Session,
-                                ast_map: &'a ast_map::Map,
-                                def_map: &'a resolve::DefMap,
-                                it: &'a Item) {
-
-    let mut visitor = CheckItemRecursionVisitor {
-        root_it: it,
-        sess: sess,
-        ast_map: ast_map,
-        def_map: def_map,
-        idstack: Vec::new()
-    };
-    visitor.visit_item(it);
-}
-
-impl<'a, 'ast, 'v> Visitor<'v> for CheckItemRecursionVisitor<'a, 'ast> {
-    fn visit_item(&mut self, it: &Item) {
-        if self.idstack.iter().any(|x| x == &(it.id)) {
-            self.sess.span_fatal(self.root_it.span, "recursive constant");
-        }
-        self.idstack.push(it.id);
-        visit::walk_item(self, it);
-        self.idstack.pop();
-    }
-
-    fn visit_expr(&mut self, e: &Expr) {
-        match e.node {
-            ExprPath(..) => {
-                match self.def_map.borrow().find(&e.id) {
-                    Some(&DefStatic(def_id, _)) if
-                            ast_util::is_local(def_id) => {
-                        self.visit_item(&*self.ast_map.expect_item(def_id.node));
-                    }
-                    _ => ()
-                }
-            },
-            _ => ()
-        }
-        visit::walk_expr(self, e);
-    }
-}

src/librustc/middle/check_static_recursion.rs

@@ -0,0 +1,109 @@
+// 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 <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.
+
+// This compiler pass detects static items that refer to themselves
+// recursively.
+
+use driver::session::Session;
+use middle::resolve;
+use middle::def::DefStatic;
+
+use syntax::ast::{Crate, Expr, ExprPath, Item, ItemStatic, NodeId};
+use syntax::{ast_util, ast_map};
+use syntax::visit::Visitor;
+use syntax::visit;
+
+struct CheckCrateVisitor<'a, 'ast: 'a> {
+    sess: &'a Session,
+    def_map: &'a resolve::DefMap,
+    ast_map: &'a ast_map::Map<'ast>
+}
+
+impl<'v, 'a, 'ast> Visitor<'v> for CheckCrateVisitor<'a, 'ast> {
+    fn visit_item(&mut self, i: &Item) {
+        check_item(self, i);
+    }
+}
+
+pub fn check_crate<'ast>(sess: &Session,
+                         krate: &Crate,
+                         def_map: &resolve::DefMap,
+                         ast_map: &ast_map::Map<'ast>) {
+    let mut visitor = CheckCrateVisitor {
+        sess: sess,
+        def_map: def_map,
+        ast_map: ast_map
+    };
+    visit::walk_crate(&mut visitor, krate);
+    sess.abort_if_errors();
+}
+
+fn check_item(v: &mut CheckCrateVisitor, it: &Item) {
+    match it.node {
+        ItemStatic(_, _, ref ex) => {
+            check_item_recursion(v.sess, v.ast_map, v.def_map, it);
+            visit::walk_expr(v, &**ex)
+        },
+        _ => visit::walk_item(v, it)
+    }
+}
+
+struct CheckItemRecursionVisitor<'a, 'ast: 'a> {
+    root_it: &'a Item,
+    sess: &'a Session,
+    ast_map: &'a ast_map::Map<'ast>,
+    def_map: &'a resolve::DefMap,
+    idstack: Vec<NodeId>
+}
+
+// Make sure a const item doesn't recursively refer to itself
+// FIXME: Should use the dependency graph when it's available (#1356)
+pub fn check_item_recursion<'a>(sess: &'a Session,
+                                ast_map: &'a ast_map::Map,
+                                def_map: &'a resolve::DefMap,
+                                it: &'a Item) {
+
+    let mut visitor = CheckItemRecursionVisitor {
+        root_it: it,
+        sess: sess,
+        ast_map: ast_map,
+        def_map: def_map,
+        idstack: Vec::new()
+    };
+    visitor.visit_item(it);
+}
+
+impl<'a, 'ast, 'v> Visitor<'v> for CheckItemRecursionVisitor<'a, 'ast> {
+    fn visit_item(&mut self, it: &Item) {
+        if self.idstack.iter().any(|x| x == &(it.id)) {
+            self.sess.span_err(self.root_it.span, "recursive constant");
+            return;
+        }
+        self.idstack.push(it.id);
+        visit::walk_item(self, it);
+        self.idstack.pop();
+    }
+
+    fn visit_expr(&mut self, e: &Expr) {
+        match e.node {
+            ExprPath(..) => {
+                match self.def_map.borrow().find(&e.id) {
+                    Some(&DefStatic(def_id, _)) if
+                            ast_util::is_local(def_id) => {
+                        self.visit_item(&*self.ast_map.expect_item(def_id.node));
+                    }
+                    _ => ()
+                }
+            },
+            _ => ()
+        }
+        visit::walk_expr(self, e);
+    }
+}

src/test/compile-fail/issue-17252.rs

@@ -0,0 +1,20 @@
+// 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 <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.
+
+static FOO: uint = FOO; //~ ERROR recursive constant
+
+fn main() {
+    let _x: [u8, ..FOO]; // caused stack overflow prior to fix
+    let _y: uint = 1 + {
+        static BAR: uint = BAR; //~ ERROR recursive constant
+        let _z: [u8, ..BAR]; // caused stack overflow prior to fix
+        1
+    };
+}