From ac1f84c153a171e641233e5d2d11404a0b520986 Mon Sep 17 00:00:00 2001 From: Tim Chevalier Date: Wed, 5 Sep 2012 15:36:11 -0700 Subject: [PATCH] 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 --- src/rustc/middle/typeck/collect.rs | 34 ++++++++++++++++++++++++--- src/rustc/util/common.rs | 5 ++++ src/rustc/util/ppaux.rs | 14 ++++++++++- src/test/compile-fail/issue-2611-3.rs | 21 +++++++++++++++++ src/test/compile-fail/issue-2611-4.rs | 18 ++++++++++++++ src/test/compile-fail/issue-2611-5.rs | 19 +++++++++++++++ src/test/run-pass/issue-2611.rs | 13 ++++++---- 7 files changed, 115 insertions(+), 9 deletions(-) create mode 100644 src/test/compile-fail/issue-2611-3.rs create mode 100644 src/test/compile-fail/issue-2611-4.rs create mode 100644 src/test/compile-fail/issue-2611-5.rs 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(x: C) -> C; +} + +struct E { + f: int; +} + +impl E: A { + fn b(_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(x: C) -> C; +} + +struct E { + f: int; +} + +impl E: A { + fn b(_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(x: C) -> C; +} + +struct E { + f: int; +} + +impl E: A { + // n.b. The error message is awful -- see #3404 + fn b(_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 for base_iter { - fn flat_map_to_vec>(op: fn(B) -> IB) -> ~[B] { - iter::flat_map_to_vec(self, op) +trait FlatMapToVec { + fn flat_map_to_vec>(op: fn(A) -> IB) -> ~[B]; +} + +impl BaseIter: FlatMapToVec { + fn flat_map_to_vec>(op: fn(A) -> IB) -> ~[B] { + iter::flat_map_to_vec(self, op) } }