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

4 files changed, 159 insertions, 154 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 }
-
-          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`, 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 {
+
+        // 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 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 }
+              // 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
+    // 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 substs = {
-            self_r: substs.self_r,
+            self_r: some(dummy_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})
-            })
+            tps: if_substs.tps + dummy_tps
         };
-        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 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<ty::t> {
+    fn lub(infcx: infer_ctxt, &&b: ty::t) -> cres<ty::t> {
         lub(infcx).tys(self, b)
     }
 
-    fn glb(infcx: infer_ctxt, b: ty::t) -> cres<ty::t> {
+    fn glb(infcx: infer_ctxt, &&b: ty::t) -> cres<ty::t> {
         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<ty::region> {
+    fn lub(infcx: infer_ctxt, &&b: ty::region) -> cres<ty::region> {
         lub(infcx).regions(self, b)
     }
 
-    fn glb(infcx: infer_ctxt, b: ty::region) -> cres<ty::region> {
+    fn glb(infcx: infer_ctxt, &&b: ty::region) -> cres<ty::region> {
         glb(infcx).regions(self, b)
     }
 }