Skip to content

Commit

Permalink
Stop writing directly to the final type/method/vtable sidetables from…
Browse files Browse the repository at this point in the history
… astconv

and from typeck, which is verboten.  We are supposed to write inference results
into the FnCtxt and then these get copied over in writeback.  Add assertions
that no inference by-products are added to this table.

Fixes rust-lang#3888
Fixes rust-lang#4036
Fixes rust-lang#4492
  • Loading branch information
nikomatsakis committed Mar 26, 2013
1 parent 6f2783d commit 3ca7c22
Show file tree
Hide file tree
Showing 13 changed files with 137 additions and 42 deletions.
2 changes: 2 additions & 0 deletions src/librustc/middle/trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1581,6 +1581,8 @@ pub fn new_fn_ctxt_w_id(ccx: @CrateContext,
param_substs: Option<@param_substs>,
sp: Option<span>) -> fn_ctxt
{
for param_substs.each |p| { p.validate(); }
debug!("new_fn_ctxt_w_id(path=%s, id=%?, impl_id=%?, \
param_substs=%s",
path_str(ccx.sess, path),
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/middle/trans/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ pub fn trans_fn_ref_with_vtables(
vtables);
let _indenter = indenter();

fail_unless!(type_params.all(|t| !ty::type_needs_infer(*t)));

// Polytype of the function item (may have type params)
let fn_tpt = ty::lookup_item_type(tcx, def_id);

Expand Down
14 changes: 14 additions & 0 deletions src/librustc/middle/trans/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,13 @@ pub struct param_substs {
self_ty: Option<ty::t>
}

pub impl param_substs {
fn validate(&self) {
for self.tys.each |t| { fail_unless!(!ty::type_needs_infer(*t)); }
for self.self_ty.each |t| { fail_unless!(!ty::type_needs_infer(*t)); }
}
}

pub fn param_substs_to_str(tcx: ty::ctxt, substs: &param_substs) -> ~str {
fmt!("param_substs {tys:%?, vtables:%?, bounds:%?}",
substs.tys.map(|t| ty_to_str(tcx, *t)),
Expand Down Expand Up @@ -1372,6 +1379,13 @@ pub fn expr_ty_adjusted(bcx: block, ex: @ast::expr) -> ty::t {
pub fn node_id_type_params(bcx: block, id: ast::node_id) -> ~[ty::t] {
let tcx = bcx.tcx();
let params = ty::node_id_to_type_params(tcx, id);

if !params.all(|t| !ty::type_needs_infer(*t)) {
bcx.sess().bug(
fmt!("Type parameters for node %d include inference types: %s",
id, str::connect(params.map(|t| bcx.ty_to_str(*t)), ",")));
}

match bcx.fcx.param_substs {
Some(substs) => {
do vec::map(params) |t| {
Expand Down
1 change: 1 addition & 0 deletions src/librustc/middle/trans/meth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ pub fn trans_static_method_callee(bcx: block,

match vtbls[bound_index] {
typeck::vtable_static(impl_did, ref rcvr_substs, rcvr_origins) => {
fail_unless!(rcvr_substs.all(|t| !ty::type_needs_infer(*t)));

let mth_id = method_with_name(bcx.ccx(), impl_did, mname);
let callee_substs = combine_impl_and_methods_tps(
Expand Down
1 change: 1 addition & 0 deletions src/librustc/middle/trans/monomorphize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ pub fn monomorphic_fn(ccx: @CrateContext,
impl_did_opt: Option<ast::def_id>,
ref_id: Option<ast::node_id>) ->
(ValueRef, bool) {
fail_unless!(real_substs.all(|t| !ty::type_needs_infer(*t)));
let _icx = ccx.insn_ctxt("monomorphic_fn");
let mut must_cast = false;
let substs = vec::map(real_substs, |t| {
Expand Down
14 changes: 5 additions & 9 deletions src/librustc/middle/typeck/astconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ use middle::ty::{ty_param_substs_and_ty};
use middle::ty;
use middle::typeck::rscope::{in_binding_rscope};
use middle::typeck::rscope::{region_scope, type_rscope, RegionError};
use middle::typeck::{CrateCtxt, write_substs_to_tcx, write_ty_to_tcx};
use middle::typeck::{CrateCtxt};

use core::result;
use core::vec;
Expand Down Expand Up @@ -186,19 +186,15 @@ pub fn ast_path_to_ty<AC:AstConv,RS:region_scope + Copy + Durable>(
self: &AC,
rscope: &RS,
did: ast::def_id,
path: @ast::path,
path_id: ast::node_id)
-> ty_param_substs_and_ty {
path: @ast::path)
-> ty_param_substs_and_ty
{
// Look up the polytype of the item and then substitute the provided types
// for any type/region parameters.
let tcx = self.tcx();
let ty::ty_param_substs_and_ty {
substs: substs,
ty: ty
} = ast_path_to_substs_and_ty(self, rscope, did, path);
write_ty_to_tcx(tcx, path_id, ty);
write_substs_to_tcx(tcx, path_id, /*bad*/copy substs.tps);

ty_param_substs_and_ty { substs: substs, ty: ty }
}

Expand Down Expand Up @@ -368,7 +364,7 @@ pub fn ast_ty_to_ty<AC:AstConv, RS:region_scope + Copy + Durable>(
};
match a_def {
ast::def_ty(did) | ast::def_struct(did) => {
ast_path_to_ty(self, rscope, did, path, id).ty
ast_path_to_ty(self, rscope, did, path).ty
}
ast::def_prim_ty(nty) => {
match nty {
Expand Down
15 changes: 11 additions & 4 deletions src/librustc/middle/typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ use middle::typeck::rscope::{RegionError};
use middle::typeck::rscope::{in_binding_rscope, region_scope, type_rscope};
use middle::typeck::rscope;
use middle::typeck::{isr_alist, lookup_def_ccx, method_map_entry};
use middle::typeck::{method_map, vtable_map};
use middle::typeck::{method_origin, method_self, method_trait, no_params};
use middle::typeck::{require_same_types};
use util::common::{block_query, indenter, loop_query};
Expand Down Expand Up @@ -160,9 +161,13 @@ pub struct SelfInfo {
pub struct inherited {
infcx: @mut infer::InferCtxt,
locals: HashMap<ast::node_id, ty::t>,

// Temporary tables:
node_types: HashMap<ast::node_id, ty::t>,
node_type_substs: HashMap<ast::node_id, ty::substs>,
adjustments: HashMap<ast::node_id, @ty::AutoAdjustment>
adjustments: HashMap<ast::node_id, @ty::AutoAdjustment>,
method_map: method_map,
vtable_map: vtable_map,
}

pub enum FnKind {
Expand Down Expand Up @@ -220,7 +225,9 @@ pub fn blank_inherited(ccx: @mut CrateCtxt) -> @inherited {
locals: HashMap(),
node_types: oldmap::HashMap(),
node_type_substs: oldmap::HashMap(),
adjustments: oldmap::HashMap()
adjustments: oldmap::HashMap(),
method_map: oldmap::HashMap(),
vtable_map: oldmap::HashMap(),
}
}

Expand Down Expand Up @@ -1357,7 +1364,7 @@ pub fn check_expr_with_unifier(fcx: @mut FnCtxt,
CheckTraitsAndInherentMethods,
AutoderefReceiver) {
Some(ref entry) => {
let method_map = fcx.ccx.method_map;
let method_map = fcx.inh.method_map;
method_map.insert(expr.id, (*entry));
}
None => {
Expand Down Expand Up @@ -1429,7 +1436,7 @@ pub fn check_expr_with_unifier(fcx: @mut FnCtxt,
deref_args, CheckTraitsOnly, autoderef_receiver) {
Some(ref origin) => {
let method_ty = fcx.node_ty(op_ex.callee_id);
let method_map = fcx.ccx.method_map;
let method_map = fcx.inh.method_map;
method_map.insert(op_ex.id, *origin);
check_call_inner(fcx, op_ex.span,
op_ex.id, method_ty,
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/middle/typeck/check/regionck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ pub fn visit_expr(expr: @ast::expr, &&rcx: @mut Rcx, v: rvt) {
// `constrain_auto_ref()` on all exprs. But that causes a
// lot of spurious errors because of how the region
// hierarchy is setup.
if rcx.fcx.ccx.method_map.contains_key(&callee.id) {
if rcx.fcx.inh.method_map.contains_key(&callee.id) {
match callee.node {
ast::expr_field(base, _, _) => {
constrain_auto_ref(rcx, base);
Expand Down Expand Up @@ -713,7 +713,7 @@ pub mod guarantor {
ast::expr_repeat(*) |
ast::expr_vec(*) => {
fail_unless!(!ty::expr_is_lval(
rcx.fcx.tcx(), rcx.fcx.ccx.method_map, expr));
rcx.fcx.tcx(), rcx.fcx.inh.method_map, expr));
None
}
}
Expand Down Expand Up @@ -765,7 +765,7 @@ pub mod guarantor {
let _i = ::util::common::indenter();

let guarantor = {
if rcx.fcx.ccx.method_map.contains_key(&expr.id) {
if rcx.fcx.inh.method_map.contains_key(&expr.id) {
None
} else {
guarantor(rcx, expr)
Expand Down
21 changes: 8 additions & 13 deletions src/librustc/middle/typeck/check/vtable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,13 +486,12 @@ pub fn connect_trait_tps(vcx: &VtableContext,
}
}

pub fn insert_vtables(ccx: @mut CrateCtxt,
pub fn insert_vtables(fcx: @mut FnCtxt,
callee_id: ast::node_id,
vtables: vtable_res) {
debug!("insert_vtables(callee_id=%d, vtables=%?)",
callee_id, vtables.map(|v| v.to_str(ccx.tcx)));
let vtable_map = ccx.vtable_map;
vtable_map.insert(callee_id, vtables);
callee_id, vtables.map(|v| v.to_str(fcx.tcx())));
fcx.inh.vtable_map.insert(callee_id, vtables);
}

pub fn location_info_for_expr(expr: @ast::expr) -> LocationInfo {
Expand Down Expand Up @@ -529,8 +528,7 @@ pub fn early_resolve_expr(ex: @ast::expr,
let vtbls = lookup_vtables(&vcx, &location_info_for_expr(ex),
item_ty.bounds, substs, is_early);
if !is_early {
let vtable_map = cx.vtable_map;
vtable_map.insert(ex.id, vtbls);
insert_vtables(fcx, ex.id, vtbls);
}
}
}
Expand All @@ -543,10 +541,10 @@ pub fn early_resolve_expr(ex: @ast::expr,
}

// Must resolve bounds on methods with bounded params
ast::expr_field(*) | ast::expr_binary(*) |
ast::expr_binary(*) |
ast::expr_unary(*) | ast::expr_assign_op(*) |
ast::expr_index(*) | ast::expr_method_call(*) => {
match ty::method_call_bounds(cx.tcx, cx.method_map, ex.id) {
match ty::method_call_bounds(cx.tcx, fcx.inh.method_map, ex.id) {
Some(bounds) => {
if has_trait_bounds(/*bad*/copy *bounds) {
let callee_id = match ex.node {
Expand All @@ -559,7 +557,7 @@ pub fn early_resolve_expr(ex: @ast::expr,
let vtbls = lookup_vtables(&vcx, &location_info_for_expr(ex),
bounds, &substs, is_early);
if !is_early {
insert_vtables(cx, callee_id, vtbls);
insert_vtables(fcx, callee_id, vtbls);
}
}
}
Expand Down Expand Up @@ -599,10 +597,7 @@ pub fn early_resolve_expr(ex: @ast::expr,
// vtable (that is: "ex has vtable
// <vtable>")
if !is_early {
let vtable_map =
cx.vtable_map;
vtable_map.insert(ex.id,
@~[vtable]);
insert_vtables(fcx, ex.id, @~[vtable]);
}
}
None => {
Expand Down
62 changes: 52 additions & 10 deletions src/librustc/middle/typeck/check/writeback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ use middle::typeck::check::{FnCtxt, SelfInfo};
use middle::typeck::infer::{force_all, resolve_all, resolve_region};
use middle::typeck::infer::{resolve_type};
use middle::typeck::infer;
use middle::typeck::method_map_entry;
use middle::typeck::{method_map_entry};
use middle::typeck::{vtable_origin, vtable_static, vtable_param};
use middle::typeck::{vtable_param, write_substs_to_tcx};
use middle::typeck::{write_ty_to_tcx};
use util::ppaux;
Expand Down Expand Up @@ -51,21 +52,60 @@ fn resolve_type_vars_in_type(fcx: @mut FnCtxt, sp: span, typ: ty::t)
}
}

fn resolve_type_vars_in_types(fcx: @mut FnCtxt, sp: span, tys: &[ty::t])
-> ~[ty::t] {
tys.map(|t| {
match resolve_type_vars_in_type(fcx, sp, *t) {
Some(t1) => t1,
None => ty::mk_err(fcx.ccx.tcx)
}
})
}

fn resolve_method_map_entry(fcx: @mut FnCtxt, sp: span, id: ast::node_id) {
// Resolve any method map entry
match fcx.ccx.method_map.find(&id) {
match fcx.inh.method_map.find(&id) {
None => {}
Some(ref mme) => {
for resolve_type_vars_in_type(fcx, sp, mme.self_arg.ty).each |t| {
let method_map = fcx.ccx.method_map;
method_map.insert(id,
method_map_entry {
self_arg: arg {
mode: mme.self_arg.mode,
ty: *t
},
.. *mme
});
let new_entry = method_map_entry {
self_arg: arg {mode: mme.self_arg.mode, ty: *t },
..*mme
};
debug!("writeback::resolve_method_map_entry(id=%?, \
new_entry=%?)",
id, new_entry);
method_map.insert(id, new_entry);
}
}
}
}

fn resolve_vtable_map_entry(fcx: @mut FnCtxt, sp: span, id: ast::node_id) {
// Resolve any method map entry
match fcx.inh.vtable_map.find(&id) {
None => {}
Some(origins) => {
let r_origins = @origins.map(|o| resolve_origin(fcx, sp, o));
let vtable_map = fcx.ccx.vtable_map;
vtable_map.insert(id, r_origins);
debug!("writeback::resolve_vtable_map_entry(id=%d, vtables=%?)",
id, r_origins.map(|v| v.to_str(fcx.tcx())));
}
}

fn resolve_origin(fcx: @mut FnCtxt,
sp: span,
origin: &vtable_origin) -> vtable_origin {
match origin {
&vtable_static(def_id, ref tys, origins) => {
let r_tys = resolve_type_vars_in_types(fcx, sp, *tys);
let r_origins = @origins.map(|o| resolve_origin(fcx, sp, o));
vtable_static(def_id, r_tys, r_origins)
}
&vtable_param(n, b) => {
vtable_param(n, b)
}
}
}
Expand Down Expand Up @@ -185,6 +225,8 @@ fn visit_expr(e: @ast::expr, &&wbcx: @mut WbCtxt, v: wb_vt) {
resolve_type_vars_for_node(wbcx, e.span, e.id);
resolve_method_map_entry(wbcx.fcx, e.span, e.id);
resolve_method_map_entry(wbcx.fcx, e.span, e.callee_id);
resolve_vtable_map_entry(wbcx.fcx, e.span, e.id);
resolve_vtable_map_entry(wbcx.fcx, e.span, e.callee_id);
match e.node {
ast::expr_fn_block(ref decl, _) => {
for vec::each(decl.inputs) |input| {
Expand Down
9 changes: 6 additions & 3 deletions src/librustc/middle/typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ use middle::typeck::astconv;
use middle::typeck::infer;
use middle::typeck::rscope::*;
use middle::typeck::rscope;
use middle::typeck::{CrateCtxt, lookup_def_tcx, no_params, write_ty_to_tcx};
use middle::typeck::{CrateCtxt, lookup_def_tcx, no_params, write_ty_to_tcx,
write_tpt_to_tcx};
use util::common::{indenter, pluralize};
use util::ppaux;

Expand Down Expand Up @@ -807,8 +808,10 @@ pub fn instantiate_trait_ref(ccx: &CrateCtxt, t: @ast::trait_ref,

match lookup_def_tcx(ccx.tcx, t.path.span, t.ref_id) {
ast::def_ty(t_id) => {
let tpt = astconv::ast_path_to_ty(ccx, &rscope, t_id, t.path,
t.ref_id);
let tpt = astconv::ast_path_to_ty(ccx, &rscope, t_id, t.path);

write_tpt_to_tcx(ccx.tcx, t.ref_id, &tpt);

match ty::get(tpt.ty).sty {
ty::ty_trait(*) => {
(t_id, tpt)
Expand Down
11 changes: 11 additions & 0 deletions src/librustc/middle/typeck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ pub enum vtable_origin {
vtable_res is the vtable itself
*/
vtable_static(ast::def_id, ~[ty::t], vtable_res),

/*
Dynamic vtable, comes from a parameter that has a bound on it:
fn foo<T:quux,baz,bar>(a: T) -- a's vtable would have a
Expand Down Expand Up @@ -184,6 +185,7 @@ pub struct CrateCtxt {
// Functions that write types into the node type table
pub fn write_ty_to_tcx(tcx: ty::ctxt, node_id: ast::node_id, ty: ty::t) {
debug!("write_ty_to_tcx(%d, %s)", node_id, ppaux::ty_to_str(tcx, ty));
fail_unless!(!ty::type_needs_infer(ty));
tcx.node_types.insert(node_id as uint, ty);
}
pub fn write_substs_to_tcx(tcx: ty::ctxt,
Expand All @@ -192,9 +194,18 @@ pub fn write_substs_to_tcx(tcx: ty::ctxt,
if substs.len() > 0u {
debug!("write_substs_to_tcx(%d, %?)", node_id,
substs.map(|t| ppaux::ty_to_str(tcx, *t)));
fail_unless!(substs.all(|t| !ty::type_needs_infer(*t)));
tcx.node_type_substs.insert(node_id, substs);
}
}
pub fn write_tpt_to_tcx(tcx: ty::ctxt,
node_id: ast::node_id,
tpt: &ty::ty_param_substs_and_ty) {
write_ty_to_tcx(tcx, node_id, tpt.ty);
if !tpt.substs.tps.is_empty() {
write_substs_to_tcx(tcx, node_id, copy tpt.substs.tps);
}
}

pub fn lookup_def_tcx(tcx: ty::ctxt, sp: span, id: ast::node_id) -> ast::def {
match tcx.def_map.find(&id) {
Expand Down
Loading

1 comment on commit 3ca7c22

@nikomatsakis
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+

Please sign in to comment.