Don't check impl ty params for equality with trait ty params

This was too restrictive. We need to check the number of ty params,
and that the bounds are equal, but otherwise require_same_types does the job.

Closes #2611

Author: catamorphism

src/rustc/middle/typeck/collect.rs

@@ -24,6 +24,8 @@ use astconv::{ast_conv, ty_of_fn_decl, ty_of_arg, ast_ty_to_ty};
 use ast_util::trait_method_to_ty_method;
 use rscope::*;
 use ty::{FnTyBase, FnMeta, FnSig};
+use util::common::pluralize;
+use util::ppaux::bound_to_str;
 
 fn collect_item_types(ccx: @crate_ctxt, crate: @ast::crate) {
 
@@ -263,9 +265,13 @@ fn compare_impl_method(tcx: ty::ctxt, sp: span,
                           self type", tcx.sess.str_of(impl_m.ident)));
     }
 
-    if impl_m.tps != trait_m.tps {
-        tcx.sess.span_err(sp, ~"method `" + tcx.sess.str_of(trait_m.ident) +
-                          ~"` has an incompatible set of type parameters");
+    if impl_m.tps.len() != trait_m.tps.len() {
+        tcx.sess.span_err(sp, #fmt("method `%s` \
+           has %u type %s, but its trait declaration has %u type %s",
+           tcx.sess.str_of(trait_m.ident), impl_m.tps.len(),
+           pluralize(impl_m.tps.len(), ~"parameter"),
+           trait_m.tps.len(),
+           pluralize(trait_m.tps.len(), ~"parameter")));
         return;
     }
 
@@ -278,6 +284,28 @@ fn compare_impl_method(tcx: ty::ctxt, sp: span,
         return;
     }
 
+    for trait_m.tps.eachi() |i, trait_param_bounds| {
+        // For each of the corresponding impl ty param's bounds...
+        let impl_param_bounds = impl_m.tps[i];
+        // Make sure the bounds lists have the same length
+        // Would be nice to use the ty param names in the error message,
+        // but we don't have easy access to them here
+        if impl_param_bounds.len() != trait_param_bounds.len() {
+           tcx.sess.span_err(sp, #fmt("in method `%s`, \
+             type parameter %u has %u %s, but the same type \
+             parameter in its trait declaration has %u %s",
+             tcx.sess.str_of(trait_m.ident),
+             i, impl_param_bounds.len(),
+             pluralize(impl_param_bounds.len(), ~"bound"),
+             trait_param_bounds.len(),
+             pluralize(trait_param_bounds.len(), ~"bound")));
+           return;
+        }
+        // tjc: I'm mildly worried that there's something I'm
+        // not checking that require_same_types doesn't catch,
+        // but I can't figure out what.
+    }
+
     // Perform substitutions so that the trait/impl methods are expressed
     // in terms of the same set of type/region parameters:
     // - replace trait type parameters with those from `trait_substs`

src/rustc/util/common.rs

@@ -94,6 +94,11 @@ fn is_main_name(path: syntax::ast_map::path) -> bool {
     )
 }
 
+fn pluralize(n: uint, s: ~str) -> ~str {
+    if n == 1 { s }
+    else { str::concat([s, ~"s"]) }
+}
+
 //
 // Local Variables:
 // mode: rust

src/rustc/util/ppaux.rs

@@ -1,9 +1,11 @@
 use std::map::hashmap;
 use middle::ty;
 use middle::ty::{arg, canon_mode};
+use middle::ty::{bound_copy, bound_const, bound_owned, bound_send,
+        bound_trait};
 use middle::ty::{bound_region, br_anon, br_named, br_self, br_cap_avoid};
 use middle::ty::{ck_block, ck_box, ck_uniq, ctxt, field, method};
-use middle::ty::{mt, t};
+use middle::ty::{mt, t, param_bound};
 use middle::ty::{re_bound, re_free, re_scope, re_var, re_static, region};
 use middle::ty::{ty_bool, ty_bot, ty_box, ty_class, ty_enum};
 use middle::ty::{ty_estr, ty_evec, ty_float, ty_fn, ty_trait, ty_int};
@@ -233,6 +235,16 @@ fn tys_to_str(cx: ctxt, ts: ~[t]) -> ~str {
     rs
 }
 
+fn bound_to_str(cx: ctxt, b: param_bound) -> ~str {
+    match b {
+      bound_copy     => ~"copy",
+      bound_owned    => ~"owned",
+      bound_send     => ~"send",
+      bound_const    => ~"const",
+      bound_trait(t) => ty_to_str(cx, t)
+    }
+}
+
 fn ty_to_str(cx: ctxt, typ: t) -> ~str {
     fn fn_input_to_str(cx: ctxt, input: {mode: ast::mode, ty: t}) ->
        ~str {

src/test/compile-fail/issue-2611-3.rs

@@ -0,0 +1,21 @@
+// Tests that impl methods are matched to traits exactly:
+// we might be tempted to think matching is contravariant, but if
+// we let an impl method can have more permissive bounds than the trait
+// method it's implementing, the return type might be less specific than
+// needed. Just punt and make it invariant.
+import iter;
+import iter::BaseIter;
+
+trait A {
+  fn b<C:copy const, D>(x: C) -> C;
+}
+
+struct E {
+ f: int;
+}
+
+impl E: A {
+  fn b<F:copy, G>(_x: F) -> F { fail } //~ ERROR in method `b`, type parameter 0 has 1 bound, but
+}
+
+fn main() {}

src/test/compile-fail/issue-2611-4.rs

@@ -0,0 +1,18 @@
+// Tests that an impl method's bounds aren't *more* restrictive
+// than the trait method it's implementing
+import iter;
+import iter::BaseIter;
+
+trait A {
+  fn b<C:copy, D>(x: C) -> C;
+}
+
+struct E {
+ f: int;
+}
+
+impl E: A {
+  fn b<F:copy const, G>(_x: F) -> F { fail } //~ ERROR in method `b`, type parameter 0 has 2 bounds, but
+}
+
+fn main() {}

src/test/compile-fail/issue-2611-5.rs

@@ -0,0 +1,19 @@
+// Tests that ty params get matched correctly when comparing
+// an impl against a trait
+import iter;
+import iter::BaseIter;
+
+trait A {
+  fn b<C:copy, D>(x: C) -> C;
+}
+
+struct E {
+ f: int;
+}
+
+impl E: A {
+  // n.b. The error message is awful -- see #3404
+  fn b<F:copy, G>(_x: G) -> G { fail } //~ ERROR method `b` has an incompatible type
+}
+
+fn main() {}

src/test/run-pass/issue-2611.rs

@@ -1,9 +1,12 @@
-// xfail-test
-use iter::base_iter;
+use iter::BaseIter;
 
-impl Q<A> for base_iter<A> {
-   fn flat_map_to_vec<B:copy, IB:base_iter<B>>(op: fn(B) -> IB) -> ~[B] {
-      iter::flat_map_to_vec(self, op)
+trait FlatMapToVec<A> {
+  fn flat_map_to_vec<B:copy, IB:BaseIter<B>>(op: fn(A) -> IB) -> ~[B];
+}
+
+impl<A:copy> BaseIter<A>: FlatMapToVec<A> {
+   fn flat_map_to_vec<B:copy, IB:BaseIter<B>>(op: fn(A) -> IB) -> ~[B] {
+     iter::flat_map_to_vec(self, op)
    }
 }