Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefer ObjectCandidate to ImplCandidate if both apply #24056

Merged
merged 1 commit into from
Apr 6, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/librustc/middle/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
42 changes: 38 additions & 4 deletions src/librustc_typeck/coherence/overlap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand Down Expand Up @@ -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);
}
}
4 changes: 3 additions & 1 deletion src/librustc_typeck/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<op>=` cannot be applied to types
E0369 // binary operation `<op>` cannot be applied to types
E0369, // binary operation `<op>` 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 }
32 changes: 32 additions & 0 deletions src/test/compile-fail/coherence-impl-trait-for-trait.rs
Original file line number Diff line number Diff line change
@@ -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 <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.

// 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() { }
27 changes: 27 additions & 0 deletions src/test/run-pass/traits-impl-object-overlap-issue-23853.rs
Original file line number Diff line number Diff line change
@@ -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 <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.

// 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<T:?Sized> Foo for T { }

fn want_foo<B:?Sized+Foo>() { }

fn main() {
want_foo::<Bar>();
}