From 0d56699d41c5683935ec4c182baa867756ae4e79 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 4 Apr 2015 05:42:24 -0400 Subject: [PATCH] If we find a blanket impl for `Trait` but we're matching on an object `Trait`, prefer the object. Also give a nice error for attempts to manually `impl Trait for Trait`, since they will be ineffectual. Fixes #24015. Fixes #24051. Fixes #24037. Fixes #23853. Fixes #21942. cc #21756. --- src/librustc/middle/traits/select.rs | 12 ++++++ src/librustc_typeck/coherence/overlap.rs | 42 +++++++++++++++++-- src/librustc_typeck/diagnostics.rs | 4 +- .../coherence-impl-trait-for-trait.rs | 32 ++++++++++++++ .../traits-impl-object-overlap-issue-23853.rs | 27 ++++++++++++ 5 files changed, 112 insertions(+), 5 deletions(-) create mode 100644 src/test/compile-fail/coherence-impl-trait-for-trait.rs create mode 100644 src/test/run-pass/traits-impl-object-overlap-issue-23853.rs diff --git a/src/librustc/middle/traits/select.rs b/src/librustc/middle/traits/select.rs index ad7d96c652d9b..08357084bb20b 100644 --- a/src/librustc/middle/traits/select.rs +++ b/src/librustc/middle/traits/select.rs @@ -1378,6 +1378,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // #18453. true } + (&ImplCandidate(..), &ObjectCandidate(..)) => { + // This means that we are matching an object of type + // `Trait` against the trait `Trait`. In that case, we + // always prefer to use the object vtable over the + // impl. Like a where clause, the impl may or may not + // be the one that is used by the object (because the + // impl may have additional where-clauses that the + // object's source might not meet) -- if it is, using + // the vtable is fine. If it is not, using the vtable + // is good. A win win! + true + } (&DefaultImplCandidate(_), _) => { // Prefer other candidates over default implementations. self.tcx().sess.bug( diff --git a/src/librustc_typeck/coherence/overlap.rs b/src/librustc_typeck/coherence/overlap.rs index f8ca51b9e496a..99b6d4154e91f 100644 --- a/src/librustc_typeck/coherence/overlap.rs +++ b/src/librustc_typeck/coherence/overlap.rs @@ -21,14 +21,14 @@ use syntax::ast_util; use syntax::visit; use syntax::codemap::Span; use util::nodemap::DefIdMap; -use util::ppaux::Repr; +use util::ppaux::{Repr, UserString}; pub fn check(tcx: &ty::ctxt) { let mut overlap = OverlapChecker { tcx: tcx, default_impls: DefIdMap() }; overlap.check_for_overlapping_impls(); - // this secondary walk specifically checks for impls of defaulted - // traits, for which additional overlap rules exist + // this secondary walk specifically checks for some other cases, + // like defaulted traits, for which additional overlap rules exist visit::walk_crate(&mut overlap, tcx.map.krate()); } @@ -153,7 +153,41 @@ impl<'cx, 'tcx,'v> visit::Visitor<'v> for OverlapChecker<'cx, 'tcx> { None => { } } } - _ => {} + ast::ItemImpl(_, _, _, Some(_), ref self_ty, _) => { + let impl_def_id = ast_util::local_def(item.id); + let trait_ref = ty::impl_trait_ref(self.tcx, impl_def_id).unwrap(); + let trait_def_id = trait_ref.def_id; + match trait_ref.self_ty().sty { + ty::ty_trait(ref data) => { + // This is something like impl Trait1 for Trait2. Illegal + // if Trait1 is a supertrait of Trait2 or Trait2 is not object safe. + + if !traits::is_object_safe(self.tcx, data.principal_def_id()) { + // this just means the self-ty is illegal, + // and probably this error should have + // been reported elsewhere, but I'm trying to avoid + // giving a misleading message below. + span_err!(self.tcx.sess, self_ty.span, E0372, + "the trait `{}` cannot be made into an object", + ty::item_path_str(self.tcx, data.principal_def_id())); + } else { + let mut supertrait_def_ids = + traits::supertrait_def_ids(self.tcx, data.principal_def_id()); + if supertrait_def_ids.any(|d| d == trait_def_id) { + span_err!(self.tcx.sess, item.span, E0371, + "the object type `{}` automatically \ + implements the trait `{}`", + trait_ref.self_ty().user_string(self.tcx), + ty::item_path_str(self.tcx, trait_def_id)); + } + } + } + _ => { } + } + } + _ => { + } } + visit::walk_item(self, item); } } diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index a8d93c8bd111a..b17702cfb8cb5 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -179,7 +179,9 @@ register_diagnostics! { E0366, // dropck forbid specialization to concrete type or region E0367, // dropck forbid specialization to predicate not in struct/enum E0368, // binary operation `=` cannot be applied to types - E0369 // binary operation `` cannot be applied to types + E0369, // binary operation `` cannot be applied to types + E0371, // impl Trait for Trait is illegal + E0372 // impl Trait for Trait where Trait is not object safe } __build_diagnostic_array! { DIAGNOSTICS } diff --git a/src/test/compile-fail/coherence-impl-trait-for-trait.rs b/src/test/compile-fail/coherence-impl-trait-for-trait.rs new file mode 100644 index 0000000000000..23f4218fc1ebd --- /dev/null +++ b/src/test/compile-fail/coherence-impl-trait-for-trait.rs @@ -0,0 +1,32 @@ +// Copyright 2015 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. + +// Test that we give suitable error messages when the user attempts to +// impl a trait `Trait` for its own object type. + +trait Foo { fn dummy(&self) { } } +trait Bar: Foo { } +trait Baz: Bar { } + +// Subtraits of Baz are not legal: +impl Foo for Baz { } //~ ERROR E0371 +impl Bar for Baz { } //~ ERROR E0371 +impl Baz for Baz { } //~ ERROR E0371 + +// But other random traits are: +trait Other { } +impl Other for Baz { } // OK, Bar not a subtrait of Baz + +// If the trait is not object-safe, we give a more tailored message +// because we're such schnuckels: +trait NotObjectSafe { fn eq(&self, other: Self); } +impl NotObjectSafe for NotObjectSafe { } //~ ERROR E0372 + +fn main() { } diff --git a/src/test/run-pass/traits-impl-object-overlap-issue-23853.rs b/src/test/run-pass/traits-impl-object-overlap-issue-23853.rs new file mode 100644 index 0000000000000..b9b2b5061375a --- /dev/null +++ b/src/test/run-pass/traits-impl-object-overlap-issue-23853.rs @@ -0,0 +1,27 @@ +// Copyright 2015 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. + +// Test that we are able to compile the case where both a blanket impl +// and the object type itself supply the required trait obligation. +// In this case, the blanket impl for `Foo` applies to any type, +// including `Bar`, but the object type `Bar` also implicitly supplies +// this context. + +trait Foo { fn dummy(&self) { } } + +trait Bar: Foo { } + +impl Foo for T { } + +fn want_foo() { } + +fn main() { + want_foo::(); +}