Skip to content

Commit 3c07835

Browse files
borsbors
committed
Auto merge of #24056 - nikomatsakis:issue-23853-crates-io, r=aturon
If we find a blanket impl for `Trait` but we're matching on an object `Trait`, prefer the object (I think we could perhaps go either way, but this seems safer). Also give a nice error for attempts to manually `impl Trait for Trait`, since they will be ineffectual. This fixes the problems around ambiguity ICEs relating to `Any` and `MarkerTrait` that were cropping up all over the place. There may still be similar ICEs reported in #21756 that this PR does not address. Fixes #24015. Fixes #24051. Fixes #24037. Fixes #23853. Fixes #21942. cc #21756. cc @alexcrichton (this fixes crates.io) r? @aturon
2 parents 9f37ba6 + 0d56699 commit 3c07835

File tree

5 files changed

+112
-5
lines changed

5 files changed

+112
-5
lines changed

‎src/librustc/middle/traits/select.rs

+12
Original file line numberDiff line numberDiff line change
@@ -1378,6 +1378,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
13781378
// #18453.
13791379
true
13801380
}
1381+
(&ImplCandidate(..), &ObjectCandidate(..)) => {
1382+
// This means that we are matching an object of type
1383+
// `Trait` against the trait `Trait`. In that case, we
1384+
// always prefer to use the object vtable over the
1385+
// impl. Like a where clause, the impl may or may not
1386+
// be the one that is used by the object (because the
1387+
// impl may have additional where-clauses that the
1388+
// object's source might not meet) -- if it is, using
1389+
// the vtable is fine. If it is not, using the vtable
1390+
// is good. A win win!
1391+
true
1392+
}
13811393
(&DefaultImplCandidate(_), _) => {
13821394
// Prefer other candidates over default implementations.
13831395
self.tcx().sess.bug(

‎src/librustc_typeck/coherence/overlap.rs

+38-4
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ use syntax::ast_util;
2121
use syntax::visit;
2222
use syntax::codemap::Span;
2323
use util::nodemap::DefIdMap;
24-
use util::ppaux::Repr;
24+
use util::ppaux::{Repr, UserString};
2525

2626
pub fn check(tcx: &ty::ctxt) {
2727
let mut overlap = OverlapChecker { tcx: tcx, default_impls: DefIdMap() };
2828
overlap.check_for_overlapping_impls();
2929

30-
// this secondary walk specifically checks for impls of defaulted
31-
// traits, for which additional overlap rules exist
30+
// this secondary walk specifically checks for some other cases,
31+
// like defaulted traits, for which additional overlap rules exist
3232
visit::walk_crate(&mut overlap, tcx.map.krate());
3333
}
3434

@@ -153,7 +153,41 @@ impl<'cx, 'tcx,'v> visit::Visitor<'v> for OverlapChecker<'cx, 'tcx> {
153153
None => { }
154154
}
155155
}
156-
_ => {}
156+
ast::ItemImpl(_, _, _, Some(_), ref self_ty, _) => {
157+
let impl_def_id = ast_util::local_def(item.id);
158+
let trait_ref = ty::impl_trait_ref(self.tcx, impl_def_id).unwrap();
159+
let trait_def_id = trait_ref.def_id;
160+
match trait_ref.self_ty().sty {
161+
ty::ty_trait(ref data) => {
162+
// This is something like impl Trait1 for Trait2. Illegal
163+
// if Trait1 is a supertrait of Trait2 or Trait2 is not object safe.
164+
165+
if !traits::is_object_safe(self.tcx, data.principal_def_id()) {
166+
// this just means the self-ty is illegal,
167+
// and probably this error should have
168+
// been reported elsewhere, but I'm trying to avoid
169+
// giving a misleading message below.
170+
span_err!(self.tcx.sess, self_ty.span, E0372,
171+
"the trait `{}` cannot be made into an object",
172+
ty::item_path_str(self.tcx, data.principal_def_id()));
173+
} else {
174+
let mut supertrait_def_ids =
175+
traits::supertrait_def_ids(self.tcx, data.principal_def_id());
176+
if supertrait_def_ids.any(|d| d == trait_def_id) {
177+
span_err!(self.tcx.sess, item.span, E0371,
178+
"the object type `{}` automatically \
179+
implements the trait `{}`",
180+
trait_ref.self_ty().user_string(self.tcx),
181+
ty::item_path_str(self.tcx, trait_def_id));
182+
}
183+
}
184+
}
185+
_ => { }
186+
}
187+
}
188+
_ => {
189+
}
157190
}
191+
visit::walk_item(self, item);
158192
}
159193
}

‎src/librustc_typeck/diagnostics.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,9 @@ register_diagnostics! {
179179
E0366, // dropck forbid specialization to concrete type or region
180180
E0367, // dropck forbid specialization to predicate not in struct/enum
181181
E0368, // binary operation `<op>=` cannot be applied to types
182-
E0369 // binary operation `<op>` cannot be applied to types
182+
E0369, // binary operation `<op>` cannot be applied to types
183+
E0371, // impl Trait for Trait is illegal
184+
E0372 // impl Trait for Trait where Trait is not object safe
183185
}
184186

185187
__build_diagnostic_array! { DIAGNOSTICS }

‎src/test/compile-fail/coherence-impl-trait-for-trait.rs

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Test that we give suitable error messages when the user attempts to
12+
// impl a trait `Trait` for its own object type.
13+
14+
trait Foo { fn dummy(&self) { } }
15+
trait Bar: Foo { }
16+
trait Baz: Bar { }
17+
18+
// Subtraits of Baz are not legal:
19+
impl Foo for Baz { } //~ ERROR E0371
20+
impl Bar for Baz { } //~ ERROR E0371
21+
impl Baz for Baz { } //~ ERROR E0371
22+
23+
// But other random traits are:
24+
trait Other { }
25+
impl Other for Baz { } // OK, Bar not a subtrait of Baz
26+
27+
// If the trait is not object-safe, we give a more tailored message
28+
// because we're such schnuckels:
29+
trait NotObjectSafe { fn eq(&self, other: Self); }
30+
impl NotObjectSafe for NotObjectSafe { } //~ ERROR E0372
31+
32+
fn main() { }

‎src/test/run-pass/traits-impl-object-overlap-issue-23853.rs

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Test that we are able to compile the case where both a blanket impl
12+
// and the object type itself supply the required trait obligation.
13+
// In this case, the blanket impl for `Foo` applies to any type,
14+
// including `Bar`, but the object type `Bar` also implicitly supplies
15+
// this context.
16+
17+
trait Foo { fn dummy(&self) { } }
18+
19+
trait Bar: Foo { }
20+
21+
impl<T:?Sized> Foo for T { }
22+
23+
fn want_foo<B:?Sized+Foo>() { }
24+
25+
fn main() {
26+
want_foo::<Bar>();
27+
}

0 commit comments

Comments
 (0)