Skip to content

Commit

Permalink
auto merge of rust-lang#16452 : epdtry/rust/unreachable-item-ice, r=p…
Browse files Browse the repository at this point in the history
…cwalton

This code produces an ICE:

```rust
#![crate_type = "rlib"]
fn main() {
    if true { return }
    // remaining code is unreachable
    match () {
        () => { static MAGIC: uint = 0; }
    }
}
```
([playpen](http://is.gd/iwOISB))

The error is "encode_symbol: id not found 18", where 18 is the `NodeId` of the declaration of `MAGIC`.  The problem is that `rustc` tries to emit metadata for `MAGIC`, but some of the information is missing because `MAGIC` never gets translated by `trans_item` - the entire body of the `match` gets skipped because the `match` itself is unreachable.

This branch simplifies the handling of inner items by always processing them using the `trans_item` visitor, instead of sometimes using the visitor and sometimes waiting until `trans_stmt` encounters the item.  This fixes the ICE by making the translation of the item no longer depend on the declaration being reachable code.  This branch also reverts rust-lang#16059 and rust-lang#16359, since the new change to item translation fixes the same problems as those but is simpler.
  • Loading branch information
bors committed Aug 13, 2014
2 parents 51c7e20 + 0f847ba commit e189122
Show file tree
Hide file tree
Showing 11 changed files with 34 additions and 65 deletions.
36 changes: 14 additions & 22 deletions src/librustc/middle/trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1343,8 +1343,7 @@ pub fn new_fn_ctxt<'a>(ccx: &'a CrateContext,
output_type: ty::t,
param_substs: &'a param_substs,
sp: Option<Span>,
block_arena: &'a TypedArena<Block<'a>>,
handle_items: HandleItemsFlag)
block_arena: &'a TypedArena<Block<'a>>)
-> FunctionContext<'a> {
param_substs.validate();

Expand Down Expand Up @@ -1379,8 +1378,7 @@ pub fn new_fn_ctxt<'a>(ccx: &'a CrateContext,
block_arena: block_arena,
ccx: ccx,
debug_context: debug_context,
scopes: RefCell::new(Vec::new()),
handle_items: handle_items,
scopes: RefCell::new(Vec::new())
};

if has_env {
Expand Down Expand Up @@ -1708,8 +1706,7 @@ pub fn trans_closure(ccx: &CrateContext,
abi: Abi,
has_env: bool,
is_unboxed_closure: IsUnboxedClosureFlag,
maybe_load_env: <'a> |&'a Block<'a>| -> &'a Block<'a>,
handle_items: HandleItemsFlag) {
maybe_load_env: <'a> |&'a Block<'a>| -> &'a Block<'a>) {
ccx.stats.n_closures.set(ccx.stats.n_closures.get() + 1);

let _icx = push_ctxt("trans_closure");
Expand All @@ -1726,8 +1723,7 @@ pub fn trans_closure(ccx: &CrateContext,
output_type,
param_substs,
Some(body.span),
&arena,
handle_items);
&arena);
let mut bcx = init_function(&fcx, false, output_type);

// cleanup scope for the incoming arguments
Expand Down Expand Up @@ -1836,8 +1832,7 @@ pub fn trans_fn(ccx: &CrateContext,
llfndecl: ValueRef,
param_substs: &param_substs,
id: ast::NodeId,
attrs: &[ast::Attribute],
handle_items: HandleItemsFlag) {
attrs: &[ast::Attribute]) {
let _s = StatRecorder::new(ccx, ccx.tcx.map.path_to_string(id).to_string());
debug!("trans_fn(param_substs={})", param_substs.repr(ccx.tcx()));
let _icx = push_ctxt("trans_fn");
Expand All @@ -1857,8 +1852,7 @@ pub fn trans_fn(ccx: &CrateContext,
abi,
false,
NotUnboxedClosure,
|bcx| bcx,
handle_items);
|bcx| bcx);
}

pub fn trans_enum_variant(ccx: &CrateContext,
Expand Down Expand Up @@ -1964,7 +1958,7 @@ fn trans_enum_variant_or_tuple_like_struct(ccx: &CrateContext,

let arena = TypedArena::new();
let fcx = new_fn_ctxt(ccx, llfndecl, ctor_id, false, result_ty,
param_substs, None, &arena, TranslateItems);
param_substs, None, &arena);
let bcx = init_function(&fcx, false, result_ty);

assert!(!fcx.needs_ret_allocas);
Expand Down Expand Up @@ -2066,24 +2060,22 @@ pub fn trans_item(ccx: &CrateContext, item: &ast::Item) {
llfn,
&param_substs::empty(),
item.id,
None,
TranslateItems);
None);
} else {
trans_fn(ccx,
&**decl,
&**body,
llfn,
&param_substs::empty(),
item.id,
item.attrs.as_slice(),
TranslateItems);
item.attrs.as_slice());
}
} else {
// Be sure to travel more than just one layer deep to catch nested
// items in blocks and such.
let mut v = TransItemVisitor{ ccx: ccx };
v.visit_block(&**body, ());
}

// Be sure to travel more than just one layer deep to catch nested
// items in blocks and such.
let mut v = TransItemVisitor{ ccx: ccx };
v.visit_block(&**body, ());
}
ast::ItemImpl(ref generics, _, _, ref ms) => {
meth::trans_impl(ccx, item.ident, ms.as_slice(), generics, item.id);
Expand Down
3 changes: 1 addition & 2 deletions src/librustc/middle/trans/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,8 +339,7 @@ pub fn trans_unboxing_shim(bcx: &Block,
return_type,
&empty_param_substs,
None,
&block_arena,
TranslateItems);
&block_arena);
let mut bcx = init_function(&fcx, false, return_type);

// Create the substituted versions of the self type.
Expand Down
8 changes: 3 additions & 5 deletions src/librustc/middle/trans/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,8 +394,7 @@ pub fn trans_expr_fn<'a>(
ty::ty_fn_abi(fty),
true,
NotUnboxedClosure,
|bcx| load_environment(bcx, cdata_ty, &freevars, store),
bcx.fcx.handle_items);
|bcx| load_environment(bcx, cdata_ty, &freevars, store));
fill_fn_pair(bcx, dest_addr, llfn, llbox);
bcx
}
Expand Down Expand Up @@ -487,8 +486,7 @@ pub fn trans_unboxed_closure<'a>(
ty::ty_fn_abi(function_type),
true,
IsUnboxedClosure,
|bcx| load_unboxed_closure_environment(bcx, freevars_ptr),
bcx.fcx.handle_items);
|bcx| load_unboxed_closure_environment(bcx, freevars_ptr));

// Don't hoist this to the top of the function. It's perfectly legitimate
// to have a zero-size unboxed closure (in which case dest will be
Expand Down Expand Up @@ -575,7 +573,7 @@ pub fn get_wrapper_for_bare_fn(ccx: &CrateContext,
let arena = TypedArena::new();
let empty_param_substs = param_substs::empty();
let fcx = new_fn_ctxt(ccx, llfn, ast::DUMMY_NODE_ID, true, f.sig.output,
&empty_param_substs, None, &arena, TranslateItems);
&empty_param_substs, None, &arena);
let bcx = init_function(&fcx, true, f.sig.output);

let args = create_datums_for_fn_args(&fcx,
Expand Down
9 changes: 0 additions & 9 deletions src/librustc/middle/trans/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,12 +224,6 @@ impl<T:Subst+Clone> SubstP for T {
pub type RvalueDatum = datum::Datum<datum::Rvalue>;
pub type LvalueDatum = datum::Datum<datum::Lvalue>;

#[deriving(Clone, Eq, PartialEq)]
pub enum HandleItemsFlag {
IgnoreItems,
TranslateItems,
}

// Function context. Every LLVM function we create will have one of
// these.
pub struct FunctionContext<'a> {
Expand Down Expand Up @@ -303,9 +297,6 @@ pub struct FunctionContext<'a> {

// Cleanup scopes.
pub scopes: RefCell<Vec<cleanup::CleanupScope<'a>> >,

// How to handle items encountered during translation of this function.
pub handle_items: HandleItemsFlag,
}

impl<'a> FunctionContext<'a> {
Expand Down
8 changes: 2 additions & 6 deletions src/librustc/middle/trans/controlflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,8 @@ pub fn trans_stmt<'a>(cx: &'a Block<'a>,
debuginfo::create_local_var_metadata(bcx, &**local);
}
}
ast::DeclItem(ref i) => {
match fcx.handle_items {
TranslateItems => trans_item(cx.fcx.ccx, &**i),
IgnoreItems => {}
}
}
// Inner items are visited by `trans_item`/`trans_meth`.
ast::DeclItem(_) => {},
}
}
ast::StmtMac(..) => cx.tcx().sess.bug("unexpanded macro")
Expand Down
11 changes: 4 additions & 7 deletions src/librustc/middle/trans/foreign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -577,8 +577,7 @@ pub fn trans_rust_fn_with_foreign_abi(ccx: &CrateContext,
llwrapfn: ValueRef,
param_substs: &param_substs,
id: ast::NodeId,
hash: Option<&str>,
handle_items: HandleItemsFlag) {
hash: Option<&str>) {
let _icx = push_ctxt("foreign::build_foreign_fn");

let fnty = ty::node_id_to_type(ccx.tcx(), id);
Expand All @@ -587,8 +586,7 @@ pub fn trans_rust_fn_with_foreign_abi(ccx: &CrateContext,

unsafe { // unsafe because we call LLVM operations
// Build up the Rust function (`foo0` above).
let llrustfn = build_rust_fn(ccx, decl, body, param_substs, attrs, id,
hash, handle_items);
let llrustfn = build_rust_fn(ccx, decl, body, param_substs, attrs, id, hash);

// Build up the foreign wrapper (`foo` above).
return build_wrap_fn(ccx, llrustfn, llwrapfn, &tys, mty);
Expand All @@ -600,8 +598,7 @@ pub fn trans_rust_fn_with_foreign_abi(ccx: &CrateContext,
param_substs: &param_substs,
attrs: &[ast::Attribute],
id: ast::NodeId,
hash: Option<&str>,
handle_items: HandleItemsFlag)
hash: Option<&str>)
-> ValueRef {
let _icx = push_ctxt("foreign::foreign::build_rust_fn");
let tcx = ccx.tcx();
Expand Down Expand Up @@ -633,7 +630,7 @@ pub fn trans_rust_fn_with_foreign_abi(ccx: &CrateContext,

let llfn = base::decl_internal_rust_fn(ccx, t, ps.as_slice());
base::set_llvm_fn_attrs(attrs, llfn);
base::trans_fn(ccx, decl, body, llfn, param_substs, id, [], handle_items);
base::trans_fn(ccx, decl, body, llfn, param_substs, id, []);
llfn
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/trans/glue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ fn make_generic_glue(ccx: &CrateContext,
let arena = TypedArena::new();
let empty_param_substs = param_substs::empty();
let fcx = new_fn_ctxt(ccx, llfn, ast::DUMMY_NODE_ID, false, ty::mk_nil(),
&empty_param_substs, None, &arena, TranslateItems);
&empty_param_substs, None, &arena);

let bcx = init_function(&fcx, false, ty::mk_nil());

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/trans/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ pub fn maybe_instantiate_inline(ccx: &CrateContext, fn_id: ast::DefId)
if unparameterized {
let llfn = get_item_val(ccx, mth.id);
trans_fn(ccx, &*mth.pe_fn_decl(), &*mth.pe_body(), llfn,
&param_substs::empty(), mth.id, [], TranslateItems);
&param_substs::empty(), mth.id, []);
}
local_def(mth.id)
}
Expand Down
8 changes: 3 additions & 5 deletions src/librustc/middle/trans/meth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,10 @@ pub fn trans_impl(ccx: &CrateContext,
llfn,
&param_substs::empty(),
method.id,
[],
TranslateItems);
} else {
let mut v = TransItemVisitor{ ccx: ccx };
visit::walk_method_helper(&mut v, &**method, ());
[]);
}
let mut v = TransItemVisitor{ ccx: ccx };
visit::walk_method_helper(&mut v, &**method, ());
}
}

Expand Down
10 changes: 4 additions & 6 deletions src/librustc/middle/trans/monomorphize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,9 @@ pub fn monomorphic_fn(ccx: &CrateContext,
if abi != abi::Rust {
foreign::trans_rust_fn_with_foreign_abi(
ccx, &**decl, &**body, [], d, &psubsts, fn_id.node,
Some(hash.as_slice()), IgnoreItems);
Some(hash.as_slice()));
} else {
trans_fn(ccx, &**decl, &**body, d, &psubsts, fn_id.node, [],
IgnoreItems);
trans_fn(ccx, &**decl, &**body, d, &psubsts, fn_id.node, []);
}

d
Expand Down Expand Up @@ -201,8 +200,7 @@ pub fn monomorphic_fn(ccx: &CrateContext,
ast_map::NodeMethod(mth) => {
let d = mk_lldecl(abi::Rust);
set_llvm_fn_attrs(mth.attrs.as_slice(), d);
trans_fn(ccx, &*mth.pe_fn_decl(), &*mth.pe_body(), d, &psubsts, mth.id, [],
IgnoreItems);
trans_fn(ccx, &*mth.pe_fn_decl(), &*mth.pe_body(), d, &psubsts, mth.id, []);
d
}
ast_map::NodeTraitMethod(method) => {
Expand All @@ -211,7 +209,7 @@ pub fn monomorphic_fn(ccx: &CrateContext,
let d = mk_lldecl(abi::Rust);
set_llvm_fn_attrs(mth.attrs.as_slice(), d);
trans_fn(ccx, &*mth.pe_fn_decl(), &*mth.pe_body(), d,
&psubsts, mth.id, [], IgnoreItems);
&psubsts, mth.id, []);
d
}
_ => {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/trans/reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ impl<'a, 'b> Reflector<'a, 'b> {
let empty_param_substs = param_substs::empty();
let fcx = new_fn_ctxt(ccx, llfdecl, ast::DUMMY_NODE_ID, false,
ty::mk_u64(), &empty_param_substs,
None, &arena, TranslateItems);
None, &arena);
let bcx = init_function(&fcx, false, ty::mk_u64());

// we know the return type of llfdecl is an int here, so
Expand Down

0 comments on commit e189122

Please sign in to comment.