Skip to content

Commit

Permalink
auto merge of #15325 : pcwalton/rust/trait-impl-bound-mismatch, r=pnk…
Browse files Browse the repository at this point in the history
…felix

with the corresponding trait parameter bounds.

This is a version of the patch in PR #12611 by Florian Hahn, modified to
address Niko's feedback.

It does not address the issue of duplicate type parameter bounds, nor
does it address the issue of implementation-defined methods that contain
*fewer* bounds than the trait, because Niko's review indicates that this
should not be necessary (and indeed I believe it is not). A test has
been added to ensure that this works.

This will break code like:

    trait Foo {
        fn bar<T:Baz>();
    }

    impl Foo for Boo {
        fn bar<T:Baz + Quux>() { ... }
        //             ^~~~ ERROR
    }

This will be rejected because the implementation requires *more* bounds
than the trait. It can be fixed by either adding the missing bound to
the trait:

    trait Foo {
        fn bar<T:Baz + Quux>();
        //             ^~~~
    }

    impl Foo for Boo {
        fn bar<T:Baz + Quux>() { ... }  // OK
    }

Or by removing the bound from the impl:

    trait Foo {
        fn bar<T:Baz>();
    }

    impl Foo for Boo {
        fn bar<T:Baz>() { ... }  // OK
        //       ^ remove Quux
    }

This patch imports the relevant tests from #2687, as well as the test
case in #5886, which is fixed as well by this patch.

Closes #2687.
Closes #5886.

[breaking-change]

r? @pnkfelix
  • Loading branch information
bors committed Jul 3, 2014
2 parents c8ae065 + e56dbad commit 67776ba
Show file tree
Hide file tree
Showing 4 changed files with 200 additions and 43 deletions.
107 changes: 64 additions & 43 deletions src/librustc/middle/typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -920,49 +920,6 @@ fn compare_impl_method(tcx: &ty::ctxt,
let it = trait_m.generics.types.get_vec(subst::FnSpace).iter()
.zip(impl_m.generics.types.get_vec(subst::FnSpace).iter());

for (i, (trait_param_def, impl_param_def)) in it.enumerate() {
// Check that the impl does not require any builtin-bounds
// that the trait does not guarantee:
let extra_bounds =
impl_param_def.bounds.builtin_bounds -
trait_param_def.bounds.builtin_bounds;
if !extra_bounds.is_empty() {
tcx.sess.span_err(
impl_m_span,
format!("in method `{}`, \
type parameter {} requires `{}`, \
which is not required by \
the corresponding type parameter \
in the trait declaration",
token::get_ident(trait_m.ident),
i,
extra_bounds.user_string(tcx)).as_slice());
return;
}

// FIXME(#2687)---we should be checking that the bounds of the
// trait imply the bounds of the subtype, but it appears we
// are...not checking this.
if impl_param_def.bounds.trait_bounds.len() !=
trait_param_def.bounds.trait_bounds.len()
{
let found = impl_param_def.bounds.trait_bounds.len();
let expected = trait_param_def.bounds.trait_bounds.len();
tcx.sess.span_err(
impl_m_span,
format!("in method `{}`, type parameter {} has {} trait \
bound{}, but the corresponding type parameter in \
the trait declaration has {} trait bound{}",
token::get_ident(trait_m.ident),
i,
found,
if found == 1 {""} else {"s"},
expected,
if expected == 1 {""} else {"s"}).as_slice());
return;
}
}

// This code is best explained by example. Consider a trait:
//
// trait Trait<T> {
Expand Down Expand Up @@ -1037,6 +994,70 @@ fn compare_impl_method(tcx: &ty::ctxt,
let trait_fty = ty::mk_bare_fn(tcx, trait_m.fty.clone());
let trait_fty = trait_fty.subst(tcx, &trait_to_skol_substs);

// Check bounds.
for (i, (trait_param_def, impl_param_def)) in it.enumerate() {
// Check that the impl does not require any builtin-bounds
// that the trait does not guarantee:
let extra_bounds =
impl_param_def.bounds.builtin_bounds -
trait_param_def.bounds.builtin_bounds;
if !extra_bounds.is_empty() {
tcx.sess.span_err(
impl_m_span,
format!("in method `{}`, \
type parameter {} requires `{}`, \
which is not required by \
the corresponding type parameter \
in the trait declaration",
token::get_ident(trait_m.ident),
i,
extra_bounds.user_string(tcx)).as_slice());
return;
}

// Check that the trait bounds of the trait imply the bounds of its
// implementation.
//
// FIXME(pcwalton): We could be laxer here regarding sub- and super-
// traits, but I doubt that'll be wanted often, so meh.
for impl_trait_bound in impl_param_def.bounds.trait_bounds.iter() {
let impl_trait_bound =
impl_trait_bound.subst(tcx, &impl_to_skol_substs);

let mut ok = false;
for trait_bound in trait_param_def.bounds.trait_bounds.iter() {
let trait_bound =
trait_bound.subst(tcx, &trait_to_skol_substs);
let infcx = infer::new_infer_ctxt(tcx);
match infer::mk_sub_trait_refs(&infcx,
true,
infer::Misc(impl_m_span),
trait_bound,
impl_trait_bound.clone()) {
Ok(_) => {
ok = true;
break
}
Err(_) => continue,
}
}

if !ok {
tcx.sess.span_err(impl_m_span,
format!("in method `{}`, type parameter {} \
requires bound `{}`, which is not \
required by the corresponding \
type parameter in the trait \
declaration",
token::get_ident(trait_m.ident),
i,
ppaux::trait_ref_to_str(
tcx,
&*impl_trait_bound)).as_slice())
}
}
}

// Check the impl method type IM is a subtype of the trait method
// type TM. To see why this makes sense, think of a vtable. The
// expected type of the function pointers in the vtable is the
Expand Down
78 changes: 78 additions & 0 deletions src/test/compile-fail/trait-bounds-impl-comparison-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
//
// Make sure rustc checks the type parameter bounds in implementations of traits,
// see #2687

trait A {}

trait B: A {}

trait C: A {}

trait Foo {
fn test_error1_fn<T: Eq>(&self);
fn test_error2_fn<T: Eq + Ord>(&self);
fn test_error3_fn<T: Eq + Ord>(&self);
fn test3_fn<T: Eq + Ord>(&self);
fn test4_fn<T: Eq + Ord>(&self);
fn test_error5_fn<T: A>(&self);
fn test6_fn<T: A + Eq>(&self);
fn test_error7_fn<T: A>(&self);
fn test_error8_fn<T: B>(&self);
}

impl Foo for int {
// invalid bound for T, was defined as Eq in trait
fn test_error1_fn<T: Ord>(&self) {}
//~^ ERROR in method `test_error1_fn`, type parameter 0 requires bound `core::cmp::Ord`

// invalid bound for T, was defined as Eq + Ord in trait
fn test_error2_fn<T: Eq + B>(&self) {}
//~^ ERROR in method `test_error2_fn`, type parameter 0 requires bound `B`

// invalid bound for T, was defined as Eq + Ord in trait
fn test_error3_fn<T: B + Eq>(&self) {}
//~^ ERROR in method `test_error3_fn`, type parameter 0 requires bound `B`

// multiple bounds, same order as in trait
fn test3_fn<T: Ord + Eq>(&self) {}

// multiple bounds, different order as in trait
fn test4_fn<T: Eq + Ord>(&self) {}

// parameters in impls must be equal or more general than in the defining trait
fn test_error5_fn<T: B>(&self) {}
//~^ ERROR in method `test_error5_fn`, type parameter 0 requires bound `B`

// bound `std::cmp::Eq` not enforced by this implementation, but this is OK
fn test6_fn<T: A>(&self) {}

fn test_error7_fn<T: A + Eq>(&self) {}
//~^ ERROR in method `test_error7_fn`, type parameter 0 requires bound `core::cmp::Eq`

fn test_error8_fn<T: C>(&self) {}
//~^ ERROR in method `test_error8_fn`, type parameter 0 requires bound `C`
}


trait Getter<T> { }

trait Trait {
fn method<G:Getter<int>>();
}

impl Trait for uint {
fn method<G: Getter<uint>>() {}
//~^ ERROR in method `method`, type parameter 0 requires bound `Getter<uint>`
}

fn main() {}

33 changes: 33 additions & 0 deletions src/test/compile-fail/trait-bounds-impl-comparison-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Issue #5886: a complex instance of issue #2687.

trait Iterator<A> {
fn next(&mut self) -> Option<A>;
}

trait IteratorUtil<A> {
fn zip<B, U: Iterator<U>>(self, other: U) -> ZipIterator<Self, U>;
}

impl<A, T: Iterator<A>> IteratorUtil<A> for T {
fn zip<B, U: Iterator<B>>(self, other: U) -> ZipIterator<T, U> {
//~^ ERROR in method `zip`, type parameter 1 requires bound `Iterator<B>`
ZipIterator{a: self, b: other}
}
}

struct ZipIterator<T, U> {
a: T, b: U
}

fn main() {}

25 changes: 25 additions & 0 deletions src/test/run-pass/trait-bounds-impl-comparison-duplicates.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Tests that type parameter bounds on an implementation need not match the
// trait exactly, as long as the implementation doesn't demand *more* bounds
// than the trait.

trait A {
fn foo<T: Eq + Ord>(&self);
}

impl A for int {
fn foo<T: Ord + Ord>(&self) {}
}

fn main() {}


0 comments on commit 67776ba

Please sign in to comment.