Update region inference for traits so that a method with

explicit self doesn't incorrectly cause the entire trait to
be tagged as being region-parameterized.

Fixes rust-lang#5224.

Author: nikomatsakis

src/librustc/middle/check_const.rs

@@ -16,7 +16,6 @@ use middle::ty;
 use middle::typeck;
 use util::ppaux;
 
-use core::option;
 use syntax::ast::*;
 use syntax::codemap;
 use syntax::{visit, ast_util, ast_map};

src/librustc/middle/check_match.rs

@@ -19,7 +19,6 @@ use middle::typeck::method_map;
 use middle::moves;
 use util::ppaux::ty_to_str;
 
-use core::option;
 use core::uint;
 use core::vec;
 use std::sort;

src/librustc/middle/kind.rs

@@ -18,7 +18,6 @@ use middle::ty;
 use middle::typeck;
 use util::ppaux::{ty_to_str, tys_to_str};
 
-use core::option;
 use core::str;
 use core::vec;
 use std::oldmap::HashMap;

src/librustc/middle/region.rs

@@ -416,9 +416,13 @@ pub struct DetermineRpCtxt {
     item_id: ast::node_id,
 
     // true when we are within an item but not within a method.
-    // see long discussion on region_is_relevant()
+    // see long discussion on region_is_relevant().
     anon_implies_rp: bool,
 
+    // true when we are not within an &self method.
+    // see long discussion on region_is_relevant().
+    self_implies_rp: bool,
+
     // encodes the context of the current type; invariant if
     // mutable, covariant otherwise
     ambient_variance: region_variance,
@@ -458,14 +462,14 @@ pub fn add_variance(+ambient_variance: region_variance,
 }
 
 pub impl DetermineRpCtxt {
-    fn add_variance(@mut self, variance: region_variance) -> region_variance {
+    fn add_variance(&self, variance: region_variance) -> region_variance {
         add_variance(self.ambient_variance, variance)
     }
 
     /// Records that item `id` is region-parameterized with the
     /// variance `variance`.  If `id` was already parameterized, then
     /// the new variance is joined with the old variance.
-    fn add_rp(@mut self, id: ast::node_id, variance: region_variance) {
+    fn add_rp(&mut self, id: ast::node_id, variance: region_variance) {
         assert id != 0;
         let old_variance = self.region_paramd_items.find(&id);
         let joined_variance = match old_variance {
@@ -490,7 +494,7 @@ pub impl DetermineRpCtxt {
     /// `from`.  Put another way, it indicates that the current item
     /// contains a value of type `from`, so if `from` is
     /// region-parameterized, so is the current item.
-    fn add_dep(@mut self, from: ast::node_id) {
+    fn add_dep(&mut self, from: ast::node_id) {
         debug!("add dependency from %d -> %d (%s -> %s) with variance %?",
                from, self.item_id,
                ast_map::node_id_to_str(self.ast_map, from,
@@ -515,42 +519,46 @@ pub impl DetermineRpCtxt {
     }
 
     // Determines whether a reference to a region that appears in the
-    // AST implies that the enclosing type is region-parameterized.
-    //
-    // This point is subtle.  Here are four examples to make it more
+    // AST implies that the enclosing type is region-parameterized (RP).
+    // This point is subtle.  Here are some examples to make it more
     // concrete.
     //
     // 1. impl foo for &int { ... }
     // 2. impl foo for &self/int { ... }
-    // 3. impl foo for bar { fn m() -> &self/int { ... } }
-    // 4. impl foo for bar { fn m() -> &int { ... } }
+    // 3. impl foo for bar { fn m(@self) -> &self/int { ... } }
+    // 4. impl foo for bar { fn m(&self) -> &self/int { ... } }
+    // 5. impl foo for bar { fn m(&self) -> &int { ... } }
     //
     // In case 1, the anonymous region is being referenced,
     // but it appears in a context where the anonymous region
-    // resolves to self, so the impl foo is region-parameterized.
+    // resolves to self, so the impl foo is RP.
     //
     // In case 2, the self parameter is written explicitly.
     //
-    // In case 3, the method refers to self, so that implies that the
-    // impl must be region parameterized.  (If the type bar is not
-    // region parameterized, that is an error, because the self region
-    // is effectively unconstrained, but that is detected elsewhere).
+    // In case 3, the method refers to the region `self`, so that
+    // implies that the impl must be region parameterized.  (If the
+    // type bar is not region parameterized, that is an error, because
+    // the self region is effectively unconstrained, but that is
+    // detected elsewhere).
+    //
+    // In case 4, the method refers to the region `self`, but the
+    // `self` region is bound by the `&self` receiver, and so this
+    // does not require that `bar` be RP.
     //
-    // In case 4, the anonymous region is referenced, but it
+    // In case 5, the anonymous region is referenced, but it
     // bound by the method, so it does not refer to self.  This impl
     // need not be region parameterized.
     //
-    // So the rules basically are: the `self` region always implies
-    // that the enclosing type is region parameterized.  The anonymous
-    // region also does, unless it appears within a method, in which
-    // case it is bound.  We handle this by setting a flag
-    // (anon_implies_rp) to true when we enter an item and setting
-    // that flag to false when we enter a method.
-    fn region_is_relevant(@mut self, r: @ast::region) -> bool {
+    // Normally, & or &self implies that the enclosing item is RP.
+    // However, within a function, & is always bound.  Within a method
+    // with &self type, &self is also bound.  We detect those last two
+    // cases via flags (anon_implies_rp and self_implies_rp) that are
+    // true when the anon or self region implies RP.
+    fn region_is_relevant(&self, r: @ast::region) -> bool {
         match r.node {
             ast::re_static => false,
             ast::re_anon => self.anon_implies_rp,
-            ast::re_self => true,
+            ast::re_self => self.self_implies_rp,
             ast::re_named(_) => false
         }
     }
@@ -561,7 +569,7 @@ pub impl DetermineRpCtxt {
     //
     // If the region is explicitly specified, then we follows the
     // normal rules.
-    fn opt_region_is_relevant(@mut self,
+    fn opt_region_is_relevant(&self,
                               opt_r: Option<@ast::region>)
                            -> bool {
         debug!("opt_region_is_relevant: %? (anon_implies_rp=%b)",
@@ -575,16 +583,23 @@ pub impl DetermineRpCtxt {
     fn with(@mut self,
             item_id: ast::node_id,
             anon_implies_rp: bool,
+            self_implies_rp: bool,
             f: &fn()) {
         let old_item_id = self.item_id;
         let old_anon_implies_rp = self.anon_implies_rp;
+        let old_self_implies_rp = self.self_implies_rp;
         self.item_id = item_id;
         self.anon_implies_rp = anon_implies_rp;
-        debug!("with_item_id(%d, %b)", item_id, anon_implies_rp);
+        self.self_implies_rp = self_implies_rp;
+        debug!("with_item_id(%d, %b, %b)",
+               item_id,
+               anon_implies_rp,
+               self_implies_rp);
         let _i = ::util::common::indenter();
         f();
         self.item_id = old_item_id;
         self.anon_implies_rp = old_anon_implies_rp;
+        self.self_implies_rp = old_self_implies_rp;
     }
 
     fn with_ambient_variance(@mut self, variance: region_variance, f: &fn()) {
@@ -598,7 +613,7 @@ pub impl DetermineRpCtxt {
 pub fn determine_rp_in_item(item: @ast::item,
                             &&cx: @mut DetermineRpCtxt,
                             visitor: visit::vt<@mut DetermineRpCtxt>) {
-    do cx.with(item.id, true) {
+    do cx.with(item.id, true, true) {
         visit::visit_item(item, cx, visitor);
     }
 }
@@ -610,7 +625,12 @@ pub fn determine_rp_in_fn(fk: &visit::fn_kind,
                           _: ast::node_id,
                           &&cx: @mut DetermineRpCtxt,
                           visitor: visit::vt<@mut DetermineRpCtxt>) {
-    do cx.with(cx.item_id, false) {
+    let self_implies_rp = match fk {
+        &visit::fk_method(_, _, m) => !m.self_ty.node.is_borrowed(),
+        _ => true
+    };
+
+    do cx.with(cx.item_id, false, self_implies_rp) {
         do cx.with_ambient_variance(rv_contravariant) {
             for decl.inputs.each |a| {
                 (visitor.visit_ty)(a.ty, cx, visitor);
@@ -626,7 +646,7 @@ pub fn determine_rp_in_fn(fk: &visit::fn_kind,
 pub fn determine_rp_in_ty_method(ty_m: &ast::ty_method,
                                  &&cx: @mut DetermineRpCtxt,
                                  visitor: visit::vt<@mut DetermineRpCtxt>) {
-    do cx.with(cx.item_id, false) {
+    do cx.with(cx.item_id, false, !ty_m.self_ty.node.is_borrowed()) {
         visit::visit_ty_method(ty_m, cx, visitor);
     }
 }
@@ -735,7 +755,7 @@ pub fn determine_rp_in_ty(ty: @ast::Ty,
       ast::ty_bare_fn(@ast::TyBareFn {decl: ref decl, _}) => {
         // fn() binds the & region, so do not consider &T types that
         // appear *inside* a fn() type to affect the enclosing item:
-        do cx.with(cx.item_id, false) {
+        do cx.with(cx.item_id, false, true) {
             // parameters are contravariant
             do cx.with_ambient_variance(rv_contravariant) {
                 for decl.inputs.each |a| {
@@ -796,6 +816,7 @@ pub fn determine_rp_in_crate(sess: Session,
         worklist: ~[],
         item_id: 0,
         anon_implies_rp: false,
+        self_implies_rp: true,
         ambient_variance: rv_covariant
     };
 

src/librustc/middle/trans/base.rs

@@ -69,7 +69,6 @@ use core::hash;
 use core::int;
 use core::io;
 use core::libc::{c_uint, c_ulonglong};
-use core::option;
 use core::uint;
 use std::oldmap::HashMap;
 use std::{oldmap, time, list};

src/librustc/middle/typeck/astconv.rs

@@ -69,12 +69,11 @@ use syntax::print::pprust::path_to_str;
 use util::common::indenter;
 
 pub trait AstConv {
-    fn tcx(@mut self) -> ty::ctxt;
-    fn ccx(@mut self) -> @mut CrateCtxt;
-    fn get_item_ty(@mut self, id: ast::def_id) -> ty::ty_param_bounds_and_ty;
+    fn tcx(&self) -> ty::ctxt;
+    fn get_item_ty(&self, id: ast::def_id) -> ty::ty_param_bounds_and_ty;
 
     // what type should we use when a type is omitted?
-    fn ty_infer(@mut self, span: span) -> ty::t;
+    fn ty_infer(&self, span: span) -> ty::t;
 }
 
 pub fn get_region_reporting_err(tcx: ty::ctxt,
@@ -92,8 +91,8 @@ pub fn get_region_reporting_err(tcx: ty::ctxt,
 }
 
 pub fn ast_region_to_region<AC:AstConv,RS:region_scope + Copy + Durable>(
-        self: @mut AC,
-        rscope: RS,
+        self: &AC,
+        rscope: &RS,
         span: span,
         a_r: @ast::region)
      -> ty::Region {
@@ -108,8 +107,8 @@ pub fn ast_region_to_region<AC:AstConv,RS:region_scope + Copy + Durable>(
 }
 
 pub fn ast_path_to_substs_and_ty<AC:AstConv,RS:region_scope + Copy + Durable>(
-        self: @mut AC,
-        rscope: RS,
+        self: &AC,
+        rscope: &RS,
         did: ast::def_id,
         path: @ast::path)
      -> ty_param_substs_and_ty {
@@ -164,8 +163,8 @@ pub fn ast_path_to_substs_and_ty<AC:AstConv,RS:region_scope + Copy + Durable>(
 }
 
 pub fn ast_path_to_ty<AC:AstConv,RS:region_scope + Copy + Durable>(
-        self: @mut AC,
-        rscope: RS,
+        self: &AC,
+        rscope: &RS,
         did: ast::def_id,
         path: @ast::path,
         path_id: ast::node_id)
@@ -189,11 +188,11 @@ pub const NO_TPS: uint = 2;
 // Parses the programmer's textual representation of a type into our
 // internal notion of a type. `getter` is a function that returns the type
 // corresponding to a definition ID:
-pub fn ast_ty_to_ty<AC:AstConv,RS:region_scope + Copy + Durable>(
-    self: @mut AC, rscope: RS, &&ast_ty: @ast::Ty) -> ty::t {
+pub fn ast_ty_to_ty<AC:AstConv, RS:region_scope + Copy + Durable>(
+    self: &AC, rscope: &RS, &&ast_ty: @ast::Ty) -> ty::t {
 
-    fn ast_mt_to_mt<AC:AstConv,RS:region_scope + Copy + Durable>(
-        self: @mut AC, rscope: RS, mt: ast::mt) -> ty::mt {
+    fn ast_mt_to_mt<AC:AstConv, RS:region_scope + Copy + Durable>(
+        self: &AC, rscope: &RS, mt: ast::mt) -> ty::mt {
 
         ty::mt {ty: ast_ty_to_ty(self, rscope, mt.ty), mutbl: mt.mutbl}
     }
@@ -202,8 +201,8 @@ pub fn ast_ty_to_ty<AC:AstConv,RS:region_scope + Copy + Durable>(
     // If a_seq_ty is a str or a vec, make it an estr/evec.
     // Also handle function sigils and first-class trait types.
     fn mk_pointer<AC:AstConv,RS:region_scope + Copy + Durable>(
-        self: @mut AC,
-        rscope: RS,
+        self: &AC,
+        rscope: &RS,
         a_seq_ty: ast::mt,
         vst: ty::vstore,
         constr: fn(ty::mt) -> ty::t) -> ty::t
@@ -316,7 +315,8 @@ pub fn ast_ty_to_ty<AC:AstConv,RS:region_scope + Copy + Durable>(
       }
       ast::ty_rptr(region, mt) => {
         let r = ast_region_to_region(self, rscope, ast_ty.span, region);
-        mk_pointer(self, in_anon_rscope(rscope, r), mt, ty::vstore_slice(r),
+        let anon_rscope = in_anon_rscope(rscope, r);
+        mk_pointer(self, &anon_rscope, mt, ty::vstore_slice(r),
                    |tmt| ty::mk_rptr(tcx, r, tmt))
       }
       ast::ty_tup(fields) => {
@@ -419,8 +419,8 @@ pub fn ast_ty_to_ty<AC:AstConv,RS:region_scope + Copy + Durable>(
 }
 
 pub fn ty_of_arg<AC:AstConv,RS:region_scope + Copy + Durable>(
-        self: @mut AC,
-        rscope: RS,
+        self: &AC,
+        rscope: &RS,
         a: ast::arg,
         expected_ty: Option<ty::arg>)
      -> ty::arg {
@@ -467,8 +467,8 @@ pub fn ty_of_arg<AC:AstConv,RS:region_scope + Copy + Durable>(
 }
 
 pub fn ty_of_bare_fn<AC:AstConv,RS:region_scope + Copy + Durable>(
-        self: @mut AC,
-        rscope: RS,
+        self: &AC,
+        rscope: &RS,
         purity: ast::purity,
         abi: ast::Abi,
         decl: &ast::fn_decl)
@@ -479,10 +479,10 @@ pub fn ty_of_bare_fn<AC:AstConv,RS:region_scope + Copy + Durable>(
     // that function type
     let rb = in_binding_rscope(rscope);
 
-    let input_tys = decl.inputs.map(|a| ty_of_arg(self, rb, *a, None));
+    let input_tys = decl.inputs.map(|a| ty_of_arg(self, &rb, *a, None));
     let output_ty = match decl.output.node {
         ast::ty_infer => self.ty_infer(decl.output.span),
-        _ => ast_ty_to_ty(self, rb, decl.output)
+        _ => ast_ty_to_ty(self, &rb, decl.output)
     };
 
     ty::BareFnTy {
@@ -493,8 +493,8 @@ pub fn ty_of_bare_fn<AC:AstConv,RS:region_scope + Copy + Durable>(
 }
 
 pub fn ty_of_closure<AC:AstConv,RS:region_scope + Copy + Durable>(
-        self: @mut AC,
-        rscope: RS,
+        self: &AC,
+        rscope: &RS,
         sigil: ast::Sigil,
         purity: ast::purity,
         onceness: ast::Onceness,
@@ -538,14 +538,14 @@ pub fn ty_of_closure<AC:AstConv,RS:region_scope + Copy + Durable>(
             // were supplied
             if i < e.inputs.len() {Some(e.inputs[i])} else {None}
         };
-        ty_of_arg(self, rb, *a, expected_arg_ty)
+        ty_of_arg(self, &rb, *a, expected_arg_ty)
     };
 
     let expected_ret_ty = expected_tys.map(|e| e.output);
     let output_ty = match decl.output.node {
         ast::ty_infer if expected_ret_ty.is_some() => expected_ret_ty.get(),
         ast::ty_infer => self.ty_infer(decl.output.span),
-        _ => ast_ty_to_ty(self, rb, decl.output)
+        _ => ast_ty_to_ty(self, &rb, decl.output)
     };
 
     ty::ClosureTy {