Address comments from @pnkfelix (thanks for the detailed review)

Author: nikomatsakis

src/librustc/middle/ty.rs

@@ -218,10 +218,10 @@ pub struct ItemVariances {
 
 #[deriving(Clone, Eq, Decodable, Encodable)]
 pub enum Variance {
-    Covariant,
-    Invariant,
-    Contravariant,
-    Bivariant,
+    Covariant,      // T<A> <: T<B> iff A <: B -- e.g., function return type
+    Invariant,      // T<A> <: T<B> iff B == A -- e.g., type of mutable cell
+    Contravariant,  // T<A> <: T<B> iff B <: A -- e.g., function param type
+    Bivariant,      // T<A> <: T<B>            -- e.g., unused type parameter
 }
 
 #[deriving(Decodable, Encodable)]

src/librustc/middle/typeck/astconv.rs

@@ -116,7 +116,7 @@ pub fn ast_region_to_region(
     r
 }
 
-pub fn opt_ast_region_to_region<AC:AstConv,RS:RegionScope>(
+fn opt_ast_region_to_region<AC:AstConv,RS:RegionScope>(
     this: &AC,
     rscope: &RS,
     default_span: Span,
@@ -129,14 +129,14 @@ pub fn opt_ast_region_to_region<AC:AstConv,RS:RegionScope>(
 
         None => {
             match rscope.anon_regions(default_span, 1) {
-                None => {
+                Err(()) => {
                     debug!("optional region in illegal location");
                     this.tcx().sess.span_err(
                         default_span, "missing lifetime specifier");
                     ty::ReStatic
                 }
 
-                Some(rs) => {
+                Ok(rs) => {
                     rs[0]
                 }
             }
@@ -178,7 +178,7 @@ fn ast_path_substs<AC:AstConv,RS:RegionScope>(
         let anon_regions =
             rscope.anon_regions(path.span, expected_num_region_params);
 
-        if supplied_num_region_params != 0 || anon_regions.is_none() {
+        if supplied_num_region_params != 0 || anon_regions.is_err() {
             tcx.sess.span_err(
                 path.span,
                 format!("wrong number of lifetime parameters: \
@@ -188,9 +188,9 @@ fn ast_path_substs<AC:AstConv,RS:RegionScope>(
         }
 
         match anon_regions {
-            Some(v) => opt_vec::from(v),
-            None => opt_vec::from(vec::from_fn(expected_num_region_params,
-                                               |_| ty::ReStatic)) // hokey
+            Ok(v) => opt_vec::from(v),
+            Err(()) => opt_vec::from(vec::from_fn(expected_num_region_params,
+                                                  |_| ty::ReStatic)) // hokey
         }
     };
 
@@ -277,8 +277,7 @@ pub static NO_REGIONS: uint = 1;
 pub static 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:
+// internal notion of a type.
 pub fn ast_ty_to_ty<AC:AstConv, RS:RegionScope>(
     this: &AC, rscope: &RS, ast_ty: &ast::Ty) -> ty::t {
 

src/librustc/middle/typeck/check/mod.rs

@@ -800,7 +800,6 @@ fn check_impl_methods_against_trait(ccx: @mut CrateCtxt,
  * - impl_m_body_id: id of the method body
  * - trait_m: the method in the trait
  * - trait_substs: the substitutions used on the type of the trait
- * - self_ty: the self type of the impl
  */
 pub fn compare_impl_method(tcx: ty::ctxt,
                            impl_generics: &ty::Generics,
@@ -1062,8 +1061,8 @@ impl FnCtxt {
 impl RegionScope for @mut infer::InferCtxt {
     fn anon_regions(&self,
                     span: Span,
-                    count: uint) -> Option<~[ty::Region]> {
-        Some(vec::from_fn(
+                    count: uint) -> Result<~[ty::Region], ()> {
+        Ok(vec::from_fn(
                 count,
                 |_| self.next_region_var(infer::MiscVariable(span))))
     }

src/librustc/middle/typeck/collect.rs

@@ -517,17 +517,17 @@ pub fn convert(ccx: &CrateCtxt, it: &ast::item) {
     let tcx = ccx.tcx;
     debug!("convert: item {} with id {}", tcx.sess.str_of(it.ident), it.id);
     match it.node {
-        // These don't define types.
-        ast::item_foreign_mod(_) | ast::item_mod(_) => {}
-        ast::item_enum(ref enum_definition, ref generics) => {
-            ensure_no_ty_param_bounds(ccx, it.span, generics, "enumeration");
-            let tpt = ty_of_item(ccx, it);
-            write_ty_to_tcx(tcx, it.id, tpt.ty);
-            get_enum_variant_types(ccx,
-                                   tpt.ty,
-                                   enum_definition.variants,
-                                   generics);
-        }
+      // These don't define types.
+      ast::item_foreign_mod(_) | ast::item_mod(_) => {}
+      ast::item_enum(ref enum_definition, ref generics) => {
+          ensure_no_ty_param_bounds(ccx, it.span, generics, "enumeration");
+          let tpt = ty_of_item(ccx, it);
+          write_ty_to_tcx(tcx, it.id, tpt.ty);
+          get_enum_variant_types(ccx,
+                                 tpt.ty,
+                                 enum_definition.variants,
+                                 generics);
+      }
       ast::item_impl(ref generics, ref opt_trait_ref, ref selfty, ref ms) => {
         let i_ty_generics = ty_generics(ccx, generics, 0);
         let selfty = ccx.to_ty(&ExplicitRscope, selfty);

src/librustc/middle/typeck/rscope.rs

@@ -16,11 +16,21 @@ use syntax::ast;
 use syntax::codemap::Span;
 use syntax::opt_vec::OptVec;
 
+/// Defines strategies for handling regions that are omitted.  For
+/// example, if one writes the type `&Foo`, then the lifetime of of
+/// this borrowed pointer has been omitted. When converting this
+/// type, the generic functions in astconv will invoke `anon_regions`
+/// on the provided region-scope to decide how to translate this
+/// omitted region.
+///
+/// It is not always legal to omit regions, therefore `anon_regions`
+/// can return `Err(())` to indicate that this is not a scope in which
+/// regions can legally be omitted.
 pub trait RegionScope {
     fn anon_regions(&self,
                     span: Span,
                     count: uint)
-                    -> Option<~[ty::Region]>;
+                    -> Result<~[ty::Region], ()>;
 }
 
 // A scope in which all regions must be explicitly named
@@ -30,11 +40,13 @@ impl RegionScope for ExplicitRscope {
     fn anon_regions(&self,
                     _span: Span,
                     _count: uint)
-                    -> Option<~[ty::Region]> {
-        None
+                    -> Result<~[ty::Region], ()> {
+        Err(())
     }
 }
 
+/// A scope in which we generate anonymous, late-bound regions for
+/// omitted regions. This occurs in function signatures.
 pub struct BindingRscope {
     binder_id: ast::NodeId,
     anon_bindings: @mut uint
@@ -53,11 +65,11 @@ impl RegionScope for BindingRscope {
     fn anon_regions(&self,
                     _: Span,
                     count: uint)
-                    -> Option<~[ty::Region]> {
+                    -> Result<~[ty::Region], ()> {
         let idx = *self.anon_bindings;
         *self.anon_bindings += count;
-        Some(vec::from_fn(count, |i| ty::ReLateBound(self.binder_id,
-                                                     ty::BrAnon(idx + i))))
+        Ok(vec::from_fn(count, |i| ty::ReLateBound(self.binder_id,
+                                                   ty::BrAnon(idx + i))))
     }
 }