Auto merge of #50966 - leodasvacas:self-in-where-clauses-is-not-object-safe, r=nikomatsakis

`Self` in where clauses may not be object safe

Needs crater, virtually certain to cause regressions.

In #50781 it was discovered that our object safety rules are not sound because we allow `Self` in where clauses without restrain. This PR is a direct fix to the rules so that we disallow methods with unsound where clauses.

This currently uses hard error to measure impact, but we will want to downgrade it to a future compat error.

Part of #50781.

r? @nikomatsakis

src/librustc/lint/builtin.rs

@@ -304,6 +304,12 @@ declare_lint! {
     "warn about documentation intra links resolution failure"
 }
 
+declare_lint! {
+    pub WHERE_CLAUSES_OBJECT_SAFETY,
+    Warn,
+    "checks the object safety of where clauses"
+}
+
 /// Does nothing as a lint pass, but registers some `Lint`s
 /// which are used by other parts of the compiler.
 #[derive(Copy, Clone)]
@@ -358,6 +364,7 @@ impl LintPass for HardwiredLints {
             DUPLICATE_ASSOCIATED_TYPE_BINDINGS,
             DUPLICATE_MACRO_EXPORTS,
             INTRA_DOC_LINK_RESOLUTION_FAILURE,
+            WHERE_CLAUSES_OBJECT_SAFETY,
         )
     }
 }

src/librustc/traits/object_safety.rs

@@ -20,14 +20,16 @@
 use super::elaborate_predicates;
 
 use hir::def_id::DefId;
+use lint;
 use traits;
 use ty::{self, Ty, TyCtxt, TypeFoldable};
 use ty::subst::Substs;
 use ty::util::ExplicitSelf;
 use std::borrow::Cow;
 use syntax::ast;
+use syntax_pos::Span;
 
-#[derive(Clone, Debug, PartialEq, Eq, Hash)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
 pub enum ObjectSafetyViolation {
     /// Self : Sized declared on the trait
     SizedSelf,
@@ -56,6 +58,9 @@ impl ObjectSafetyViolation {
             ObjectSafetyViolation::Method(name, MethodViolationCode::ReferencesSelf) =>
                 format!("method `{}` references the `Self` type \
                          in its arguments or return type", name).into(),
+            ObjectSafetyViolation::Method(name,
+                                            MethodViolationCode::WhereClauseReferencesSelf(_)) =>
+                format!("method `{}` references the `Self` type in where clauses", name).into(),
             ObjectSafetyViolation::Method(name, MethodViolationCode::Generic) =>
                 format!("method `{}` has generic type parameters", name).into(),
             ObjectSafetyViolation::Method(name, MethodViolationCode::NonStandardSelfType) =>
@@ -75,6 +80,9 @@ pub enum MethodViolationCode {
     /// e.g., `fn foo(&self, x: Self)` or `fn foo(&self) -> Self`
     ReferencesSelf,
 
+    /// e.g. `fn foo(&self) where Self: Clone`
+    WhereClauseReferencesSelf(Span),
+
     /// e.g., `fn foo<A>()`
     Generic,
 
@@ -123,6 +131,22 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
             .filter_map(|item| {
                 self.object_safety_violation_for_method(trait_def_id, &item)
                     .map(|code| ObjectSafetyViolation::Method(item.name, code))
+            }).filter(|violation| {
+                if let ObjectSafetyViolation::Method(_,
+                                MethodViolationCode::WhereClauseReferencesSelf(span)) = violation {
+                    // Using`CRATE_NODE_ID` is wrong, but it's hard to get a more precise id.
+                    // It's also hard to get a use site span, so we use the method definition span.
+                    self.lint_node_note(
+                        lint::builtin::WHERE_CLAUSES_OBJECT_SAFETY,
+                        ast::CRATE_NODE_ID,
+                        *span,
+                        &format!("the trait `{}` cannot be made into an object",
+                                self.item_path_str(trait_def_id)),
+                        &violation.error_msg());
+                    false
+                } else {
+                    true
+                }
             }).collect();
 
         // Check the trait itself.
@@ -245,7 +269,10 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
             return false;
         }
 
-        self.virtual_call_violation_for_method(trait_def_id, method).is_none()
+        match self.virtual_call_violation_for_method(trait_def_id, method) {
+            None | Some(MethodViolationCode::WhereClauseReferencesSelf(_)) => true,
+            Some(_) => false,
+        }
     }
 
     /// Returns `Some(_)` if this method cannot be called on a trait
@@ -288,6 +315,18 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
             return Some(MethodViolationCode::Generic);
         }
 
+        if self.predicates_of(method.def_id).predicates.into_iter()
+                // A trait object can't claim to live more than the concrete type,
+                // so outlives predicates will always hold.
+                .filter(|p| p.to_opt_type_outlives().is_none())
+                .collect::<Vec<_>>()
+                // Do a shallow visit so that `contains_illegal_self_type_reference`
+                // may apply it's custom visiting.
+                .visit_tys_shallow(|t| self.contains_illegal_self_type_reference(trait_def_id, t)) {
+            let span = self.def_span(method.def_id);
+            return Some(MethodViolationCode::WhereClauseReferencesSelf(span));
+        }
+
         None
     }
 

src/librustc/ty/fold.rs

@@ -136,6 +136,20 @@ pub trait TypeFoldable<'tcx>: fmt::Debug + Clone {
     fn has_late_bound_regions(&self) -> bool {
         self.has_type_flags(TypeFlags::HAS_RE_LATE_BOUND)
     }
+
+    /// A visitor that does not recurse into types, works like `fn walk_shallow` in `Ty`.
+    fn visit_tys_shallow(&self, visit: impl FnMut(Ty<'tcx>) -> bool) -> bool {
+
+        pub struct Visitor<F>(F);
+
+        impl<'tcx, F: FnMut(Ty<'tcx>) -> bool> TypeVisitor<'tcx> for Visitor<F> {
+            fn visit_ty(&mut self, ty: Ty<'tcx>) -> bool {
+                self.0(ty)
+            }
+        }
+
+        self.visit_with(&mut Visitor(visit))
+    }
 }
 
 /// The TypeFolder trait defines the actual *folding*. There is a

src/librustc_lint/lib.rs

@@ -292,6 +292,11 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
             reference: "issue TBD",
             edition: Some(Edition::Edition2018),
         },
+        FutureIncompatibleInfo {
+            id: LintId::of(WHERE_CLAUSES_OBJECT_SAFETY),
+            reference: "issue #51443 <https://github.com/rust-lang/rust/issues/51443>",
+            edition: None,
+        },
         FutureIncompatibleInfo {
             id: LintId::of(DUPLICATE_ASSOCIATED_TYPE_BINDINGS),
             reference: "issue #50589 <https://github.com/rust-lang/rust/issues/50589>",

src/test/compile-fail/issue-43431.rs

@@ -11,7 +11,7 @@
 #![feature(fn_traits)]
 
 trait CallSingle<A, B> {
-    fn call(&self, a: A) -> B where Self: Fn(A) -> B;
+    fn call(&self, a: A) -> B where Self: Sized, Self: Fn(A) -> B;
 }
 
 impl<A, B, F: Fn(A) -> B> CallSingle<A, B> for F {

src/test/compile-fail/wf-trait-fn-where-clause.rs

@@ -17,7 +17,7 @@
 struct Bar<T:Eq+?Sized> { value: Box<T> }
 
 trait Foo {
-    fn bar(&self) where Bar<Self>: Copy;
+    fn bar(&self) where Self: Sized, Bar<Self>: Copy;
         //~^ ERROR E0277
         //
         // Here, Eq ought to be implemented.

src/test/run-pass/issue-23435.rs

@@ -1,37 +0,0 @@
-// 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 do not ICE when a default method implementation has
-// requirements (in this case, `Self : Baz`) that do not hold for some
-// specific impl (in this case, `Foo : Bar`). This causes problems
-// only when building a vtable, because that goes along and
-// instantiates all the methods, even those that could not otherwise
-// be called.
-
-// pretty-expanded FIXME #23616
-
-struct Foo {
-    x: i32
-}
-
-trait Bar {
-    fn bar(&self) where Self : Baz { self.baz(); }
-}
-
-trait Baz {
-    fn baz(&self);
-}
-
-impl Bar for Foo {
-}
-
-fn main() {
-    let x: &Bar = &Foo { x: 22 };
-}

src/test/ui/issue-50781.rs

@@ -0,0 +1,29 @@
+// Copyright 2018 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.
+
+#![deny(where_clauses_object_safety)]
+
+trait Trait {}
+
+trait X {
+    fn foo(&self) where Self: Trait; //~ ERROR the trait `X` cannot be made into an object
+    //~^ WARN this was previously accepted by the compiler but is being phased out
+}
+
+impl X for () {
+    fn foo(&self) {}
+}
+
+impl Trait for dyn X {}
+
+pub fn main() {
+    // Check that this does not segfault.
+    <X as X>::foo(&());
+}

src/test/ui/issue-50781.stderr

@@ -0,0 +1,17 @@
+error: the trait `X` cannot be made into an object
+  --> $DIR/issue-50781.rs:16:5
+   |
+LL |     fn foo(&self) where Self: Trait; //~ ERROR the trait `X` cannot be made into an object
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: lint level defined here
+  --> $DIR/issue-50781.rs:11:9
+   |
+LL | #![deny(where_clauses_object_safety)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #51443 <https://github.com/rust-lang/rust/issues/51443>
+   = note: method `foo` references the `Self` type in where clauses
+
+error: aborting due to previous error
+