From f4acaf6934bf836e6d3feb14738378366f47bef0 Mon Sep 17 00:00:00 2001 From: Marijn Haverbeke Date: Sun, 18 Dec 2011 19:30:40 +0100 Subject: [PATCH] Only look for a matching method when normal field access fails We should probalby warn when defining a method foo on {foo: int} etc. This should reduce the amount of useless typevars that are allocated. Issue #1227 --- src/comp/middle/trans.rs | 4 +- src/comp/middle/trans_alt.rs | 6 +- src/comp/middle/trans_objects.rs | 4 +- src/comp/middle/ty.rs | 18 ++- src/comp/middle/typeck.rs | 144 ++++++++++--------- src/comp/syntax/print/pprust.rs | 8 +- src/test/compile-fail/direct-obj-fn-call.rs | 2 +- src/test/compile-fail/self-missing-method.rs | 2 +- src/test/compile-fail/vec-field.rs | 2 +- src/test/run-pass/static-impl.rs | 25 +++- 10 files changed, 120 insertions(+), 95 deletions(-) diff --git a/src/comp/middle/trans.rs b/src/comp/middle/trans.rs index de5954796fc0..acec464e6f8e 100644 --- a/src/comp/middle/trans.rs +++ b/src/comp/middle/trans.rs @@ -2759,7 +2759,7 @@ fn trans_object_field_inner(bcx: @block_ctxt, o: ValueRef, let ccx = bcx_ccx(bcx), tcx = ccx.tcx; let mths = alt ty::struct(tcx, o_ty) { ty::ty_obj(ms) { ms } }; - let ix = ty::method_idx(ccx.sess, bcx.sp, field, mths); + let ix = option::get(ty::method_idx(field, mths)); let vtbl = Load(bcx, GEPi(bcx, o, [0, abi::obj_field_vtbl])); let vtbl_type = T_ptr(T_array(T_ptr(T_nil()), ix + 1u)); vtbl = PointerCast(bcx, vtbl, vtbl_type); @@ -2782,7 +2782,7 @@ fn trans_rec_field(bcx: @block_ctxt, base: @ast::expr, let {bcx, val} = trans_temp_expr(bcx, base); let {bcx, val, ty} = autoderef(bcx, val, ty::expr_ty(bcx_tcx(bcx), base)); let fields = alt ty::struct(bcx_tcx(bcx), ty) { ty::ty_rec(fs) { fs } }; - let ix = ty::field_idx(bcx_ccx(bcx).sess, bcx.sp, field, fields); + let ix = option::get(ty::field_idx(field, fields)); // Silly check check type_is_tup_like(bcx, ty); let {bcx, val} = GEP_tup_like(bcx, ty, val, [0, ix as int]); diff --git a/src/comp/middle/trans_alt.rs b/src/comp/middle/trans_alt.rs index baf81ba296d3..6e00521ef3b0 100644 --- a/src/comp/middle/trans_alt.rs +++ b/src/comp/middle/trans_alt.rs @@ -413,8 +413,7 @@ fn compile_submatch(bcx: @block_ctxt, m: match, vals: [ValueRef], f: mk_fail, alt ty::struct(ccx.tcx, rec_ty) { ty::ty_rec(fields) { fields } }; let rec_vals = []; for field_name: ast::ident in rec_fields { - let ix: uint = - ty::field_idx(ccx.sess, dummy_sp(), field_name, fields); + let ix = option::get(ty::field_idx(field_name, fields)); // not sure how to get rid of this check check type_is_tup_like(bcx, rec_ty); let r = trans::GEP_tup_like(bcx, rec_ty, val, [0, ix as int]); @@ -722,8 +721,7 @@ fn bind_irrefutable_pat(bcx: @block_ctxt, pat: @ast::pat, val: ValueRef, let rec_fields = alt ty::struct(ccx.tcx, rec_ty) { ty::ty_rec(fields) { fields } }; for f: ast::field_pat in fields { - let ix: uint = - ty::field_idx(ccx.sess, pat.span, f.ident, rec_fields); + let ix = option::get(ty::field_idx(f.ident, rec_fields)); // how to get rid of this check? check type_is_tup_like(bcx, rec_ty); let r = trans::GEP_tup_like(bcx, rec_ty, val, [0, ix as int]); diff --git a/src/comp/middle/trans_objects.rs b/src/comp/middle/trans_objects.rs index 72015faab22d..b8c80da9ac71 100644 --- a/src/comp/middle/trans_objects.rs +++ b/src/comp/middle/trans_objects.rs @@ -642,7 +642,7 @@ fn process_bkwding_mthd(cx: @local_ctxt, sp: span, m: @ty::method, let ix: uint = 0u; alt ty::struct(bcx_tcx(bcx), outer_obj_ty) { ty::ty_obj(methods) { - ix = ty::method_idx(cx.ccx.sess, sp, m.ident, methods); + ix = option::get(ty::method_idx(m.ident, methods)); } _ { // Shouldn't happen. @@ -787,7 +787,7 @@ fn process_fwding_mthd(cx: @local_ctxt, sp: span, m: @ty::method, let ix: uint = 0u; alt ty::struct(bcx_tcx(bcx), inner_obj_ty) { ty::ty_obj(methods) { - ix = ty::method_idx(cx.ccx.sess, sp, m.ident, methods); + ix = option::get(ty::method_idx(m.ident, methods)); } _ { // Shouldn't happen. diff --git a/src/comp/middle/ty.rs b/src/comp/middle/ty.rs index d27f3d79b75b..521b73f3c11d 100644 --- a/src/comp/middle/ty.rs +++ b/src/comp/middle/ty.rs @@ -1698,11 +1698,10 @@ fn stmt_node_id(s: @ast::stmt) -> ast::node_id { } } -fn field_idx(sess: session::session, sp: span, id: ast::ident, - fields: [field]) -> uint { - let i: uint = 0u; - for f: field in fields { if str::eq(f.ident, id) { ret i; } i += 1u; } - sess.span_fatal(sp, "unknown field '" + id + "' of record"); +fn field_idx(id: ast::ident, fields: [field]) -> option::t { + let i = 0u; + for f in fields { if f.ident == id { ret some(i); } i += 1u; } + ret none; } fn get_field(tcx: ctxt, rec_ty: t, id: ast::ident) -> field { @@ -1715,11 +1714,10 @@ fn get_field(tcx: ctxt, rec_ty: t, id: ast::ident) -> field { } } -fn method_idx(sess: session::session, sp: span, id: ast::ident, - meths: [method]) -> uint { - let i: uint = 0u; - for m: method in meths { if str::eq(m.ident, id) { ret i; } i += 1u; } - sess.span_fatal(sp, "unknown method '" + id + "' of obj"); +fn method_idx(id: ast::ident, meths: [method]) -> option::t { + let i = 0u; + for m in meths { if m.ident == id { ret some(i); } i += 1u; } + ret none; } fn sort_methods(meths: [method]) -> [method] { diff --git a/src/comp/middle/typeck.rs b/src/comp/middle/typeck.rs index 2a8673a265a3..ffe5004002ed 100644 --- a/src/comp/middle/typeck.rs +++ b/src/comp/middle/typeck.rs @@ -1466,30 +1466,32 @@ fn lookup_method(fcx: @fn_ctxt, isc: resolve::iscopes, -> option::t<{method: @resolve::method_info, ids: [int]}> { let result = none; std::list::iter(isc) {|impls| + if option::is_some(result) { ret; } for @{did, methods, _} in *impls { - let (n_tps, self_ty) = if did.crate == ast::local_crate { - alt fcx.ccx.tcx.items.get(did.node) { - ast_map::node_item(@{node: ast::item_impl(tps, st, _), _}) { - (vec::len(tps), ast_ty_to_ty_crate(fcx.ccx, st)) - } - } - } else { - let tpt = csearch::get_type(fcx.ccx.tcx, did); - (vec::len(tpt.kinds), tpt.ty) - }; - let {ids, ty: self_ty} = if n_tps > 0u { - bind_params_in_type(ast_util::dummy_sp(), fcx.ccx.tcx, - bind next_ty_var_id(fcx), self_ty, n_tps) - } else { {ids: [], ty: self_ty} }; - // FIXME[impl] Don't unify in the current fcx, use - // scratch context - alt unify::unify(fcx, ty, self_ty) { - ures_ok(_) { - for m in methods { - if m.ident == name { - result = some({method: m, ids: ids}); - ret; + alt vec::find(methods, {|m| m.ident == name}) { + some(m) { + let (n_tps, self_ty) = if did.crate == ast::local_crate { + alt fcx.ccx.tcx.items.get(did.node) { + ast_map::node_item(@{node: ast::item_impl(tps, st, _), + _}) { + (vec::len(tps), ast_ty_to_ty_crate(fcx.ccx, st)) + } } + } else { + let tpt = csearch::get_type(fcx.ccx.tcx, did); + (vec::len(tpt.kinds), tpt.ty) + }; + let {ids, ty: self_ty} = if n_tps > 0u { + bind_params_in_type(ast_util::dummy_sp(), fcx.ccx.tcx, + bind next_ty_var_id(fcx), self_ty, + n_tps) + } else { {ids: [], ty: self_ty} }; + alt unify::unify(fcx, ty, self_ty) { + ures_ok(_) { + result = some({method: m, ids: ids}); + ret; + } + _ {} } } _ {} @@ -2128,60 +2130,66 @@ fn check_expr_with_unifier(fcx: @fn_ctxt, expr: @ast::expr, unify: unifier, } ast::expr_field(base, field) { bot |= check_expr(fcx, base); - let base_t = expr_ty(tcx, base); - let iscope = fcx.ccx.impl_map.get(expr.id); - alt lookup_method(fcx, iscope, field, base_t) { - some({method, ids}) { - let fty = if method.did.crate == ast::local_crate { - alt tcx.items.get(method.did.node) { - ast_map::node_method(m) { - let mt = ty_of_method(tcx, m_check, m); - ty::mk_fn(tcx, mt.proto, mt.inputs, - mt.output, mt.cf, mt.constrs) - } - } - } else { csearch::get_type(tcx, method.did).ty }; - let ids = ids; - if method.n_tps > 0u { - let b = bind_params_in_type(expr.span, tcx, - bind next_ty_var_id(fcx), - fty, method.n_tps); - ids += b.ids; - fty = b.ty; - } - let substs = vec::map(ids, {|id| ty::mk_var(tcx, id)}); - write::ty_fixup(fcx, id, {substs: some(substs), ty: fty}); - fcx.ccx.method_map.insert(id, method.did); - } - _ { - base_t = do_autoderef(fcx, expr.span, base_t); - alt structure_of(fcx, expr.span, base_t) { - ty::ty_rec(fields) { - let ix = ty::field_idx(tcx.sess, expr.span, field, fields); - if ix >= vec::len::(fields) { - tcx.sess.span_fatal(expr.span, "bad index on record"); - } + let expr_t = expr_ty(tcx, base); + let base_t = do_autoderef(fcx, expr.span, expr_t); + let handled = false; + alt structure_of(fcx, expr.span, base_t) { + ty::ty_rec(fields) { + alt ty::field_idx(field, fields) { + some(ix) { write::ty_only_fixup(fcx, id, fields[ix].mt.ty); + handled = true; } - ty::ty_obj(methods) { - let ix = ty::method_idx(tcx.sess, expr.span, field, methods); - if ix >= vec::len::(methods) { - tcx.sess.span_fatal(expr.span, "bad index on obj"); - } + _ {} + } + } + ty::ty_obj(methods) { + alt ty::method_idx(field, methods) { + some(ix) { let meth = methods[ix]; - let t = ty::mk_fn(tcx, meth.proto, meth.inputs, meth.output, - meth.cf, meth.constrs); + let t = ty::mk_fn(tcx, meth.proto, meth.inputs, + meth.output, meth.cf, meth.constrs); write::ty_only_fixup(fcx, id, t); + handled = true; } - _ { - let t_err = resolve_type_vars_if_possible(fcx, base_t); - let msg = #fmt["attempted field access on type %s, but no \ - method implementation was found", - ty_to_str(tcx, t_err)]; + _ {} + } + } + _ {} + } + if !handled { + let iscope = fcx.ccx.impl_map.get(expr.id); + alt lookup_method(fcx, iscope, field, expr_t) { + some({method, ids}) { + let fty = if method.did.crate == ast::local_crate { + alt tcx.items.get(method.did.node) { + ast_map::node_method(m) { + let mt = ty_of_method(tcx, m_check, m); + ty::mk_fn(tcx, mt.proto, mt.inputs, + mt.output, mt.cf, mt.constrs) + } + } + } else { csearch::get_type(tcx, method.did).ty }; + let ids = ids; + if method.n_tps > 0u { + let b = bind_params_in_type(expr.span, tcx, + bind next_ty_var_id(fcx), + fty, method.n_tps); + ids += b.ids; + fty = b.ty; + } + let substs = vec::map(ids, {|id| ty::mk_var(tcx, id)}); + write::ty_fixup(fcx, id, {substs: some(substs), ty: fty}); + fcx.ccx.method_map.insert(id, method.did); + } + none. { + let t_err = resolve_type_vars_if_possible(fcx, expr_t); + let msg = #fmt["attempted access of field %s on type %s, but \ + no method implementation was found", + field, ty_to_str(tcx, t_err)]; tcx.sess.span_fatal(expr.span, msg); } } - } } } ast::expr_index(base, idx) { diff --git a/src/comp/syntax/print/pprust.rs b/src/comp/syntax/print/pprust.rs index 54388102f535..9b3d047d4357 100644 --- a/src/comp/syntax/print/pprust.rs +++ b/src/comp/syntax/print/pprust.rs @@ -467,11 +467,10 @@ fn print_item(s: ps, &&item: @ast::item) { space(s.s); bopen(s); for meth: @ast::method in _obj.methods { - let typarams: [ast::ty_param] = []; hardbreak_if_not_bol(s); maybe_print_comment(s, meth.span.lo); print_fn(s, meth.node.meth.decl, meth.node.meth.proto, - meth.node.ident, typarams, []); + meth.node.ident, meth.node.tps, []); word(s.s, " "); print_block(s, meth.node.meth.body); } @@ -490,7 +489,7 @@ fn print_item(s: ps, &&item: @ast::item) { hardbreak_if_not_bol(s); maybe_print_comment(s, meth.span.lo); print_fn(s, meth.node.meth.decl, meth.node.meth.proto, - meth.node.ident, [], []); + meth.node.ident, meth.node.tps, []); word(s.s, " "); print_block(s, meth.node.meth.body); } @@ -964,11 +963,10 @@ fn print_expr(s: ps, &&expr: @ast::expr) { // Methods for meth: @ast::method in anon_obj.methods { - let typarams: [ast::ty_param] = []; hardbreak_if_not_bol(s); maybe_print_comment(s, meth.span.lo); print_fn(s, meth.node.meth.decl, meth.node.meth.proto, - meth.node.ident, typarams, []); + meth.node.ident, meth.node.tps, []); word(s.s, " "); print_block(s, meth.node.meth.body); } diff --git a/src/test/compile-fail/direct-obj-fn-call.rs b/src/test/compile-fail/direct-obj-fn-call.rs index 01ec855bf7cc..cb6557dea8ce 100644 --- a/src/test/compile-fail/direct-obj-fn-call.rs +++ b/src/test/compile-fail/direct-obj-fn-call.rs @@ -1,4 +1,4 @@ -// error-pattern: attempted field access +// error-pattern: attempted access of field hello obj x() { fn hello() { log "hello"; } diff --git a/src/test/compile-fail/self-missing-method.rs b/src/test/compile-fail/self-missing-method.rs index b9c7086f6e7b..21069a4832e2 100644 --- a/src/test/compile-fail/self-missing-method.rs +++ b/src/test/compile-fail/self-missing-method.rs @@ -1,4 +1,4 @@ -// error-pattern:attempted field access on type fn +// error-pattern:attempted access of field m on type fn fn main() { obj foo() { diff --git a/src/test/compile-fail/vec-field.rs b/src/test/compile-fail/vec-field.rs index 1ec888670c49..9e2c54003fe6 100644 --- a/src/test/compile-fail/vec-field.rs +++ b/src/test/compile-fail/vec-field.rs @@ -1,4 +1,4 @@ -// error-pattern:attempted field access on type [int] +// error-pattern:attempted access of field some_field_name on type [int] // issue #367 fn f() { diff --git a/src/test/run-pass/static-impl.rs b/src/test/run-pass/static-impl.rs index ab4fbc5915f4..9d245a22bb7a 100644 --- a/src/test/run-pass/static-impl.rs +++ b/src/test/run-pass/static-impl.rs @@ -9,10 +9,33 @@ mod b { impl baz for str { fn plus() -> int { 200 } } } +impl util for uint { + fn str() -> str { uint::str(self) } + fn times(f: block(uint)) { + let c = 0u; + while c < self { f(c); c += 1u; } + } +} + +impl util for [T] { + fn len() -> uint { vec::len(self) } + fn iter(f: block(T)) { for x in self { f(x); } } + fn map(f: block(T) -> U) -> [U] { + let r = []; + for elt in self { r += [f(elt)]; } + r + } +} + fn main() { impl foo for int { fn plus() -> int { self + 10 } } assert 10.plus() == 20; assert 10u.plus() == 30; assert "hi".plus() == 200; -} + assert [1].len().str() == "1"; + assert [3, 4].map({|a| a + 4})[0] == 7; + let x = 0u; + 10u.times {|_n| x += 2u;} + assert x == 20u; +}