From 83b7cb7ceb75d9fb4897d2f944baa09d63a34f31 Mon Sep 17 00:00:00 2001 From: Brian Koropoff Date: Sun, 14 Sep 2014 17:21:25 -0700 Subject: [PATCH 1/2] Separate static item recursion check into its own pass This new pass is run before type checking so that recursive items are detected beforehand. This prevents going into an infinite recursion during type checking when a recursive item is used in an array type. As a bonus, use `span_err` instead of `span_fatal` so multiple errors can be reported. Closes issue #17252 --- src/librustc/driver/driver.rs | 3 + src/librustc/lib.rs | 1 + src/librustc/middle/check_const.rs | 57 +-------- src/librustc/middle/check_static_recursion.rs | 109 ++++++++++++++++++ 4 files changed, 114 insertions(+), 56 deletions(-) create mode 100644 src/librustc/middle/check_static_recursion.rs diff --git a/src/librustc/driver/driver.rs b/src/librustc/driver/driver.rs index b80d53922f8a5..23c10c892a1b2 100644 --- a/src/librustc/driver/driver.rs +++ b/src/librustc/driver/driver.rs @@ -394,6 +394,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, diff --git a/src/librustc/lib.rs b/src/librustc/lib.rs index fd643a70c7b95..c6eee6569a6c5 100644 --- a/src/librustc/lib.rs +++ b/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; diff --git a/src/librustc/middle/check_const.rs b/src/librustc/middle/check_const.rs index 303961105b526..9b699a240cbaf 100644 --- a/src/librustc/middle/check_const.rs +++ b/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 -} - -// 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); - } -} diff --git a/src/librustc/middle/check_static_recursion.rs b/src/librustc/middle/check_static_recursion.rs new file mode 100644 index 0000000000000..b571a18c1ece7 --- /dev/null +++ b/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 or the MIT license +// , 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 +} + +// 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); + } +} From 0818955905bd508bff1af2546a39094fcc5eeb7b Mon Sep 17 00:00:00 2001 From: Brian Koropoff Date: Sun, 14 Sep 2014 17:45:13 -0700 Subject: [PATCH 2/2] Add regression test for issue #17252 --- src/test/compile-fail/issue-17252.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 src/test/compile-fail/issue-17252.rs diff --git a/src/test/compile-fail/issue-17252.rs b/src/test/compile-fail/issue-17252.rs new file mode 100644 index 0000000000000..4a6b80d765b71 --- /dev/null +++ b/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 or the MIT license +// , 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 + }; +}