Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix stack overflow by enum and cont issue #36163 #38093

Merged
merged 1 commit into from
Dec 5, 2016
Merged

fix stack overflow by enum and cont issue #36163 #38093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
125 changes: 60 additions & 65 deletions src/librustc_passes/static_recursion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,23 @@ use rustc::dep_graph::DepNode;
use rustc::hir::map as ast_map;
use rustc::session::{CompileResult, Session};
use rustc::hir::def::{Def, CtorKind};
use rustc::util::nodemap::NodeMap;
use rustc::util::nodemap::{NodeMap, NodeSet};

use syntax::ast;
use syntax::feature_gate::{GateIssue, emit_feature_err};
use syntax_pos::Span;
use rustc::hir::intravisit::{self, Visitor, NestedVisitorMap};
use rustc::hir;

use std::cell::RefCell;

struct CheckCrateVisitor<'a, 'ast: 'a> {
sess: &'a Session,
ast_map: &'a ast_map::Map<'ast>,
// `discriminant_map` is a cache that associates the `NodeId`s of local
// variant definitions with the discriminant expression that applies to
// each one. If the variant uses the default values (starting from `0`),
// then `None` is stored.
discriminant_map: RefCell<NodeMap<Option<&'ast hir::Expr>>>,
discriminant_map: NodeMap<Option<&'ast hir::Expr>>,
detected_recursive_ids: NodeSet,
}

impl<'a, 'ast: 'a> Visitor<'ast> for CheckCrateVisitor<'a, 'ast> {
Expand Down Expand Up @@ -90,46 +89,49 @@ impl<'a, 'ast: 'a> Visitor<'ast> for CheckCrateVisitor<'a, 'ast> {
}
}

pub fn check_crate<'ast>(sess: &Session,
ast_map: &ast_map::Map<'ast>)
-> CompileResult {
pub fn check_crate<'ast>(sess: &Session, ast_map: &ast_map::Map<'ast>) -> CompileResult {
let _task = ast_map.dep_graph.in_task(DepNode::CheckStaticRecursion);

let mut visitor = CheckCrateVisitor {
sess: sess,
ast_map: ast_map,
discriminant_map: RefCell::new(NodeMap()),
discriminant_map: NodeMap(),
detected_recursive_ids: NodeSet(),
};
sess.track_errors(|| {
// FIXME(#37712) could use ItemLikeVisitor if trait items were item-like
ast_map.krate().visit_all_item_likes(&mut visitor.as_deep_visitor());
})
}

struct CheckItemRecursionVisitor<'a, 'ast: 'a> {
root_span: &'a Span,
sess: &'a Session,
ast_map: &'a ast_map::Map<'ast>,
discriminant_map: &'a RefCell<NodeMap<Option<&'ast hir::Expr>>>,
struct CheckItemRecursionVisitor<'a, 'b: 'a, 'ast: 'b> {
root_span: &'b Span,
sess: &'b Session,
ast_map: &'b ast_map::Map<'ast>,
discriminant_map: &'a mut NodeMap<Option<&'ast hir::Expr>>,
idstack: Vec<ast::NodeId>,
detected_recursive_ids: &'a mut NodeSet,
}

impl<'a, 'ast: 'a> CheckItemRecursionVisitor<'a, 'ast> {
fn new(v: &'a CheckCrateVisitor<'a, 'ast>,
span: &'a Span)
-> CheckItemRecursionVisitor<'a, 'ast> {
impl<'a, 'b: 'a, 'ast: 'b> CheckItemRecursionVisitor<'a, 'b, 'ast> {
fn new(v: &'a mut CheckCrateVisitor<'b, 'ast>, span: &'b Span) -> Self {
CheckItemRecursionVisitor {
root_span: span,
sess: v.sess,
ast_map: v.ast_map,
discriminant_map: &v.discriminant_map,
discriminant_map: &mut v.discriminant_map,
idstack: Vec::new(),
detected_recursive_ids: &mut v.detected_recursive_ids,
}
}
fn with_item_id_pushed<F>(&mut self, id: ast::NodeId, f: F, span: Span)
where F: Fn(&mut Self)
{
if self.idstack.iter().any(|&x| x == id) {
if self.detected_recursive_ids.contains(&id) {
return;
}
self.detected_recursive_ids.insert(id);
let any_static = self.idstack.iter().any(|&x| {
if let ast_map::NodeItem(item) = self.ast_map.get(x) {
if let hir::ItemStatic(..) = item.node {
Expand Down Expand Up @@ -168,15 +170,14 @@ impl<'a, 'ast: 'a> CheckItemRecursionVisitor<'a, 'ast> {
// So for every variant, we need to track whether there is an expression
// somewhere in the enum definition that controls its discriminant. We do
// this by starting from the end and searching backward.
fn populate_enum_discriminants(&self, enum_definition: &'ast hir::EnumDef) {
fn populate_enum_discriminants(&mut self, enum_definition: &'ast hir::EnumDef) {
// Get the map, and return if we already processed this enum or if it
// has no variants.
let mut discriminant_map = self.discriminant_map.borrow_mut();
match enum_definition.variants.first() {
None => {
return;
}
Some(variant) if discriminant_map.contains_key(&variant.node.data.id()) => {
Some(variant) if self.discriminant_map.contains_key(&variant.node.data.id()) => {
return;
}
_ => {}
Expand All @@ -190,24 +191,23 @@ impl<'a, 'ast: 'a> CheckItemRecursionVisitor<'a, 'ast> {
// is affected by that expression.
if let Some(ref expr) = variant.node.disr_expr {
for id in &variant_stack {
discriminant_map.insert(*id, Some(expr));
self.discriminant_map.insert(*id, Some(expr));
}
variant_stack.clear()
}
}
// If we are at the top, that always starts at 0, so any variant on the
// stack has a default value and does not need to be checked.
for id in &variant_stack {
discriminant_map.insert(*id, None);
self.discriminant_map.insert(*id, None);
}
}
}

impl<'a, 'ast: 'a> Visitor<'ast> for CheckItemRecursionVisitor<'a, 'ast> {
impl<'a, 'b: 'a, 'ast: 'b> Visitor<'ast> for CheckItemRecursionVisitor<'a, 'b, 'ast> {
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'ast> {
NestedVisitorMap::OnlyBodies(&self.ast_map)
}

fn visit_item(&mut self, it: &'ast hir::Item) {
self.with_item_id_pushed(it.id, |v| intravisit::walk_item(v, it), it.span);
}
Expand All @@ -227,7 +227,7 @@ impl<'a, 'ast: 'a> Visitor<'ast> for CheckItemRecursionVisitor<'a, 'ast> {
_: ast::NodeId) {
let variant_id = variant.node.data.id();
let maybe_expr;
if let Some(get_expr) = self.discriminant_map.borrow().get(&variant_id) {
if let Some(get_expr) = self.discriminant_map.get(&variant_id) {
// This is necessary because we need to let the `discriminant_map`
// borrow fall out of scope, so that we can reborrow farther down.
maybe_expr = (*get_expr).clone();
Expand All @@ -251,51 +251,46 @@ impl<'a, 'ast: 'a> Visitor<'ast> for CheckItemRecursionVisitor<'a, 'ast> {
self.with_item_id_pushed(ii.id, |v| intravisit::walk_impl_item(v, ii), ii.span);
}

fn visit_expr(&mut self, e: &'ast hir::Expr) {
match e.node {
hir::ExprPath(hir::QPath::Resolved(_, ref path)) => {
match path.def {
Def::Static(def_id, _) |
Def::AssociatedConst(def_id) |
Def::Const(def_id) => {
if let Some(node_id) = self.ast_map.as_local_node_id(def_id) {
match self.ast_map.get(node_id) {
ast_map::NodeItem(item) => self.visit_item(item),
ast_map::NodeTraitItem(item) => self.visit_trait_item(item),
ast_map::NodeImplItem(item) => self.visit_impl_item(item),
ast_map::NodeForeignItem(_) => {}
_ => {
span_bug!(e.span,
"expected item, found {}",
self.ast_map.node_to_string(node_id));
}
}
fn visit_path(&mut self, path: &'ast hir::Path, _: ast::NodeId) {
match path.def {
Def::Static(def_id, _) |
Def::AssociatedConst(def_id) |
Def::Const(def_id) => {
if let Some(node_id) = self.ast_map.as_local_node_id(def_id) {
match self.ast_map.get(node_id) {
ast_map::NodeItem(item) => self.visit_item(item),
ast_map::NodeTraitItem(item) => self.visit_trait_item(item),
ast_map::NodeImplItem(item) => self.visit_impl_item(item),
ast_map::NodeForeignItem(_) => {}
_ => {
span_bug!(path.span,
"expected item, found {}",
self.ast_map.node_to_string(node_id));
}
}
// For variants, we only want to check expressions that
// affect the specific variant used, but we need to check
// the whole enum definition to see what expression that
// might be (if any).
Def::VariantCtor(variant_id, CtorKind::Const) => {
if let Some(variant_id) = self.ast_map.as_local_node_id(variant_id) {
let variant = self.ast_map.expect_variant(variant_id);
let enum_id = self.ast_map.get_parent(variant_id);
let enum_item = self.ast_map.expect_item(enum_id);
if let hir::ItemEnum(ref enum_def, ref generics) = enum_item.node {
self.populate_enum_discriminants(enum_def);
self.visit_variant(variant, generics, enum_id);
} else {
span_bug!(e.span,
"`check_static_recursion` found \
non-enum in Def::VariantCtor");
}
}
}
}
// For variants, we only want to check expressions that
// affect the specific variant used, but we need to check
// the whole enum definition to see what expression that
// might be (if any).
Def::VariantCtor(variant_id, CtorKind::Const) => {
if let Some(variant_id) = self.ast_map.as_local_node_id(variant_id) {
let variant = self.ast_map.expect_variant(variant_id);
let enum_id = self.ast_map.get_parent(variant_id);
let enum_item = self.ast_map.expect_item(enum_id);
if let hir::ItemEnum(ref enum_def, ref generics) = enum_item.node {
self.populate_enum_discriminants(enum_def);
self.visit_variant(variant, generics, enum_id);
} else {
span_bug!(path.span,
"`check_static_recursion` found \
non-enum in Def::VariantCtor");
}
_ => (),
}
}
_ => (),
}
intravisit::walk_expr(self, e);
intravisit::walk_path(self, path);
}
}
26 changes: 26 additions & 0 deletions src/test/compile-fail/issue-36163.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2012 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.

const A: i32 = Foo::B; //~ ERROR E0265
//~^ NOTE recursion not allowed in constant

enum Foo {
B = A, //~ ERROR E0265
//~^ NOTE recursion not allowed in constant
}

enum Bar {
C = Bar::C, //~ ERROR E0265
//~^ NOTE recursion not allowed in constant
}

const D: i32 = A;

fn main() {}