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

7 files changed, 115 insertions, 9 deletions

diff --git a/src/rustc/middle/typeck/collect.rs b/src/rustc/middle/typeck/collect.rs
index 43d6cf14d93..3206d1709c5 100644
--- a/src/rustc/middle/typeck/collect.rs
+++ b/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`
diff --git a/src/rustc/util/common.rs b/src/rustc/util/common.rs
index ee941c0ed01..901d5d504eb 100644
--- a/src/rustc/util/common.rs
+++ b/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
diff --git a/src/rustc/util/ppaux.rs b/src/rustc/util/ppaux.rs
index f78495ef198..3a27a521985 100644
--- a/src/rustc/util/ppaux.rs
+++ b/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 {
diff --git a/src/test/compile-fail/issue-2611-3.rs b/src/test/compile-fail/issue-2611-3.rs
new file mode 100644
index 00000000000..172fb81d3bb
--- /dev/null
+++ b/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() {}
\ No newline at end of file
diff --git a/src/test/compile-fail/issue-2611-4.rs b/src/test/compile-fail/issue-2611-4.rs
new file mode 100644
index 00000000000..95f5685d84f
--- /dev/null
+++ b/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() {}
\ No newline at end of file
diff --git a/src/test/compile-fail/issue-2611-5.rs b/src/test/compile-fail/issue-2611-5.rs
new file mode 100644
index 00000000000..4278dcb2fb6
--- /dev/null
+++ b/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() {}
\ No newline at end of file
diff --git a/src/test/run-pass/issue-2611.rs b/src/test/run-pass/issue-2611.rs
index 9f1ef849830..94456ecb25b 100644
--- a/src/test/run-pass/issue-2611.rs
+++ b/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)
    }
 }