Correctly forbid upvars in nested impls, traits and classes

Previously, resolve was allowing impls, traits or classes that were
nested within a fn to refer to upvars, as well as referring to type
parameters bound by the fn. Fixing this required adding a new kind of
def: def_typaram_binder, which can refer to any of an impl, trait or
class that has bound ty params. resolve uses this to enforce that
methods can refer to their parent item's type parameters, but not to
outer items' type parameters; other stages ignore it. I also made
sure that impl, trait and class methods get checked inside a
MethodRibKind thing so as to forbid upvars, and changed the definition
of MethodRibKind so that its second argument is an optional node_id
(so that required trait method signatures can be checked with a
MethodRibKind as well).

Author: catamorphism

src/libsyntax/ast.rs

@@ -88,6 +88,7 @@ enum def {
               @def    /* closed over def */,
               node_id /* expr node that creates the closure */),
     def_class(def_id, bool /* has constructor */),
+    def_typaram_binder(node_id), /* class, impl or trait that has ty params */
     def_region(node_id)
 }
 

src/libsyntax/ast_util.rs

@@ -55,7 +55,8 @@ pure fn def_id_of_def(d: def) -> def_id {
       def_variant(_, id) | def_ty(id) | def_ty_param(id, _) |
       def_use(id) | def_class(id, _) { id }
       def_arg(id, _) | def_local(id, _) | def_self(id) |
-      def_upvar(id, _, _) | def_binding(id) | def_region(id) {
+      def_upvar(id, _, _) | def_binding(id) | def_region(id)
+      | def_typaram_binder(id) {
         local_def(id)
       }
 

src/rustc/middle/astencode.rs

@@ -373,6 +373,9 @@ impl of tr for ast::def {
             ast::def_class(did.tr(xcx), has_constructor)
           }
           ast::def_region(nid) { ast::def_region(xcx.tr_id(nid)) }
+          ast::def_typaram_binder(nid) {
+            ast::def_typaram_binder(xcx.tr_id(nid))
+          }
         }
     }
 }

src/rustc/middle/borrowck/categorization.rs

@@ -192,12 +192,12 @@ impl public_methods for borrowck_ctxt {
                expr_ty: ty::t,
                def: ast::def) -> cmt {
         alt def {
-          ast::def_fn(_, _) | ast::def_mod(_) |
+          ast::def_fn(*) | ast::def_mod(_) |
           ast::def_foreign_mod(_) | ast::def_const(_) |
-          ast::def_use(_) | ast::def_variant(_, _) |
+          ast::def_use(_) | ast::def_variant(*) |
           ast::def_ty(_) | ast::def_prim_ty(_) |
-          ast::def_ty_param(_, _) | ast::def_class(_, _) |
-          ast::def_region(_) {
+          ast::def_ty_param(*) | ast::def_class(*) |
+          ast::def_typaram_binder(*) | ast::def_region(_) {
             @{id:id, span:span,
               cat:cat_special(sk_static_item), lp:none,
               mutbl:m_imm, ty:expr_ty}

src/rustc/middle/resolve3.rs

@@ -10,7 +10,8 @@ import syntax::ast::{bound_send, capture_clause, class_ctor, class_dtor};
 import syntax::ast::{class_member, class_method, crate, crate_num, decl_item};
 import syntax::ast::{def, def_arg, def_binding, def_class, def_const, def_fn};
 import syntax::ast::{def_foreign_mod, def_id, def_local, def_mod};
-import syntax::ast::{def_prim_ty, def_region, def_self, def_ty, def_ty_param};
+import syntax::ast::{def_prim_ty, def_region, def_self, def_ty, def_ty_param,
+                     def_typaram_binder};
 import syntax::ast::{def_upvar, def_use, def_variant, expr, expr_assign_op};
 import syntax::ast::{expr_binary, expr_cast, expr_field, expr_fn};
 import syntax::ast::{expr_fn_block, expr_index, expr_new, expr_path};
@@ -171,10 +172,23 @@ enum RibKind {
     // upvars as appropriate.
     FunctionRibKind(node_id),
 
+    // We passed through a class, impl, or trait and are now in one of its
+    // methods. Allow references to ty params that that class, impl or trait
+    // binds. Disallow any other upvars (including other ty params that are
+    // upvars).
+              // parent;   method itself
+    MethodRibKind(node_id, MethodSort),
+
     // We passed through a function *item* scope. Disallow upvars.
     OpaqueFunctionRibKind
 }
 
+// Methods can be required or provided. Required methods only occur in traits.
+enum MethodSort {
+    Required,
+    Provided(node_id)
+}
+
 // The X-ray flag indicates that a context has the X-ray privilege, which
 // allows it to reference private names. Currently, this is used for the test
 // runner.
@@ -1392,7 +1406,8 @@ class Resolver {
                         }
                         def_self(*) | def_arg(*) | def_local(*) |
                         def_prim_ty(*) | def_ty_param(*) | def_binding(*) |
-                        def_use(*) | def_upvar(*) | def_region(*) {
+                        def_use(*) | def_upvar(*) | def_region(*) |
+                          def_typaram_binder(*) {
                             fail #fmt("didn't expect `%?`", def);
                         }
                     }
@@ -2891,6 +2906,36 @@ class Resolver {
                                         function_id);
                     }
                 }
+                MethodRibKind(item_id, method_id) {
+                  // If the def is a ty param, and came from the parent
+                  // item, it's ok
+                  alt def {
+                    def_ty_param(did, _) if self.def_map.find(copy(did.node))
+                      == some(def_typaram_binder(item_id)) {
+                      // ok
+                    }
+                    _ {
+                    if !is_ty_param {
+                        // This was an attempt to access an upvar inside a
+                        // named function item. This is not allowed, so we
+                        // report an error.
+
+                        self.session.span_err(
+                            span,
+                            ~"attempted dynamic environment-capture");
+                    } else {
+                        // This was an attempt to use a type parameter outside
+                        // its scope.
+
+                        self.session.span_err(span,
+                                              ~"attempt to use a type \
+                                               argument out of scope");
+                    }
+
+                    ret none;
+                    }
+                  }
+                }
                 OpaqueFunctionRibKind {
                     if !is_ty_param {
                         // This was an attempt to access an upvar inside a
@@ -3029,7 +3074,7 @@ class Resolver {
                                 (HasTypeParameters(&ty_m.tps,
                                                    item.id,
                                                    type_parameters.len(),
-                                                   NormalRibKind)) {
+                                        MethodRibKind(item.id, Required))) {
 
                                 // Resolve the method-specific type
                                 // parameters.
@@ -3044,7 +3089,8 @@ class Resolver {
                             }
                           }
                           provided(m) {
-                              self.resolve_method(NormalRibKind,
+                              self.resolve_method(MethodRibKind(item.id,
+                                                     Provided(m.id)),
                                                   m,
                                                   type_parameters.len(),
                                                   visitor)
@@ -3148,9 +3194,15 @@ class Resolver {
                 for (*type_parameters).eachi |index, type_parameter| {
                     let name =
                         (*self.atom_table).intern(type_parameter.ident);
+                    #debug("with_type_parameter_rib: %d %d", node_id,
+                           type_parameter.id);
                     let def_like = dl_def(def_ty_param
                         (local_def(type_parameter.id),
                          index + initial_index));
+                    // Associate this type parameter with
+                    // the item that bound it
+                    self.record_def(type_parameter.id,
+                                    def_typaram_binder(node_id));
                     (*function_type_rib).bindings.insert(name, def_like);
                 }
             }
@@ -3332,7 +3384,8 @@ class Resolver {
             for class_members.each |class_member| {
                 alt class_member.node {
                     class_method(method) {
-                      self.resolve_method(NormalRibKind,
+                      self.resolve_method(MethodRibKind(id,
+                                               Provided(method.id)),
                                           method,
                                           outer_type_parameter_count,
                                           visitor);
@@ -3451,7 +3504,7 @@ class Resolver {
                 // type parameters.
 
                 let borrowed_type_parameters = &method.tps;
-                self.resolve_function(NormalRibKind,
+                self.resolve_function(MethodRibKind(id, Provided(method.id)),
                                       some(@method.decl),
                                       HasTypeParameters
                                         (borrowed_type_parameters,

src/rustc/middle/typeck/check.rs

@@ -2186,6 +2186,10 @@ fn ty_param_bounds_and_ty_for_def(fcx: @fn_ctxt, sp: span, defn: ast::def) ->
       ast::def_region(*) {
         fcx.ccx.tcx.sess.span_fatal(sp, ~"expected value but found region");
       }
+      ast::def_typaram_binder(*) {
+        fcx.ccx.tcx.sess.span_fatal(sp, ~"expected value but found type \
+                                          parameter");
+      }
     }
 }
 

src/rustc/middle/typeck/check/regionmanip.rs

@@ -200,8 +200,8 @@ fn region_of(fcx: @fn_ctxt, expr: @ast::expr) -> ty::region {
           ast::def_foreign_mod(_) | ast::def_const(_) |
           ast::def_use(_) | ast::def_variant(_, _) |
           ast::def_ty(_) | ast::def_prim_ty(_) |
-          ast::def_ty_param(_, _) | ast::def_class(_, _) |
-          ast::def_region(_) {
+          ast::def_ty_param(_, _) | ast::def_typaram_binder(*) |
+          ast::def_class(_, _) | ast::def_region(_) {
             ty::re_static
           }
         }

src/test/compile-fail/issue-3021-b.rs

@@ -0,0 +1,15 @@
+use std;
+
+fn siphash(k0 : u64) {
+
+    class siphash {
+        let mut v0: u64;
+        fn reset() {
+           self.v0 = k0 ^ 0x736f6d6570736575; //~ ERROR attempted dynamic environment-capture
+           //~^ ERROR unresolved name: k0
+        }
+        new() { self.v0 = 0; }
+    }
+}
+
+fn main() {}

src/test/compile-fail/issue-3021-c.rs

@@ -0,0 +1,13 @@
+use std;
+
+fn siphash<T>() {
+
+    trait t {
+        fn g(x: T) -> T;  //~ ERROR attempt to use a type argument out of scope
+        //~^ ERROR attempt to use a type argument out of scope
+        //~^^ ERROR use of undeclared type name `T`
+        //~^^^ ERROR use of undeclared type name `T`
+    }
+}
+
+fn main() {}

src/test/compile-fail/issue-3021-d.rs

@@ -0,0 +1,32 @@
+use std;
+
+iface siphash {
+    fn result() -> u64;
+    fn reset();
+}
+
+fn siphash(k0 : u64, k1 : u64) -> siphash {
+    type sipstate = {
+        mut v0 : u64,
+        mut v1 : u64,
+    };
+
+    fn mk_result(st : sipstate) -> u64 {
+
+        let v0 = st.v0,
+            v1 = st.v1;
+        ret v0 ^ v1;
+    }
+
+   impl of siphash for sipstate {
+        fn reset() {
+            self.v0 = k0 ^ 0x736f6d6570736575;  //~ ERROR attempted dynamic environment-capture
+            //~^ ERROR unresolved name: k0
+            self.v1 = k1 ^ 0x646f72616e646f6d;   //~ ERROR attempted dynamic environment-capture
+            //~^ ERROR unresolved name: k1
+        }
+        fn result() -> u64 { ret mk_result(self); }
+    }
+}
+
+fn main() {}

src/test/compile-fail/issue-3021.rs

@@ -0,0 +1,22 @@
+use std;
+
+iface siphash {
+    fn reset();
+}
+
+fn siphash(k0 : u64) -> siphash {
+    type sipstate = {
+        mut v0 : u64,
+    };
+
+
+   impl of siphash for sipstate {
+        fn reset() {
+           self.v0 = k0 ^ 0x736f6d6570736575; //~ ERROR attempted dynamic environment-capture
+           //~^ ERROR unresolved name: k0
+        }
+    }
+    fail;
+}
+
+fn main() {}