From 22a10f0e4afc504384f5a64ecb8d6b44cb3d9503 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 30 May 2012 11:01:31 -0700 Subject: [PATCH] refactor region manip. to remove redundancy, get closer to fn subtyping also: remove "auto-mode-matching" for implemented interfaces, as it is complex and interacts poorly with classes cc #2263 --- src/rustc/middle/typeck/check.rs | 4 +- src/rustc/middle/typeck/check/regionmanip.rs | 198 +++++++++---------- src/rustc/middle/typeck/collect.rs | 99 ++++++---- src/rustc/middle/typeck/infer.rs | 12 +- src/test/compile-fail/selftype-astparam.rs | 4 +- src/test/run-pass/selftype-add-ints.rs | 2 +- 6 files changed, 162 insertions(+), 157 deletions(-) diff --git a/src/rustc/middle/typeck/check.rs b/src/rustc/middle/typeck/check.rs index 20c9db7b3fd..deede6b5e6f 100644 --- a/src/rustc/middle/typeck/check.rs +++ b/src/rustc/middle/typeck/check.rs @@ -70,9 +70,7 @@ import astconv::{ast_conv, ast_ty_to_ty}; import collect::{methods}; // ccx.to_ty() import method::{methods}; // methods for method::lookup import middle::ty::tys_in_fn_ty; -import regionmanip::{replace_bound_regions_in_fn_ty, - region_of, replace_bound_regions, - collect_bound_regions_in_tys}; +import regionmanip::{replace_bound_regions_in_fn_ty, region_of}; import rscope::*; type fn_ctxt = diff --git a/src/rustc/middle/typeck/check/regionmanip.rs b/src/rustc/middle/typeck/check/regionmanip.rs index 41451c9e150..5d9572e029d 100644 --- a/src/rustc/middle/typeck/check/regionmanip.rs +++ b/src/rustc/middle/typeck/check/regionmanip.rs @@ -20,7 +20,7 @@ fn replace_bound_regions_in_fn_ty( all_tys.map { |t| ty_to_str(tcx, t) }]; let _i = indenter(); - let isr = collect_bound_regions_in_tys(tcx, isr, all_tys) { |br| + let isr = create_bound_region_mapping(tcx, isr, all_tys) { |br| #debug["br=%?", br]; mapf(br) }; @@ -36,47 +36,105 @@ fn replace_bound_regions_in_fn_ty( ret {isr: isr, self_ty: t_self, fn_ty: alt check ty::get(t_fn).struct { ty::ty_fn(o) {o} }}; -} -// Takes `isr`, a mapping from in-scope region names ("isr"s) to their -// corresponding regions (possibly produced by a call to -// collect_bound_regions_in_tys; and `ty`, a type. Returns an updated -// version of `ty`, in which bound regions in `ty` have been replaced -// with the corresponding bindings in `isr`. -fn replace_bound_regions( - tcx: ty::ctxt, - isr: isr_alist, - ty: ty::t) -> ty::t { - ty::fold_regions(tcx, ty) { |r, in_fn| - alt r { - // As long as we are not within a fn() type, `&T` is mapped to the - // free region anon_r. But within a fn type, it remains bound. - ty::re_bound(ty::br_anon) if in_fn { r } + // Takes `isr`, a (possibly empty) mapping from in-scope region + // names ("isr"s) to their corresponding regions; `tys`, a list of + // types, and `to_r`, a closure that takes a bound_region and + // returns a region. Returns an updated version of `isr`, + // extended with the in-scope region names from all of the bound + // regions appearing in the types in the `tys` list (if they're + // not in `isr` already), with each of those in-scope region names + // mapped to a region that's the result of applying `to_r` to + // itself. + fn create_bound_region_mapping( + tcx: ty::ctxt, + isr: isr_alist, + tys: [ty::t], + to_r: fn(ty::bound_region) -> ty::region) -> isr_alist { - ty::re_bound(br) { - alt isr.find(br) { - // In most cases, all named, bound regions will be mapped to - // some free region. - some(fr) { fr } - - // But in the case of a fn() type, there may be named regions - // within that remain bound: - none if in_fn { r } - none { - tcx.sess.bug( - #fmt["Bound region not found in \ - in_scope_regions list: %s", - region_to_str(tcx, r)]); + // Takes `isr` (described above), `to_r` (described above), + // and `r`, a region. If `r` is anything other than a bound + // region, or if it's a bound region that already appears in + // `isr`, then we return `isr` unchanged. If `r` is a bound + // region that doesn't already appear in `isr`, we return an + // updated isr_alist that now contains a mapping from `r` to + // the result of calling `to_r` on it. + fn append_isr(isr: isr_alist, + to_r: fn(ty::bound_region) -> ty::region, + r: ty::region) -> isr_alist { + alt r { + ty::re_free(_, _) | ty::re_static | ty::re_scope(_) | + ty::re_var(_) { + isr + } + ty::re_bound(br) { + alt isr.find(br) { + some(_) { isr } + none { @cons((br, to_r(br)), isr) } + } } } - } + } - // Free regions like these just stay the same: - ty::re_static | - ty::re_scope(_) | - ty::re_free(_, _) | - ty::re_var(_) { r } + // For each type `ty` in `tys`... + tys.foldl(isr) { |isr, ty| + let mut isr = isr; + + // Using fold_regions is inefficient, because it + // constructs new types, but it avoids code duplication in + // terms of locating all the regions within the various + // kinds of types. This had already caused me several + // bugs so I decided to switch over. + ty::fold_regions(tcx, ty) { |r, in_fn| + if !in_fn { isr = append_isr(isr, to_r, r); } + r + }; + + isr + } + } + + // Takes `isr`, a mapping from in-scope region names ("isr"s) to + // their corresponding regions; and `ty`, a type. Returns an + // updated version of `ty`, in which bound regions in `ty` have + // been replaced with the corresponding bindings in `isr`. + fn replace_bound_regions( + tcx: ty::ctxt, + isr: isr_alist, + ty: ty::t) -> ty::t { + + ty::fold_regions(tcx, ty) { |r, in_fn| + alt r { + // As long as we are not within a fn() type, `&T` is + // mapped to the free region anon_r. But within a fn + // type, it remains bound. + ty::re_bound(ty::br_anon) if in_fn { r } + + ty::re_bound(br) { + alt isr.find(br) { + // In most cases, all named, bound regions will be + // mapped to some free region. + some(fr) { fr } + + // But in the case of a fn() type, there may be + // named regions within that remain bound: + none if in_fn { r } + none { + tcx.sess.bug( + #fmt["Bound region not found in \ + in_scope_regions list: %s", + region_to_str(tcx, r)]); + } + } + } + + // Free regions like these just stay the same: + ty::re_static | + ty::re_scope(_) | + ty::re_free(_, _) | + ty::re_var(_) { r } + } } } } @@ -149,71 +207,3 @@ fn region_of(fcx: @fn_ctxt, expr: @ast::expr) -> ty::region { } } } - -// Takes `isr`, a (possibly empty) mapping from in-scope region names ("isr"s) -// to their corresponding regions; `tys`, a list of types, and `to_r`, a -// closure that takes a bound_region and returns a region. Returns an updated -// version of `isr`, extended with the in-scope region names from all of the -// bound regions appearing in the types in the `tys` list (if they're not in -// `isr` already), with each of those in-scope region names mapped to a region -// that's the result of applying `to_r` to itself. - -// "collect" is something of a misnomer -- we're not merely collecting -// a list of the bound regions, but also doing the work of applying -// `to_r` to them! -fn collect_bound_regions_in_tys( - tcx: ty::ctxt, - isr: isr_alist, - tys: [ty::t], - to_r: fn(ty::bound_region) -> ty::region) -> isr_alist { - - // Takes `isr` (described above), `to_r` (described above), and `r`, a - // region. If `r` is anything other than a bound region, or if it's a - // bound region that already appears in `isr`, then we return `isr` - // unchanged. If `r` is a bound region that doesn't already appear in - // `isr`, we return an updated isr_alist that now contains a mapping from - // `r` to the result of calling `to_r` on it. - fn append_isr(isr: isr_alist, - to_r: fn(ty::bound_region) -> ty::region, - r: ty::region) -> isr_alist { - alt r { - ty::re_free(_, _) | ty::re_static | ty::re_scope(_) | - ty::re_var(_) { - isr - } - ty::re_bound(br) { - alt isr.find(br) { - some(_) { isr } - none { @cons((br, to_r(br)), isr) } - } - } - } - } - - // For each region in `t`, apply the `append_isr` function to that - // region, accumulating and returning the results in an isr_alist. - fn fold_over_regions_in_type( - tcx: ty::ctxt, - isr: isr_alist, - ty: ty::t, - to_r: fn(ty::bound_region) -> ty::region) -> isr_alist { - - let mut isr = isr; - - // Using fold_regions is inefficient, because it constructs new types, - // but it avoids code duplication in terms of locating all the regions - // within the various kinds of types. This had already caused me - // several bugs so I decided to switch over. - ty::fold_regions(tcx, ty) { |r, in_fn| - if !in_fn { isr = append_isr(isr, to_r, r); } - r - }; - - ret isr; - } - - // For each type `t` in `tys`... - tys.foldl(isr) { |isr, t| - fold_over_regions_in_type(tcx, isr, t, to_r) - } -} diff --git a/src/rustc/middle/typeck/collect.rs b/src/rustc/middle/typeck/collect.rs index edaa25396a8..71051954fb7 100644 --- a/src/rustc/middle/typeck/collect.rs +++ b/src/rustc/middle/typeck/collect.rs @@ -151,48 +151,74 @@ fn ensure_iface_methods(ccx: @crate_ctxt, id: ast::node_id) { } } -fn compare_impl_method(tcx: ty::ctxt, sp: span, impl_m: ty::method, - impl_tps: uint, if_m: ty::method, substs: ty::substs, - self_ty: ty::t) -> ty::t { +#[doc = " +Checks that a method from an impl/class conforms to the signature of +the same method as declared in the iface. + +# Parameters + +- impl_m: the method in the impl +- impl_tps: the type params declared on the impl itself (not the method!) +- if_m: the method in the iface +- if_substs: the substitutions used on the type of the iface +- self_ty: the self type of the impl +"] +fn compare_impl_method(tcx: ty::ctxt, sp: span, + impl_m: ty::method, impl_tps: uint, + if_m: ty::method, if_substs: ty::substs, + self_ty: ty::t) { if impl_m.tps != if_m.tps { tcx.sess.span_err(sp, "method `" + if_m.ident + "` has an incompatible set of type parameters"); - ty::mk_fn(tcx, impl_m.fty) - } else if vec::len(impl_m.fty.inputs) != vec::len(if_m.fty.inputs) { + ret; + } + + if vec::len(impl_m.fty.inputs) != vec::len(if_m.fty.inputs) { tcx.sess.span_err(sp,#fmt["method `%s` has %u parameters \ but the iface has %u", if_m.ident, vec::len(impl_m.fty.inputs), vec::len(if_m.fty.inputs)]); - ty::mk_fn(tcx, impl_m.fty) - } else { - let auto_modes = vec::map2(impl_m.fty.inputs, if_m.fty.inputs, {|i, f| - alt ty::get(f.ty).struct { - ty::ty_param(*) | ty::ty_self - if alt i.mode { ast::infer(_) { true } _ { false } } { - {mode: ast::expl(ast::by_ref) with i} - } - _ { i } - } - }); - let impl_fty = ty::mk_fn(tcx, {inputs: auto_modes with impl_m.fty}); + ret; + } - // Add dummy substs for the parameters of the impl method - let substs = { - self_r: substs.self_r, - self_ty: some(self_ty), - tps: substs.tps + vec::from_fn(vec::len(*if_m.tps), {|i| - ty::mk_param(tcx, i + impl_tps, {crate: 0, node: 0}) - }) + // Perform substitutions so that the iface/impl methods are expressed + // in terms of the same set of type/region parameters: + // - replace iface type parameters with those from `if_substs` + // - replace method parameters on the iface with fresh, dummy parameters + // that correspond to the parameters we will find on the impl + // - replace self region with a fresh, dummy region + let dummy_self_r = ty::re_free(0, ty::br_self); + let impl_fty = { + let impl_fty = ty::mk_fn(tcx, impl_m.fty); + replace_bound_self(tcx, impl_fty, dummy_self_r) + }; + let if_fty = { + let dummy_tps = vec::from_fn((*if_m.tps).len()) { |i| + // hack: we don't know the def id of the impl tp, but it + // is not important for unification + ty::mk_param(tcx, i + impl_tps, {crate: 0, node: 0}) }; - let mut if_fty = ty::mk_fn(tcx, if_m.fty); - if_fty = ty::subst(tcx, substs, if_fty); - require_same_types( - tcx, sp, impl_fty, if_fty, - {|| "method `" + if_m.ident + - "` has an incompatible type"}); - ret impl_fty; + let substs = { + self_r: some(dummy_self_r), + self_ty: some(self_ty), + tps: if_substs.tps + dummy_tps + }; + let if_fty = ty::mk_fn(tcx, if_m.fty); + ty::subst(tcx, substs, if_fty) + }; + require_same_types( + tcx, sp, impl_fty, if_fty, + {|| "method `" + if_m.ident + "` has an incompatible type"}); + ret; + + // Replaces bound references to the self region with `with_r`. + fn replace_bound_self(tcx: ty::ctxt, ty: ty::t, + with_r: ty::region) -> ty::t { + ty::fold_regions(tcx, ty) { |r, _in_fn| + if r == ty::re_bound(ty::br_self) {with_r} else {r} + } } } @@ -219,18 +245,9 @@ fn check_methods_against_iface(ccx: @crate_ctxt, not match the iface method's \ purity", m.ident]); } - let mt = compare_impl_method( + compare_impl_method( ccx.tcx, span, m, vec::len(tps), if_m, tpt.substs, selfty); - let old = tcx.tcache.get(local_def(id)); - if old.ty != mt { - tcx.tcache.insert( - local_def(id), - {bounds: old.bounds, - rp: old.rp, - ty: mt}); - write_ty_to_tcx(tcx, id, mt); - } } none { tcx.sess.span_err( diff --git a/src/rustc/middle/typeck/infer.rs b/src/rustc/middle/typeck/infer.rs index 8579da8ed93..97bd43cc527 100644 --- a/src/rustc/middle/typeck/infer.rs +++ b/src/rustc/middle/typeck/infer.rs @@ -361,29 +361,29 @@ iface st { } impl of st for ty::t { - fn sub(infcx: infer_ctxt, b: ty::t) -> ures { + fn sub(infcx: infer_ctxt, &&b: ty::t) -> ures { sub(infcx).tys(self, b).to_ures() } - fn lub(infcx: infer_ctxt, b: ty::t) -> cres { + fn lub(infcx: infer_ctxt, &&b: ty::t) -> cres { lub(infcx).tys(self, b) } - fn glb(infcx: infer_ctxt, b: ty::t) -> cres { + fn glb(infcx: infer_ctxt, &&b: ty::t) -> cres { glb(infcx).tys(self, b) } } impl of st for ty::region { - fn sub(infcx: infer_ctxt, b: ty::region) -> ures { + fn sub(infcx: infer_ctxt, &&b: ty::region) -> ures { sub(infcx).regions(self, b).chain {|_r| ok(()) } } - fn lub(infcx: infer_ctxt, b: ty::region) -> cres { + fn lub(infcx: infer_ctxt, &&b: ty::region) -> cres { lub(infcx).regions(self, b) } - fn glb(infcx: infer_ctxt, b: ty::region) -> cres { + fn glb(infcx: infer_ctxt, &&b: ty::region) -> cres { glb(infcx).regions(self, b) } } diff --git a/src/test/compile-fail/selftype-astparam.rs b/src/test/compile-fail/selftype-astparam.rs index b4c6cf5c82d..f0d79ccc589 100644 --- a/src/test/compile-fail/selftype-astparam.rs +++ b/src/test/compile-fail/selftype-astparam.rs @@ -1,9 +1,9 @@ iface add { - fn +(x: self) -> self; + fn +(++x: self) -> self; } impl of add for int { - fn +(x: int) -> int { self + x } + fn +(++x: int) -> int { self + x } } fn do_add(x: A, y: A) -> A { x + y } diff --git a/src/test/run-pass/selftype-add-ints.rs b/src/test/run-pass/selftype-add-ints.rs index dabb700da35..64aae3d1544 100644 --- a/src/test/run-pass/selftype-add-ints.rs +++ b/src/test/run-pass/selftype-add-ints.rs @@ -1,5 +1,5 @@ iface add { - fn +(x: self) -> self; + fn +(++x: self) -> self; } impl of add for int {