Add test for comparing impl method type with trait bounds

src/librustc/middle/lint.rs

@@ -102,6 +102,7 @@ pub enum Lint {
     DeadCode,
     VisiblePrivateTypes,
     UnnecessaryTypecast,
+    DuplicatedTypeBound,
 
     MissingDoc,
     UnreachableCode,
@@ -304,6 +305,13 @@ static lint_table: &'static [(&'static str, LintSpec)] = &[
         default: allow,
     }),
 
+    ("duplicated_type_bound",
+     LintSpec {
+        lint: DuplicatedTypeBound,
+        desc: "detects duplicated type bound",
+        default: warn
+    }),
+
     ("unused_mut",
      LintSpec {
         lint: UnusedMut,

src/librustc/middle/typeck/check/mod.rs

@@ -112,15 +112,16 @@ use middle::lang_items::TypeIdLangItem;
 use util::common::{block_query, indenter, loop_query};
 use util::ppaux;
 use util::ppaux::{UserString, Repr};
-use util::nodemap::{FnvHashMap, NodeMap};
+use util::nodemap::{NodeMap, FnvHashMap};
+use collections::{HashMap, HashSet};
 
 use std::cell::{Cell, RefCell};
-use collections::HashMap;
 use std::mem::replace;
 use std::result;
 use std::vec;
 use std::vec_ng::Vec;
 use std::vec_ng;
+use std::str::StrVector;
 use syntax::abi::AbiSet;
 use syntax::ast::{Provided, Required};
 use syntax::ast;
@@ -864,25 +865,60 @@ fn compare_impl_method(tcx: ty::ctxt,
            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()
-        {
+        // check that the bounds of the trait imply the bounds of the implementation type
+        let mut enforced_bounds = HashSet::new();
+        let mut unspecified_bounds = ~[];
+        for impl_bound in impl_param_def.bounds.trait_bounds.iter() {
+            let mut specified = false;
+            for trait_bound in trait_param_def.bounds.trait_bounds.iter() {
+                if !enforced_bounds.contains(&trait_bound.def_id) {
+                    match infer::mk_sub_trait_refs(&infcx,
+                                                   false,
+                                                   infer::RelateTraitRefs(impl_m_span),
+                                                   *impl_bound,
+                                                   *trait_bound) {
+                        result::Ok(()) => {
+                            enforced_bounds.insert(trait_bound.def_id);
+                            specified = true;
+                        } // Ok.
+                        result::Err(_) => {}
+                    }
+                }
+            }
+            if !specified {
+                unspecified_bounds.push(format!("`{}`", impl_bound.repr(tcx)));
+            }
+        }
+
+        let unenforced_bounds: ~[~str] =
+            trait_param_def.bounds.trait_bounds.iter().filter_map(|&trait_bound| {
+            if !enforced_bounds.contains(&trait_bound.def_id) {
+                Some(format!("`{}`", trait_bound.repr(tcx)))
+            } else {
+                None
+            }
+        }).collect();
+
+        if unenforced_bounds.len() > 0 {
+             tcx.sess.span_err(
+                impl_m_span,
+                format!("in method `{method}`, \
+                        {nbounds, plural, =1{trait bound} other{trait bounds}} \
+                        {bounds} not enforced by this implementation",
+                        method = token::get_ident(trait_m.ident),
+                        nbounds = unenforced_bounds.len(),
+                        bounds = unenforced_bounds.connect(", ")));
+        }
+        if unspecified_bounds.len() > 0 {
             tcx.sess.span_err(
                 impl_m_span,
                 format!("in method `{method}`, \
-                        type parameter {typaram} has \
-                        {nimpl, plural, =1{# trait bound} other{# trait bounds}}, \
-                        but the corresponding type parameter in \
-                        the trait declaration has \
-                        {ntrait, plural, =1{# trait bound} other{# trait bounds}}",
+                        implementation {nbounds, plural, =1{bound} other{bounds}} \
+                        {bounds} {nbounds, plural, =1{was} other{were}} \
+                        not specified in trait definition",
                         method = token::get_ident(trait_m.ident),
-                        typaram = i,
-                        nimpl = impl_param_def.bounds.trait_bounds.len(),
-                        ntrait = trait_param_def.bounds.trait_bounds.len()));
-            return;
+                        nbounds = unspecified_bounds.len(),
+                        bounds = unspecified_bounds.connect(", ")));
         }
     }
 

src/librustc/middle/typeck/collect.rs

@@ -33,6 +33,7 @@ are represented as `ty_param()` instances.
 
 use metadata::csearch;
 use middle::resolve_lifetime;
+use middle::lint::DuplicatedTypeBound;
 use middle::ty::{ImplContainer, MethodContainer, TraitContainer, substs};
 use middle::ty::{ty_param_bounds_and_ty};
 use middle::ty;
@@ -45,6 +46,8 @@ use middle::typeck::{CrateCtxt, lookup_def_tcx, no_params, write_ty_to_tcx};
 use util::ppaux;
 use util::ppaux::Repr;
 
+use collections::HashSet;
+
 use std::rc::Rc;
 use std::vec_ng::Vec;
 use std::vec_ng;
@@ -1029,11 +1032,23 @@ pub fn ty_generics(ccx: &CrateCtxt,
             builtin_bounds: ty::EmptyBuiltinBounds(),
             trait_bounds: Vec::new()
         };
+
+        let mut added_bounds = HashSet::new();
         for ast_bound in ast_bounds.iter() {
             match *ast_bound {
                 TraitTyParamBound(ref b) => {
                     let ty = ty::mk_param(ccx.tcx, param_ty.idx, param_ty.def_id);
                     let trait_ref = instantiate_trait_ref(ccx, b, ty);
+                    if !added_bounds.insert(trait_ref.def_id) {
+                        ccx.tcx.sess.add_lint(
+                            DuplicatedTypeBound,
+                            0,
+                            b.path.span,
+                            format!("duplicated bound `{}`, ignoring it",
+                                    trait_ref.repr(ccx.tcx)));
+                        continue;
+                    }
+
                     if !ty::try_add_builtin_trait(
                         ccx.tcx, trait_ref.def_id,
                         &mut param_bounds.builtin_bounds)

src/test/compile-fail/typeck-duplicated-trait-bounds.rs

@@ -0,0 +1,36 @@
+// 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 contravariant type parameters in implementations of traits, see #2687
+
+#[deny(duplicated_type_bound)];
+
+trait A {}
+
+trait Foo {
+    fn test_duplicated_builtin_bounds_fn<T: Eq+Ord+Eq>(&self);
+    //~^ ERROR duplicated bound `std::cmp::Eq`, ignoring it
+
+    fn test_duplicated_user_bounds_fn<T: A+A>(&self);
+    //~^ ERROR duplicated bound `A`, ignoring it
+}
+
+impl Foo for int {
+    fn test_duplicated_builtin_bounds_fn<T: Eq+Ord+Eq+Eq>(&self) {}
+    //~^ ERROR duplicated bound `std::cmp::Eq`, ignoring it
+    //~^^ ERROR duplicated bound `std::cmp::Eq`, ignoring it
+
+    fn test_duplicated_user_bounds_fn<T: A+A+A+A>(&self) {}
+    //~^ ERROR duplicated bound `A`, ignoring it
+    //~^^ ERROR duplicated bound `A`, ignoring it
+    //~^^^ ERROR duplicated bound `A`, ignoring it
+}
+
+fn main() {}

src/test/compile-fail/typeck-trait-bounds-impl-comparison.rs

@@ -0,0 +1,81 @@
+// 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 test_error6_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 bound `std::cmp::Eq` not enforced by this implementation
+    //~^^ ERROR implementation bound `std::cmp::Ord` was not specified in trait definition
+
+    // invalid bound for T, was defined as Eq + Ord in trait
+    fn test_error2_fn<T: Eq + B>(&self) {}
+    //~^ ERROR bound `std::cmp::Ord` not enforced by this implementation
+    //~^^ ERROR implementation bound `B` was not specified in trait definition
+
+    // invalid bound for T, was defined as Eq + Ord in trait
+    fn test_error3_fn<T: B + Eq>(&self) {}
+    //~^ ERROR bound `std::cmp::Ord` not enforced by this implementation
+    //~^^ ERROR implementation bound `B` was not specified in trait definition
+
+    // 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 bound `A` not enforced by this implementation
+    //~^^ ERROR implementation bound `B` was not specified in trait definition
+
+    fn test_error6_fn<T: A>(&self) {}
+    //~^ ERROR bound `std::cmp::Eq` not enforced by this implementation
+
+    fn test_error7_fn<T: A + Eq>(&self) {}
+    //~^ ERROR implementation bound `std::cmp::Eq` was not specified in trait definition
+
+    fn test_error8_fn<T: C>(&self) {}
+    //~^ ERROR implementation bound `C` was not specified in trait definition
+    //~^^ ERROR bound `B` not enforced by this implementation
+}
+
+
+trait Getter<T> { }
+
+trait Trait {
+    fn method<G:Getter<int>>();
+}
+
+impl Trait for uint {
+    fn method<G: Getter<uint>>() {}
+    //~^ ERROR requires Getter<uint> but Trait provides Getter<int>
+}
+fn main() {}

src/test/compile-fail/typeck-trait-bounds-impl-comparison2.rs

@@ -0,0 +1,23 @@
+// 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 Getter<T> { }
+
+trait Trait {
+    fn method<G:Getter<int>>();
+}
+
+impl Trait for uint {
+    fn method<G: Getter<uint>>() {}
+    //~^ ERROR requires Getter<uint> but Trait provides Getter<int>
+}
+fn main() {}

src/test/run-pass/typeck-contravariant-trait-bounds-impl.rs

@@ -0,0 +1,63 @@
+// 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 contravariant type parameters in implementations of traits, see #2687
+
+trait A {
+    fn a(&self) -> int;
+}
+
+impl A for u8 {
+    fn a(&self) -> int {
+        8
+    }
+}
+
+impl A for u32 {
+    fn a(&self) -> int {
+        32
+    }
+}
+
+trait B: A {
+    fn b(&self) -> int;
+}
+
+impl B for u32 {
+    fn b(&self) -> int {
+        -1
+    }
+}
+
+trait Foo {
+    fn test_fn<T: B>(&self, x: T) -> int;
+    fn test_duplicated_bounds1_fn<T: B+B>(&self) -> int;
+    fn test_duplicated_bounds2_fn<T: B>(&self) -> int;
+}
+
+impl Foo for int {
+    fn test_fn<T: A>(&self, x: T) -> int {
+        x.a()
+    }
+
+    fn test_duplicated_bounds1_fn<T: B+B>(&self) -> int {
+        99
+    }
+
+    fn test_duplicated_bounds2_fn<T: B+B>(&self) -> int {
+        199
+    }
+}
+
+fn main() {
+    let x: int = 0;
+    assert!(8 == x.test_fn(5u8));
+    assert!(32 == x.test_fn(5u32));
+}