From e11d13f3de084fe73d754ca206ad70737bb5957d Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 20 Mar 2013 21:28:30 -0400 Subject: [PATCH 1/3] Rip out old code that still structured method calls as a expr_call(expr_field(...)) rather than an expr_method_call. There is probably more such code in trans that should be removed. --- src/librustc/middle/typeck/check/mod.rs | 62 +++++-------- src/libsyntax/ext/auto_encode.rs | 111 +++++++++-------------- src/libsyntax/ext/build.rs | 7 ++ src/libsyntax/ext/deriving/clone.rs | 5 +- src/libsyntax/ext/deriving/eq.rs | 8 +- src/libsyntax/ext/deriving/iter_bytes.rs | 13 +-- src/libsyntax/ext/quote.rs | 68 +++++++------- 7 files changed, 114 insertions(+), 160 deletions(-) diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index 12c88ae7198d..ea5301af9b29 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/src/librustc/middle/typeck/check/mod.rs @@ -1321,13 +1321,7 @@ pub fn check_expr_with_unifier(fcx: @mut FnCtxt, sugar: ast::CallSugar) { // Index expressions need to be handled separately, to inform them // that they appear in call position. - match f.node { - ast::expr_field(ref base, ref field, ref tys) => { - check_field(fcx, f, true, *base, *field, *tys) - } - _ => check_expr(fcx, f) - }; - + let mut bot = check_expr(fcx, f); check_call_or_method(fcx, sp, call_expr_id, @@ -1689,7 +1683,6 @@ pub fn check_expr_with_unifier(fcx: @mut FnCtxt, // Check field access expressions fn check_field(fcx: @mut FnCtxt, expr: @ast::expr, - is_callee: bool, base: @ast::expr, field: ast::ident, tys: &[@ast::Ty]) { @@ -1723,7 +1716,6 @@ pub fn check_expr_with_unifier(fcx: @mut FnCtxt, } let tps = vec::map(tys, |ty| fcx.to_ty(*ty)); - match method::lookup(fcx, expr, base, @@ -1734,34 +1726,30 @@ pub fn check_expr_with_unifier(fcx: @mut FnCtxt, DontDerefArgs, CheckTraitsAndInherentMethods, AutoderefReceiver) { - Some(ref entry) => { - let method_map = fcx.ccx.method_map; - method_map.insert(expr.id, (*entry)); - - // If we have resolved to a method but this is not in - // a callee position, error - if !is_callee { - tcx.sess.span_err( - expr.span, - ~"attempted to take value of method \ - (try writing an anonymous function)"); - // Add error type for the result - fcx.write_error(expr.id); - } + Some(_) => { + fcx.type_error_message( + expr.span, + |actual| { + fmt!("attempted to take value of method `%s` on type `%s` \ + (try writing an anonymous function)", + *tcx.sess.str_of(field), actual) + }, + expr_t, None); } + None => { - fcx.type_error_message(expr.span, - |actual| { - fmt!("attempted access of field `%s` on type `%s`, but \ - no field or method with that name was found", - *tcx.sess.str_of(field), actual) - }, - expr_t, None); - // Add error type for the result - fcx.write_error(expr.id); + fcx.type_error_message( + expr.span, + |actual| { + fmt!("attempted access of field `%s` on type `%s`, \ + but no field with that name was found", + *tcx.sess.str_of(field), actual) + }, + expr_t, None); } } + fcx.write_error(expr.id); } fn check_struct_or_variant_fields(fcx: @mut FnCtxt, @@ -2750,15 +2738,7 @@ pub fn check_expr_with_unifier(fcx: @mut FnCtxt, } } ast::expr_field(base, field, ref tys) => { - check_field(fcx, expr, false, base, field, * tys); - let base_t = fcx.expr_ty(base); - if ty::type_is_error(base_t) { - fcx.write_error(id); - } - else if ty::type_is_bot(base_t) { - fcx.write_bot(id); - } - // Otherwise, type already got written + check_field(fcx, expr, base, field, *tys); } ast::expr_index(base, idx) => { check_expr(fcx, base); diff --git a/src/libsyntax/ext/auto_encode.rs b/src/libsyntax/ext/auto_encode.rs index 54ca5dc0d72f..e81e460e832e 100644 --- a/src/libsyntax/ext/auto_encode.rs +++ b/src/libsyntax/ext/auto_encode.rs @@ -416,6 +416,16 @@ priv impl @ext_ctxt { self.expr(span, ast::expr_call(expr, args, ast::NoSugar)) } + fn expr_method_call( + &self, + span: span, + expr: @ast::expr, + ident: ast::ident, + +args: ~[@ast::expr] + ) -> @ast::expr { + self.expr(span, ast::expr_method_call(expr, ident, ~[], args, ast::NoSugar)) + } + fn lambda_expr(&self, expr: @ast::expr) -> @ast::expr { self.lambda(self.expr_blk(expr)) } @@ -712,30 +722,24 @@ fn mk_struct_ser_impl( let fields = do mk_struct_fields(fields).mapi |idx, field| { // ast for `|| self.$(name).encode(__s)` let expr_lambda = cx.lambda_expr( - cx.expr_call( + cx.expr_method_call( span, cx.expr_field( span, - cx.expr_field( - span, - cx.expr_var(span, ~"self"), - field.ident - ), - cx.ident_of(~"encode") + cx.expr_var(span, ~"self"), + field.ident ), + cx.ident_of(~"encode"), ~[cx.expr_var(span, ~"__s")] ) ); // ast for `__s.emit_field($(name), $(idx), $(expr_lambda))` cx.stmt( - cx.expr_call( + cx.expr_method_call( span, - cx.expr_field( - span, - cx.expr_var(span, ~"__s"), - cx.ident_of(~"emit_field") - ), + cx.expr_var(span, ~"__s"), + cx.ident_of(~"emit_field"), ~[ cx.lit_str(span, @cx.str_of(field.ident)), cx.lit_uint(span, idx), @@ -746,13 +750,10 @@ fn mk_struct_ser_impl( }; // ast for `__s.emit_struct($(name), || $(fields))` - let ser_body = cx.expr_call( + let ser_body = cx.expr_method_call( span, - cx.expr_field( - span, - cx.expr_var(span, ~"__s"), - cx.ident_of(~"emit_struct") - ), + cx.expr_var(span, ~"__s"), + cx.ident_of(~"emit_struct"), ~[ cx.lit_str(span, @cx.str_of(ident)), cx.lit_uint(span, vec::len(fields)), @@ -788,13 +789,10 @@ fn mk_struct_deser_impl( ); // ast for `__d.read_field($(name), $(idx), $(expr_lambda))` - let expr: @ast::expr = cx.expr_call( + let expr: @ast::expr = cx.expr_method_call( span, - cx.expr_field( - span, - cx.expr_var(span, ~"__d"), - cx.ident_of(~"read_field") - ), + cx.expr_var(span, ~"__d"), + cx.ident_of(~"read_field"), ~[ cx.lit_str(span, @cx.str_of(field.ident)), cx.lit_uint(span, idx), @@ -813,13 +811,10 @@ fn mk_struct_deser_impl( }; // ast for `read_struct($(name), || $(fields))` - let body = cx.expr_call( + let body = cx.expr_method_call( span, - cx.expr_field( - span, - cx.expr_var(span, ~"__d"), - cx.ident_of(~"read_struct") - ), + cx.expr_var(span, ~"__d"), + cx.ident_of(~"read_struct"), ~[ cx.lit_str(span, @cx.str_of(ident)), cx.lit_uint(span, vec::len(fields)), @@ -943,13 +938,10 @@ fn ser_variant( // ast for `|| $(v).encode(__s)` let expr_encode = cx.lambda_expr( - cx.expr_call( + cx.expr_method_call( span, - cx.expr_field( - span, - cx.expr_path(span, ~[names[a_idx]]), - cx.ident_of(~"encode") - ), + cx.expr_path(span, ~[names[a_idx]]), + cx.ident_of(~"encode"), ~[cx.expr_var(span, ~"__s")] ) ); @@ -965,13 +957,10 @@ fn ser_variant( }; // ast for `__s.emit_enum_variant($(name), $(idx), $(sz), $(lambda))` - let body = cx.expr_call( + let body = cx.expr_method_call( span, - cx.expr_field( - span, - cx.expr_var(span, ~"__s"), - cx.ident_of(~"emit_enum_variant") - ), + cx.expr_var(span, ~"__s"), + cx.ident_of(~"emit_enum_variant"), ~[ cx.lit_str(span, @cx.str_of(v_name)), cx.lit_uint(span, v_idx), @@ -1019,13 +1008,10 @@ fn mk_enum_ser_body( ); // ast for `__s.emit_enum($(name), || $(match_expr))` - cx.expr_call( + cx.expr_method_call( span, - cx.expr_field( - span, - cx.expr_var(span, ~"__s"), - cx.ident_of(~"emit_enum") - ), + cx.expr_var(span, ~"__s"), + cx.ident_of(~"emit_enum"), ~[ cx.lit_str(span, @cx.str_of(name)), cx.lambda_expr(match_expr), @@ -1055,13 +1041,10 @@ fn mk_enum_deser_variant_nary( ); // ast for `__d.read_enum_variant_arg($(a_idx), $(expr_lambda))` - cx.expr_call( + cx.expr_method_call( span, - cx.expr_field( - span, - cx.expr_var(span, ~"__d"), - cx.ident_of(~"read_enum_variant_arg") - ), + cx.expr_var(span, ~"__d"), + cx.ident_of(~"read_enum_variant_arg"), ~[cx.lit_uint(span, idx), expr_lambda] ) }; @@ -1171,25 +1154,19 @@ fn mk_enum_deser_body( // ast for `__d.read_enum_variant($(expr_lambda))` let expr_lambda = ext_cx.lambda_expr( - ext_cx.expr_call( + ext_cx.expr_method_call( span, - ext_cx.expr_field( - span, - ext_cx.expr_var(span, ~"__d"), - ext_cx.ident_of(~"read_enum_variant") - ), + ext_cx.expr_var(span, ~"__d"), + ext_cx.ident_of(~"read_enum_variant"), ~[expr_lambda] ) ); // ast for `__d.read_enum($(e_name), $(expr_lambda))` - ext_cx.expr_call( + ext_cx.expr_method_call( span, - ext_cx.expr_field( - span, - ext_cx.expr_var(span, ~"__d"), - ext_cx.ident_of(~"read_enum") - ), + ext_cx.expr_var(span, ~"__d"), + ext_cx.ident_of(~"read_enum"), ~[ ext_cx.lit_str(span, @ext_cx.str_of(name)), expr_lambda diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index c2f4cbf3db24..ad71441e0466 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -111,6 +111,13 @@ pub fn mk_addr_of(cx: @ext_ctxt, sp: span, e: @ast::expr) -> @ast::expr { pub fn mk_mut_addr_of(cx: @ext_ctxt, sp: span, e: @ast::expr) -> @ast::expr { return mk_expr(cx, sp, ast::expr_addr_of(ast::m_mutbl, e)); } +pub fn mk_method_call(cx: @ext_ctxt, + sp: span, + rcvr_expr: @ast::expr, + method_ident: ast::ident, + +args: ~[@ast::expr]) -> @ast::expr { + mk_expr(cx, sp, ast::expr_method_call(rcvr_expr, method_ident, ~[], args, ast::NoSugar)) +} pub fn mk_call_(cx: @ext_ctxt, sp: span, fn_expr: @ast::expr, +args: ~[@ast::expr]) -> @ast::expr { mk_expr(cx, sp, ast::expr_call(fn_expr, args, ast::NoSugar)) diff --git a/src/libsyntax/ext/deriving/clone.rs b/src/libsyntax/ext/deriving/clone.rs index 4ab83cb5f1e4..68458bd516ee 100644 --- a/src/libsyntax/ext/deriving/clone.rs +++ b/src/libsyntax/ext/deriving/clone.rs @@ -120,8 +120,9 @@ fn call_substructure_clone_method(cx: @ext_ctxt, -> @expr { // Call the substructure method. let clone_ident = cx.ident_of(~"clone"); - let self_method = build::mk_access_(cx, span, self_field, clone_ident); - build::mk_call_(cx, span, self_method, ~[]) + build::mk_method_call(cx, span, + self_field, clone_ident, + ~[]) } fn expand_deriving_clone_struct_def(cx: @ext_ctxt, diff --git a/src/libsyntax/ext/deriving/eq.rs b/src/libsyntax/ext/deriving/eq.rs index 5e94134f70af..8cee6bd7290b 100644 --- a/src/libsyntax/ext/deriving/eq.rs +++ b/src/libsyntax/ext/deriving/eq.rs @@ -147,11 +147,9 @@ fn call_substructure_eq_method(cx: @ext_ctxt, junction: Junction, chain_expr: &mut Option<@expr>) { // Call the substructure method. - let self_method = build::mk_access_(cx, span, self_field, method_ident); - let self_call = build::mk_call_(cx, - span, - self_method, - ~[ other_field_ref ]); + let self_call = build::mk_method_call(cx, span, + self_field, method_ident, + ~[ other_field_ref ]); // Connect to the outer expression if necessary. *chain_expr = match *chain_expr { diff --git a/src/libsyntax/ext/deriving/iter_bytes.rs b/src/libsyntax/ext/deriving/iter_bytes.rs index 75d7b396c7e1..3c1ee7e02964 100644 --- a/src/libsyntax/ext/deriving/iter_bytes.rs +++ b/src/libsyntax/ext/deriving/iter_bytes.rs @@ -125,14 +125,11 @@ fn call_substructure_iter_bytes_method(cx: @ext_ctxt, // Call the substructure method. let iter_bytes_ident = cx.ident_of(~"iter_bytes"); - let self_method = build::mk_access_(cx, - span, - self_field, - iter_bytes_ident); - let self_call = build::mk_call_(cx, - span, - self_method, - ~[ lsb0_expr, f_expr ]); + let self_call = build::mk_method_call(cx, + span, + self_field, + iter_bytes_ident, + ~[ lsb0_expr, f_expr ]); // Create a statement out of this expression. build::mk_stmt(cx, span, self_call) diff --git a/src/libsyntax/ext/quote.rs b/src/libsyntax/ext/quote.rs index 2cf52c47959b..6044c3ad3d24 100644 --- a/src/libsyntax/ext/quote.rs +++ b/src/libsyntax/ext/quote.rs @@ -270,11 +270,11 @@ fn id_ext(cx: @ext_ctxt, +str: ~str) -> ast::ident { // Lift an ident to the expr that evaluates to that ident. fn mk_ident(cx: @ext_ctxt, sp: span, ident: ast::ident) -> @ast::expr { - let e_meth = build::mk_access(cx, sp, - ids_ext(cx, ~[~"ext_cx"]), - id_ext(cx, ~"ident_of")); let e_str = build::mk_uniq_str(cx, sp, cx.str_of(ident)); - build::mk_call_(cx, sp, e_meth, ~[e_str]) + build::mk_method_call(cx, sp, + build::mk_path(cx, sp, ids_ext(cx, ~[~"ext_cx"])), + id_ext(cx, ~"ident_of"), + ~[e_str]) } fn mk_bytepos(cx: @ext_ctxt, sp: span, bpos: BytePos) -> @ast::expr { @@ -462,11 +462,10 @@ fn mk_tt(cx: @ext_ctxt, sp: span, tt: &ast::token_tree) ids_ext(cx, ~[~"tt_tok"]), ~[e_sp, mk_token(cx, sp, *tok)]); let e_push = - build::mk_call_(cx, sp, - build::mk_access(cx, sp, - ids_ext(cx, ~[~"tt"]), - id_ext(cx, ~"push")), - ~[e_tok]); + build::mk_method_call(cx, sp, + build::mk_path(cx, sp, ids_ext(cx, ~[~"tt"])), + id_ext(cx, ~"push"), + ~[e_tok]); ~[build::mk_stmt(cx, sp, e_push)] } @@ -479,21 +478,17 @@ fn mk_tt(cx: @ext_ctxt, sp: span, tt: &ast::token_tree) // tt.push_all_move($ident.to_tokens(ext_cx)) let e_to_toks = - build::mk_call_(cx, sp, - build::mk_access - (cx, sp, - ~[ident], - id_ext(cx, ~"to_tokens")), - ~[build::mk_path(cx, sp, - ids_ext(cx, ~[~"ext_cx"]))]); + build::mk_method_call(cx, sp, + build::mk_path(cx, sp, ~[ident]), + id_ext(cx, ~"to_tokens"), + ~[build::mk_path(cx, sp, + ids_ext(cx, ~[~"ext_cx"]))]); let e_push = - build::mk_call_(cx, sp, - build::mk_access - (cx, sp, - ids_ext(cx, ~[~"tt"]), - id_ext(cx, ~"push_all_move")), - ~[e_to_toks]); + build::mk_method_call(cx, sp, + build::mk_path(cx, sp, ids_ext(cx, ~[~"tt"])), + id_ext(cx, ~"push_all_move"), + ~[e_to_toks]); ~[build::mk_stmt(cx, sp, e_push)] } @@ -562,11 +557,10 @@ fn expand_tts(cx: @ext_ctxt, // of quotes, for example) but at this point it seems not likely to be // worth the hassle. - let e_sp = build::mk_call_(cx, sp, - build::mk_access(cx, sp, - ids_ext(cx, ~[~"ext_cx"]), - id_ext(cx, ~"call_site")), - ~[]); + let e_sp = build::mk_method_call(cx, sp, + build::mk_path(cx, sp, ids_ext(cx, ~[~"ext_cx"])), + id_ext(cx, ~"call_site"), + ~[]); let stmt_let_sp = build::mk_local(cx, sp, false, id_ext(cx, ~"sp"), @@ -590,13 +584,13 @@ fn expand_parse_call(cx: @ext_ctxt, tts: &[ast::token_tree]) -> @ast::expr { let tts_expr = expand_tts(cx, sp, tts); - let cfg_call = || build::mk_call_( - cx, sp, build::mk_access(cx, sp, ids_ext(cx, ~[~"ext_cx"]), - id_ext(cx, ~"cfg")), ~[]); + let cfg_call = || build::mk_method_call( + cx, sp, build::mk_path(cx, sp, ids_ext(cx, ~[~"ext_cx"])), + id_ext(cx, ~"cfg"), ~[]); - let parse_sess_call = || build::mk_call_( - cx, sp, build::mk_access(cx, sp, ids_ext(cx, ~[~"ext_cx"]), - id_ext(cx, ~"parse_sess")), ~[]); + let parse_sess_call = || build::mk_method_call( + cx, sp, build::mk_path(cx, sp, ids_ext(cx, ~[~"ext_cx"])), + id_ext(cx, ~"parse_sess"), ~[]); let new_parser_call = build::mk_call_global(cx, sp, @@ -609,9 +603,9 @@ fn expand_parse_call(cx: @ext_ctxt, cfg_call(), tts_expr]); - build::mk_call_(cx, sp, - build::mk_access_(cx, sp, new_parser_call, - id_ext(cx, parse_method)), - arg_exprs) + build::mk_method_call(cx, sp, + new_parser_call, + id_ext(cx, parse_method), + arg_exprs) } From 6f2783d5151cb8e9ee3aa9ab591b65847042cf1b Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 21 Mar 2013 08:33:52 -0400 Subject: [PATCH 2/3] Add various debug statements to trans that I used to help track down the problem and which seem like they could be useful in the future. --- src/librustc/middle/trans/base.rs | 10 +++++++++- src/librustc/middle/trans/callee.rs | 2 +- src/librustc/middle/trans/common.rs | 7 ++++++- src/librustc/middle/trans/meth.rs | 5 +++++ src/librustc/middle/trans/monomorphize.rs | 6 ++++-- 5 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 62fff5837666..ebeab857b19b 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -1579,7 +1579,15 @@ pub fn new_fn_ctxt_w_id(ccx: @CrateContext, id: ast::node_id, impl_id: Option, param_substs: Option<@param_substs>, - sp: Option) -> fn_ctxt { + sp: Option) -> fn_ctxt +{ + debug!("new_fn_ctxt_w_id(path=%s, id=%?, impl_id=%?, \ + param_substs=%s", + path_str(ccx.sess, path), + id, + impl_id, + opt_param_substs_to_str(ccx.tcx, ¶m_substs)); + let llbbs = mk_standard_basic_blocks(llfndecl); return @mut fn_ctxt_ { llfn: llfndecl, diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs index 607d4cff8d6a..9916af6fab09 100644 --- a/src/librustc/middle/trans/callee.rs +++ b/src/librustc/middle/trans/callee.rs @@ -217,7 +217,7 @@ pub fn trans_fn_ref_with_vtables( // - `type_params`: values for each of the fn/method's type parameters // - `vtables`: values for each bound on each of the type parameters - let _icx = bcx.insn_ctxt("trans_fn_with_vtables"); + let _icx = bcx.insn_ctxt("trans_fn_ref_with_vtables"); let ccx = bcx.ccx(); let tcx = ccx.tcx; diff --git a/src/librustc/middle/trans/common.rs b/src/librustc/middle/trans/common.rs index 0b7d6f5c39bf..625738ab1b9f 100644 --- a/src/librustc/middle/trans/common.rs +++ b/src/librustc/middle/trans/common.rs @@ -257,6 +257,11 @@ pub fn param_substs_to_str(tcx: ty::ctxt, substs: ¶m_substs) -> ~str { substs.bounds.map(|b| ty::param_bounds_to_str(tcx, *b))) } +pub fn opt_param_substs_to_str(tcx: ty::ctxt, + substs: &Option<@param_substs>) -> ~str { + substs.map_default(~"None", |&ps| param_substs_to_str(tcx, ps)) +} + // Function context. Every LLVM function we create will have one of // these. pub struct fn_ctxt_ { @@ -1423,7 +1428,7 @@ pub fn resolve_vtable_in_fn_ctxt(fcx: fn_ctxt, +vt: typeck::vtable_origin) } pub fn find_vtable(tcx: ty::ctxt, ps: ¶m_substs, - n_param: uint, n_bound: uint) + n_param: uint, n_bound: uint) -> typeck::vtable_origin { debug!("find_vtable_in_fn_ctxt(n_param=%u, n_bound=%u, ps=%?)", n_param, n_bound, param_substs_to_str(tcx, ps)); diff --git a/src/librustc/middle/trans/meth.rs b/src/librustc/middle/trans/meth.rs index ce6f6c8efa70..2ac8742f6e07 100644 --- a/src/librustc/middle/trans/meth.rs +++ b/src/librustc/middle/trans/meth.rs @@ -175,6 +175,9 @@ pub fn trans_method_callee(bcx: block, -> Callee { let _icx = bcx.insn_ctxt("impl::trans_method_callee"); + debug!("trans_method_callee(callee_id=%?, self=%s, mentry=%?)", + callee_id, bcx.expr_to_str(self), mentry); + // Replace method_self with method_static here. let mut origin = mentry.origin; match origin { @@ -218,6 +221,8 @@ pub fn trans_method_callee(bcx: block, typeck::method_trait(*) => {} } + debug!("origin=%?", origin); + match origin { typeck::method_static(did) => { let callee_fn = callee::trans_fn_ref(bcx, did, callee_id); diff --git a/src/librustc/middle/trans/monomorphize.rs b/src/librustc/middle/trans/monomorphize.rs index feddbabdcad3..a38c82d37c40 100644 --- a/src/librustc/middle/trans/monomorphize.rs +++ b/src/librustc/middle/trans/monomorphize.rs @@ -67,9 +67,11 @@ pub fn monomorphic_fn(ccx: @CrateContext, must_cast = true; } - debug!("monomorphic_fn(fn_id=%? (%s), real_substs=%?, substs=%?, \ - hash_id = %?", + debug!("monomorphic_fn(fn_id=%? (%s), vtables=%?, \ + real_substs=%?, substs=%?, \ + hash_id = %?", fn_id, ty::item_path_str(ccx.tcx, fn_id), + vtables, real_substs.map(|s| ty_to_str(ccx.tcx, *s)), substs.map(|s| ty_to_str(ccx.tcx, *s)), hash_id); From 3ca7c225e5e2c907393e7e87660509cf877bffc8 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 21 Mar 2013 08:34:18 -0400 Subject: [PATCH 3/3] Stop writing directly to the final type/method/vtable sidetables from 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 #3888 Fixes #4036 Fixes #4492 --- src/librustc/middle/trans/base.rs | 2 + src/librustc/middle/trans/callee.rs | 2 + src/librustc/middle/trans/common.rs | 14 +++++ src/librustc/middle/trans/meth.rs | 1 + src/librustc/middle/trans/monomorphize.rs | 1 + src/librustc/middle/typeck/astconv.rs | 14 ++--- src/librustc/middle/typeck/check/mod.rs | 15 +++-- src/librustc/middle/typeck/check/regionck.rs | 6 +- src/librustc/middle/typeck/check/vtable.rs | 21 +++---- src/librustc/middle/typeck/check/writeback.rs | 62 ++++++++++++++++--- src/librustc/middle/typeck/collect.rs | 9 ++- src/librustc/middle/typeck/mod.rs | 11 ++++ src/test/run-pass/issue-4036.rs | 21 +++++++ 13 files changed, 137 insertions(+), 42 deletions(-) create mode 100644 src/test/run-pass/issue-4036.rs diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index ebeab857b19b..1a5b01feb0af 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -1581,6 +1581,8 @@ pub fn new_fn_ctxt_w_id(ccx: @CrateContext, param_substs: Option<@param_substs>, sp: Option) -> 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), diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs index 9916af6fab09..8c5729637ac5 100644 --- a/src/librustc/middle/trans/callee.rs +++ b/src/librustc/middle/trans/callee.rs @@ -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); diff --git a/src/librustc/middle/trans/common.rs b/src/librustc/middle/trans/common.rs index 625738ab1b9f..bcdfc7cf95b4 100644 --- a/src/librustc/middle/trans/common.rs +++ b/src/librustc/middle/trans/common.rs @@ -250,6 +250,13 @@ pub struct param_substs { self_ty: Option } +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: ¶m_substs) -> ~str { fmt!("param_substs {tys:%?, vtables:%?, bounds:%?}", substs.tys.map(|t| ty_to_str(tcx, *t)), @@ -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| { diff --git a/src/librustc/middle/trans/meth.rs b/src/librustc/middle/trans/meth.rs index 2ac8742f6e07..b49bdbba4263 100644 --- a/src/librustc/middle/trans/meth.rs +++ b/src/librustc/middle/trans/meth.rs @@ -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( diff --git a/src/librustc/middle/trans/monomorphize.rs b/src/librustc/middle/trans/monomorphize.rs index a38c82d37c40..67bba3e0bd98 100644 --- a/src/librustc/middle/trans/monomorphize.rs +++ b/src/librustc/middle/trans/monomorphize.rs @@ -47,6 +47,7 @@ pub fn monomorphic_fn(ccx: @CrateContext, impl_did_opt: Option, ref_id: Option) -> (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| { diff --git a/src/librustc/middle/typeck/astconv.rs b/src/librustc/middle/typeck/astconv.rs index c288151308fe..5d69ab5766d7 100644 --- a/src/librustc/middle/typeck/astconv.rs +++ b/src/librustc/middle/typeck/astconv.rs @@ -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; @@ -186,19 +186,15 @@ pub fn ast_path_to_ty( 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 } } @@ -368,7 +364,7 @@ pub fn ast_ty_to_ty( }; 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 { diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index ea5301af9b29..e216f9266e91 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/src/librustc/middle/typeck/check/mod.rs @@ -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}; @@ -160,9 +161,13 @@ pub struct SelfInfo { pub struct inherited { infcx: @mut infer::InferCtxt, locals: HashMap, + + // Temporary tables: node_types: HashMap, node_type_substs: HashMap, - adjustments: HashMap + adjustments: HashMap, + method_map: method_map, + vtable_map: vtable_map, } pub enum FnKind { @@ -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(), } } @@ -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 => { @@ -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, diff --git a/src/librustc/middle/typeck/check/regionck.rs b/src/librustc/middle/typeck/check/regionck.rs index d6a7e6103b14..fa0d9e81c2de 100644 --- a/src/librustc/middle/typeck/check/regionck.rs +++ b/src/librustc/middle/typeck/check/regionck.rs @@ -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); @@ -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 } } @@ -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) diff --git a/src/librustc/middle/typeck/check/vtable.rs b/src/librustc/middle/typeck/check/vtable.rs index 2a705a8feb8b..6dfe34191d3c 100644 --- a/src/librustc/middle/typeck/check/vtable.rs +++ b/src/librustc/middle/typeck/check/vtable.rs @@ -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 { @@ -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); } } } @@ -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 { @@ -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); } } } @@ -599,10 +597,7 @@ pub fn early_resolve_expr(ex: @ast::expr, // vtable (that is: "ex has vtable // ") if !is_early { - let vtable_map = - cx.vtable_map; - vtable_map.insert(ex.id, - @~[vtable]); + insert_vtables(fcx, ex.id, @~[vtable]); } } None => { diff --git a/src/librustc/middle/typeck/check/writeback.rs b/src/librustc/middle/typeck/check/writeback.rs index 18f232379169..53b0275f95de 100644 --- a/src/librustc/middle/typeck/check/writeback.rs +++ b/src/librustc/middle/typeck/check/writeback.rs @@ -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; @@ -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) } } } @@ -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| { diff --git a/src/librustc/middle/typeck/collect.rs b/src/librustc/middle/typeck/collect.rs index a8e91f0097a0..a74ea5dff8ba 100644 --- a/src/librustc/middle/typeck/collect.rs +++ b/src/librustc/middle/typeck/collect.rs @@ -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; @@ -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) diff --git a/src/librustc/middle/typeck/mod.rs b/src/librustc/middle/typeck/mod.rs index 7724b43b50f8..a5265deafdde 100644 --- a/src/librustc/middle/typeck/mod.rs +++ b/src/librustc/middle/typeck/mod.rs @@ -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(a: T) -- a's vtable would have a @@ -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, @@ -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) { diff --git a/src/test/run-pass/issue-4036.rs b/src/test/run-pass/issue-4036.rs new file mode 100644 index 000000000000..f24875cbf8e0 --- /dev/null +++ b/src/test/run-pass/issue-4036.rs @@ -0,0 +1,21 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Issue #4036: Test for an issue that arose around fixing up type inference +// byproducts in vtable records. + +extern mod std; +use self::std::json; +use self::std::serialize; + +pub fn main() { + let json = json::from_str("[1]").unwrap(); + let _x: ~[int] = serialize::Decodable::decode(&json::Decoder(json)); +}