From d9346d8eae286b4710c4b0eddfa6f0026c57cade Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Thu, 13 Feb 2014 22:29:57 +0100 Subject: [PATCH 1/3] Add check to compare an impl method type with trait bounds closes #2687 --- src/librustc/middle/typeck/check/mod.rs | 72 ++++++++++++++----- src/librustc/middle/typeck/collect.rs | 11 +++ .../typeck-trait-bounds-impl-comparison.rs | 71 ++++++++++++++++++ .../typeck-contravariant-trait-bounds-impl.rs | 66 +++++++++++++++++ 4 files changed, 203 insertions(+), 17 deletions(-) create mode 100644 src/test/compile-fail/typeck-trait-bounds-impl-comparison.rs create mode 100644 src/test/run-pass/typeck-contravariant-trait-bounds-impl.rs diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index 51efcb7d1c387..fd5416ce6a5c4 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/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; +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,62 @@ 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) && + impl_bound.def_id == trait_bound.def_id { + enforced_bounds.insert(trait_bound.def_id); + specified = true; + break + } else { + for s_trait_bound in ty::trait_supertraits(tcx, trait_bound.def_id).iter() { + if !enforced_bounds.contains(&trait_bound.def_id) && + impl_bound.def_id == s_trait_bound.def_id { + enforced_bounds.insert(trait_bound.def_id); + specified = true; + break + } + } + } + } + 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(", "))); } } diff --git a/src/librustc/middle/typeck/collect.rs b/src/librustc/middle/typeck/collect.rs index 0c6aeda425830..6629e7bce8146 100644 --- a/src/librustc/middle/typeck/collect.rs +++ b/src/librustc/middle/typeck/collect.rs @@ -1034,6 +1034,17 @@ pub fn ty_generics(ccx: &CrateCtxt, 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.contains_key(&trait_ref.def_id) { + ccx.tcx.sess.span_warn( + b.path.span, + format!("duplicated bound `{}`, ignoring it", + trait_ref.repr(ccx.tcx))); + continue; + + } else { + added_bounds.insert(trait_ref.def_id, ()); + } + if !ty::try_add_builtin_trait( ccx.tcx, trait_ref.def_id, &mut param_bounds.builtin_bounds) diff --git a/src/test/compile-fail/typeck-trait-bounds-impl-comparison.rs b/src/test/compile-fail/typeck-trait-bounds-impl-comparison.rs new file mode 100644 index 0000000000000..0bc5b9b7d5177 --- /dev/null +++ b/src/test/compile-fail/typeck-trait-bounds-impl-comparison.rs @@ -0,0 +1,71 @@ +// 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 test_error6_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 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(&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(&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(&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 bound `A` not enforced by this implementation + //~^^ ERROR implementation bound `B` was not specified in trait definition + + fn test_error6_fn(&self) {} + //~^ ERROR bound `std::cmp::Eq` not enforced by this implementation + + fn test_error7_fn(&self) {} + //~^ ERROR implementation bound `std::cmp::Eq` was not specified in trait definition + + fn test_error8_fn(&self) {} + //~^ ERROR implementation bound `C` was not specified in trait definition + //~^^ ERROR bound `B` not enforced by this implementation + +} + +fn main() {} diff --git a/src/test/run-pass/typeck-contravariant-trait-bounds-impl.rs b/src/test/run-pass/typeck-contravariant-trait-bounds-impl.rs new file mode 100644 index 0000000000000..33fa7f3011e9f --- /dev/null +++ b/src/test/run-pass/typeck-contravariant-trait-bounds-impl.rs @@ -0,0 +1,66 @@ +// 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 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(&self, x: T) -> int; + fn test_duplicated_bounds1_fn(&self) -> int; + //~^ warn: duplicated bound `B`, ignoring it + fn test_duplicated_bounds2_fn(&self) -> int; +} + +impl Foo for int { + fn test_fn(&self, x: T) -> int { + x.a() + } + + fn test_duplicated_bounds1_fn(&self) -> int { + //~^ warn: duplicated bound `B`, ignoring it + 99 + } + + fn test_duplicated_bounds2_fn(&self) -> int { + //~^ warn: duplicated bound `B`, ignoring it + 199 + } +} + +fn main() { + let x: int = 0; + assert!(8 == x.test_fn(5u8)); + assert!(32 == x.test_fn(5u32)); +} From e7d98d5fd74c44a94775f2071db3d10b8456db41 Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Tue, 4 Mar 2014 23:16:39 +0100 Subject: [PATCH 2/3] Check duplicated paramter bounds, refs #2687 --- src/librustc/middle/lint.rs | 8 +++++ src/librustc/middle/typeck/collect.rs | 14 +++++--- .../typeck-duplicated-trait-bounds.rs | 36 +++++++++++++++++++ .../typeck-contravariant-trait-bounds-impl.rs | 3 -- 4 files changed, 53 insertions(+), 8 deletions(-) create mode 100644 src/test/compile-fail/typeck-duplicated-trait-bounds.rs diff --git a/src/librustc/middle/lint.rs b/src/librustc/middle/lint.rs index ae2600533559b..d344362dd41cb 100644 --- a/src/librustc/middle/lint.rs +++ b/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, diff --git a/src/librustc/middle/typeck/collect.rs b/src/librustc/middle/typeck/collect.rs index 6629e7bce8146..21f238f3a9e33 100644 --- a/src/librustc/middle/typeck/collect.rs +++ b/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,20 +1032,21 @@ 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.contains_key(&trait_ref.def_id) { - ccx.tcx.sess.span_warn( + 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; - - } else { - added_bounds.insert(trait_ref.def_id, ()); } if !ty::try_add_builtin_trait( diff --git a/src/test/compile-fail/typeck-duplicated-trait-bounds.rs b/src/test/compile-fail/typeck-duplicated-trait-bounds.rs new file mode 100644 index 0000000000000..028ba6ef1a27c --- /dev/null +++ b/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 or the MIT license +// , 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(&self); + //~^ ERROR duplicated bound `std::cmp::Eq`, ignoring it + + fn test_duplicated_user_bounds_fn(&self); + //~^ ERROR duplicated bound `A`, ignoring it +} + +impl Foo for int { + fn test_duplicated_builtin_bounds_fn(&self) {} + //~^ ERROR duplicated bound `std::cmp::Eq`, ignoring it + //~^^ ERROR duplicated bound `std::cmp::Eq`, ignoring it + + fn test_duplicated_user_bounds_fn(&self) {} + //~^ ERROR duplicated bound `A`, ignoring it + //~^^ ERROR duplicated bound `A`, ignoring it + //~^^^ ERROR duplicated bound `A`, ignoring it +} + +fn main() {} diff --git a/src/test/run-pass/typeck-contravariant-trait-bounds-impl.rs b/src/test/run-pass/typeck-contravariant-trait-bounds-impl.rs index 33fa7f3011e9f..bcb804f718456 100644 --- a/src/test/run-pass/typeck-contravariant-trait-bounds-impl.rs +++ b/src/test/run-pass/typeck-contravariant-trait-bounds-impl.rs @@ -39,7 +39,6 @@ impl B for u32 { trait Foo { fn test_fn(&self, x: T) -> int; fn test_duplicated_bounds1_fn(&self) -> int; - //~^ warn: duplicated bound `B`, ignoring it fn test_duplicated_bounds2_fn(&self) -> int; } @@ -49,12 +48,10 @@ impl Foo for int { } fn test_duplicated_bounds1_fn(&self) -> int { - //~^ warn: duplicated bound `B`, ignoring it 99 } fn test_duplicated_bounds2_fn(&self) -> int { - //~^ warn: duplicated bound `B`, ignoring it 199 } } From a1bedb7521361c578fa5dc71eecbb566e7094e35 Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Fri, 14 Mar 2014 22:47:57 +0100 Subject: [PATCH 3/3] Use mk_sub_trait_refs --- src/librustc/middle/typeck/check/mod.rs | 22 ++++++++---------- .../typeck-trait-bounds-impl-comparison.rs | 10 ++++++++ .../typeck-trait-bounds-impl-comparison2.rs | 23 +++++++++++++++++++ 3 files changed, 43 insertions(+), 12 deletions(-) create mode 100644 src/test/compile-fail/typeck-trait-bounds-impl-comparison2.rs diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index fd5416ce6a5c4..55033ae6a4080 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/src/librustc/middle/typeck/check/mod.rs @@ -112,7 +112,7 @@ use middle::lang_items::TypeIdLangItem; use util::common::{block_query, indenter, loop_query}; use util::ppaux; use util::ppaux::{UserString, Repr}; -use util::nodemap::NodeMap; +use util::nodemap::{NodeMap, FnvHashMap}; use collections::{HashMap, HashSet}; use std::cell::{Cell, RefCell}; @@ -871,19 +871,17 @@ fn compare_impl_method(tcx: ty::ctxt, 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) && - impl_bound.def_id == trait_bound.def_id { - enforced_bounds.insert(trait_bound.def_id); - specified = true; - break - } else { - for s_trait_bound in ty::trait_supertraits(tcx, trait_bound.def_id).iter() { - if !enforced_bounds.contains(&trait_bound.def_id) && - impl_bound.def_id == s_trait_bound.def_id { + 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; - break - } + } // Ok. + result::Err(_) => {} } } } diff --git a/src/test/compile-fail/typeck-trait-bounds-impl-comparison.rs b/src/test/compile-fail/typeck-trait-bounds-impl-comparison.rs index 0bc5b9b7d5177..34394b4c8b981 100644 --- a/src/test/compile-fail/typeck-trait-bounds-impl-comparison.rs +++ b/src/test/compile-fail/typeck-trait-bounds-impl-comparison.rs @@ -65,7 +65,17 @@ impl Foo for int { fn test_error8_fn(&self) {} //~^ ERROR implementation bound `C` was not specified in trait definition //~^^ ERROR bound `B` not enforced by this implementation +} + +trait Getter { } + +trait Trait { + fn method>(); } +impl Trait for uint { + fn method>() {} + //~^ ERROR requires Getter but Trait provides Getter +} fn main() {} diff --git a/src/test/compile-fail/typeck-trait-bounds-impl-comparison2.rs b/src/test/compile-fail/typeck-trait-bounds-impl-comparison2.rs new file mode 100644 index 0000000000000..64b97438666e3 --- /dev/null +++ b/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 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 Getter { } + +trait Trait { + fn method>(); +} + +impl Trait for uint { + fn method>() {} + //~^ ERROR requires Getter but Trait provides Getter +} +fn main() {}