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

Author: bors

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(

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);
     }
 }

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 `<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 }

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 <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() { }

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 <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>();
+}