Skip to content

Commit

Permalink
Don't check impl ty params for equality with trait ty params
Browse files Browse the repository at this point in the history
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 rust-lang#2611
  • Loading branch information
catamorphism committed Sep 7, 2012
1 parent c6b5154 commit ac1f84c
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 9 deletions.
34 changes: 31 additions & 3 deletions src/rustc/middle/typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

Expand Down Expand Up @@ -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;
}

Expand All @@ -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`
Expand Down
5 changes: 5 additions & 0 deletions src/rustc/util/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 13 additions & 1 deletion src/rustc/util/ppaux.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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 {
Expand Down
21 changes: 21 additions & 0 deletions src/test/compile-fail/issue-2611-3.rs
Original file line number Diff line number Diff line change
@@ -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() {}
18 changes: 18 additions & 0 deletions src/test/compile-fail/issue-2611-4.rs
Original file line number Diff line number Diff line change
@@ -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() {}
19 changes: 19 additions & 0 deletions src/test/compile-fail/issue-2611-5.rs
Original file line number Diff line number Diff line change
@@ -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() {}
13 changes: 8 additions & 5 deletions src/test/run-pass/issue-2611.rs
Original file line number Diff line number Diff line change
@@ -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)
}
}

Expand Down

0 comments on commit ac1f84c

Please sign in to comment.