From e56dbad9f7c3691bcb12edfaeda5cb3b660496de Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Tue, 1 Jul 2014 21:59:16 -0700 Subject: [PATCH] librustc: Properly compare implementation method type parameter bounds 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(); } impl Foo for Boo { fn bar() { ... } // ^~~~ 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(); // ^~~~ } impl Foo for Boo { fn bar() { ... } // OK } Or by removing the bound from the impl: trait Foo { fn bar(); } impl Foo for Boo { fn bar() { ... } // 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] --- src/librustc/middle/typeck/check/mod.rs | 107 +++++++++++------- .../trait-bounds-impl-comparison-1.rs | 78 +++++++++++++ .../trait-bounds-impl-comparison-2.rs | 33 ++++++ ...trait-bounds-impl-comparison-duplicates.rs | 25 ++++ 4 files changed, 200 insertions(+), 43 deletions(-) create mode 100644 src/test/compile-fail/trait-bounds-impl-comparison-1.rs create mode 100644 src/test/compile-fail/trait-bounds-impl-comparison-2.rs create mode 100644 src/test/run-pass/trait-bounds-impl-comparison-duplicates.rs diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index d808120db99ba..0cef67ec2e557 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/src/librustc/middle/typeck/check/mod.rs @@ -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 { @@ -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 diff --git a/src/test/compile-fail/trait-bounds-impl-comparison-1.rs b/src/test/compile-fail/trait-bounds-impl-comparison-1.rs new file mode 100644 index 0000000000000..eec116855c986 --- /dev/null +++ b/src/test/compile-fail/trait-bounds-impl-comparison-1.rs @@ -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 or the MIT license +// , 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(&self); + fn test_error2_fn(&self); + fn test_error3_fn(&self); + fn test3_fn(&self); + fn test4_fn(&self); + fn test_error5_fn(&self); + fn test6_fn(&self); + fn test_error7_fn(&self); + fn test_error8_fn(&self); +} + +impl Foo for int { + // invalid bound for T, was defined as Eq in trait + fn test_error1_fn(&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(&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(&self) {} + //~^ ERROR in method `test_error3_fn`, type parameter 0 requires bound `B` + + // multiple bounds, same order as in trait + fn test3_fn(&self) {} + + // multiple bounds, different order as in trait + fn test4_fn(&self) {} + + // parameters in impls must be equal or more general than in the defining trait + fn test_error5_fn(&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(&self) {} + + fn test_error7_fn(&self) {} + //~^ ERROR in method `test_error7_fn`, type parameter 0 requires bound `core::cmp::Eq` + + fn test_error8_fn(&self) {} + //~^ ERROR in method `test_error8_fn`, type parameter 0 requires bound `C` +} + + +trait Getter { } + +trait Trait { + fn method>(); +} + +impl Trait for uint { + fn method>() {} + //~^ ERROR in method `method`, type parameter 0 requires bound `Getter` +} + +fn main() {} + diff --git a/src/test/compile-fail/trait-bounds-impl-comparison-2.rs b/src/test/compile-fail/trait-bounds-impl-comparison-2.rs new file mode 100644 index 0000000000000..a970a86408e90 --- /dev/null +++ b/src/test/compile-fail/trait-bounds-impl-comparison-2.rs @@ -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 or the MIT license +// , 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 { + fn next(&mut self) -> Option; +} + +trait IteratorUtil { + fn zip>(self, other: U) -> ZipIterator; +} + +impl> IteratorUtil for T { + fn zip>(self, other: U) -> ZipIterator { + //~^ ERROR in method `zip`, type parameter 1 requires bound `Iterator` + ZipIterator{a: self, b: other} + } +} + +struct ZipIterator { + a: T, b: U +} + +fn main() {} + diff --git a/src/test/run-pass/trait-bounds-impl-comparison-duplicates.rs b/src/test/run-pass/trait-bounds-impl-comparison-duplicates.rs new file mode 100644 index 0000000000000..55ad22dd0bfdf --- /dev/null +++ b/src/test/run-pass/trait-bounds-impl-comparison-duplicates.rs @@ -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 or the MIT license +// , 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(&self); +} + +impl A for int { + fn foo(&self) {} +} + +fn main() {} + +