Auto merge of #28669 - arielb1:well-formed-methods, r=nikomatsakis

By RFC1214:
>    Before calling a fn, we check that its argument and return types are WF.
    
The previous code only checked the trait-ref, which was not enough
in several cases.
    
As this is a soundness fix, it is a [breaking-change]. Some new annotations are needed, which I think are because of #18653 and the imperfection of `projection_must_outlive` (that can probably be worked around by moving the wf obligation later).
    
Fixes #28609

r? @nikomatsakis 

10 files changed, 206 insertions, 10 deletions

diff --git a/src/librustc/middle/ty/structural_impls.rs b/src/librustc/middle/ty/structural_impls.rs
index 4ed87e673d9..176dc5a743d 100644
--- a/src/librustc/middle/ty/structural_impls.rs
+++ b/src/librustc/middle/ty/structural_impls.rs
@@ -417,7 +417,9 @@ impl<'tcx, A: Lift<'tcx>, B: Lift<'tcx>> Lift<'tcx> for (A, B) {
 impl<'tcx, T: Lift<'tcx>> Lift<'tcx> for [T] {
     type Lifted = Vec<T::Lifted>;
     fn lift_to_tcx(&self, tcx: &ty::ctxt<'tcx>) -> Option<Self::Lifted> {
-        let mut result = Vec::with_capacity(self.len());
+        // type annotation needed to inform `projection_must_outlive`
+        let mut result : Vec<<T as Lift<'tcx>>::Lifted>
+            = Vec::with_capacity(self.len());
         for x in self {
             if let Some(value) = tcx.lift(x) {
                 result.push(value);
diff --git a/src/librustc_typeck/check/method/confirm.rs b/src/librustc_typeck/check/method/confirm.rs
index b0e81803ba7..72131627aa5 100644
--- a/src/librustc_typeck/check/method/confirm.rs
+++ b/src/librustc_typeck/check/method/confirm.rs
@@ -103,22 +103,23 @@ impl<'a,'tcx> ConfirmContext<'a,'tcx> {
         // Unify the (adjusted) self type with what the method expects.
         self.unify_receivers(self_ty, method_self_ty);
 
-        // Add any trait/regions obligations specified on the method's type parameters.
-        self.add_obligations(&pick, &all_substs, &method_predicates);
-
-        // Create the final `MethodCallee`.
+        // Create the method type
         let method_ty = pick.item.as_opt_method().unwrap();
         let fty = self.tcx().mk_fn(None, self.tcx().mk_bare_fn(ty::BareFnTy {
             sig: ty::Binder(method_sig),
             unsafety: method_ty.fty.unsafety,
             abi: method_ty.fty.abi.clone(),
         }));
+
+        // Add any trait/regions obligations specified on the method's type parameters.
+        self.add_obligations(fty, &all_substs, &method_predicates);
+
+        // Create the final `MethodCallee`.
         let callee = ty::MethodCallee {
             def_id: pick.item.def_id(),
             ty: fty,
             substs: self.tcx().mk_substs(all_substs)
         };
-
         // If this is an `&mut self` method, bias the receiver
         // expression towards mutability (this will switch
         // e.g. `Deref` to `DerefMut` in overloaded derefs and so on).
@@ -422,11 +423,11 @@ impl<'a,'tcx> ConfirmContext<'a,'tcx> {
     }
 
     fn add_obligations(&mut self,
-                       pick: &probe::Pick<'tcx>,
+                       fty: Ty<'tcx>,
                        all_substs: &subst::Substs<'tcx>,
                        method_predicates: &ty::InstantiatedPredicates<'tcx>) {
-        debug!("add_obligations: pick={:?} all_substs={:?} method_predicates={:?}",
-               pick,
+        debug!("add_obligations: fty={:?} all_substs={:?} method_predicates={:?}",
+               fty,
                all_substs,
                method_predicates);
 
@@ -439,6 +440,11 @@ impl<'a,'tcx> ConfirmContext<'a,'tcx> {
         self.fcx.add_wf_bounds(
             all_substs,
             self.call_expr);
+
+        // the function type must also be well-formed (this is not
+        // implied by the substs being well-formed because of inherent
+        // impls and late-bound regions - see issue #28609).
+        self.fcx.register_wf_obligation(fty, self.span, traits::MiscObligation);
     }
 
     ///////////////////////////////////////////////////////////////////////////
diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs
index edf1cc9b7ef..e0ad51b4ea1 100644
--- a/src/librustc_typeck/check/method/mod.rs
+++ b/src/librustc_typeck/check/method/mod.rs
@@ -255,6 +255,9 @@ pub fn lookup_in_trait_adjusted<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
         traits::ObligationCause::misc(span, fcx.body_id),
         &method_bounds);
 
+    // Also register an obligation for the method type being well-formed.
+    fcx.register_wf_obligation(fty, span, traits::MiscObligation);
+
     // FIXME(#18653) -- Try to resolve obligations, giving us more
     // typing information, which can sometimes be needed to avoid
     // pathological region inference failures.
diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs
index 43c23ec8a47..134b1ce16e2 100644
--- a/src/libstd/thread/mod.rs
+++ b/src/libstd/thread/mod.rs
@@ -266,7 +266,8 @@ impl Builder {
         let my_thread = Thread::new(name);
         let their_thread = my_thread.clone();
 
-        let my_packet = Arc::new(UnsafeCell::new(None));
+        let my_packet : Arc<UnsafeCell<Option<Result<T>>>>
+            = Arc::new(UnsafeCell::new(None));
         let their_packet = my_packet.clone();
 
         let main = move || {
diff --git a/src/test/compile-fail/issue-18959.rs b/src/test/compile-fail/issue-18959.rs
index 95176da9020..5f6216a898a 100644
--- a/src/test/compile-fail/issue-18959.rs
+++ b/src/test/compile-fail/issue-18959.rs
@@ -22,6 +22,7 @@ fn foo(b: &Bar) {
     b.foo(&0)
     //~^ ERROR the trait `Foo` is not implemented for the type `Bar`
     //~| ERROR E0038
+    //~| WARNING E0038
 }
 
 fn main() {
diff --git a/src/test/compile-fail/object-safety-issue-22040.rs b/src/test/compile-fail/object-safety-issue-22040.rs
index 12407f06ca0..06d2441d3c0 100644
--- a/src/test/compile-fail/object-safety-issue-22040.rs
+++ b/src/test/compile-fail/object-safety-issue-22040.rs
@@ -26,6 +26,7 @@ struct SExpr<'x> {
 impl<'x> PartialEq for SExpr<'x> {
     fn eq(&self, other:&SExpr<'x>) -> bool {
         println!("L1: {} L2: {}", self.elements.len(), other.elements.len());
+
         let result = self.elements.len() == other.elements.len();
 
         println!("Got compare {}", result);
diff --git a/src/test/compile-fail/trait-test-2.rs b/src/test/compile-fail/trait-test-2.rs
index b11cbde2929..13fe314fbcd 100644
--- a/src/test/compile-fail/trait-test-2.rs
+++ b/src/test/compile-fail/trait-test-2.rs
@@ -21,4 +21,5 @@ fn main() {
     //~^ ERROR E0038
     //~| ERROR E0038
     //~| ERROR E0277
+    //~| WARNING E0038
 }
diff --git a/src/test/compile-fail/wf-method-late-bound-regions.rs b/src/test/compile-fail/wf-method-late-bound-regions.rs
new file mode 100644
index 00000000000..b9d292fd156
--- /dev/null
+++ b/src/test/compile-fail/wf-method-late-bound-regions.rs
@@ -0,0 +1,33 @@
+// 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.
+
+// A method's receiver must be well-formed, even if it has late-bound regions.
+// Because of this, a method's substs being well-formed does not imply that
+// the method's implied bounds are met.
+
+struct Foo<'b>(Option<&'b ()>);
+
+trait Bar<'b> {
+    fn xmute<'a>(&'a self, u: &'b u32) -> &'a u32;
+}
+
+impl<'b> Bar<'b> for Foo<'b> {
+    fn xmute<'a>(&'a self, u: &'b u32) -> &'a u32 { u }
+}
+
+fn main() {
+    let f = Foo(None);
+    let f2 = f;
+    let dangling = {
+        let pointer = Box::new(42);
+        f2.xmute(&pointer) //~ ERROR `pointer` does not live long enough
+    };
+    println!("{}", dangling);
+}
diff --git a/src/test/compile-fail/wf-misc-methods-issue-28609.rs b/src/test/compile-fail/wf-misc-methods-issue-28609.rs
new file mode 100644
index 00000000000..055c86a03a7
--- /dev/null
+++ b/src/test/compile-fail/wf-misc-methods-issue-28609.rs
@@ -0,0 +1,84 @@
+// 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.
+
+// check that misc. method calls are well-formed
+
+use std::marker::PhantomData;
+use std::ops::{Deref, Shl};
+
+#[derive(Copy, Clone)]
+struct S<'a, 'b: 'a> {
+    marker: PhantomData<&'a &'b ()>,
+    bomb: Option<&'b u32>
+}
+
+type S2<'a> = S<'a, 'a>;
+
+impl<'a, 'b> S<'a, 'b> {
+    fn transmute_inherent(&self, a: &'b u32) -> &'a u32 {
+        a
+    }
+}
+
+fn return_dangling_pointer_inherent(s: S2) -> &u32 {
+    let s = s;
+    s.transmute_inherent(&mut 42) //~ ERROR does not live long enough
+}
+
+impl<'a, 'b> Deref for S<'a, 'b> {
+    type Target = &'a u32;
+    fn deref(&self) -> &&'a u32 {
+        self.bomb.as_ref().unwrap()
+    }
+}
+
+fn return_dangling_pointer_coerce(s: S2) -> &u32 {
+    let four = 4;
+    let mut s = s;
+    s.bomb = Some(&four); //~ ERROR does not live long enough
+    &s
+}
+
+fn return_dangling_pointer_unary_op(s: S2) -> &u32 {
+    let four = 4;
+    let mut s = s;
+    s.bomb = Some(&four); //~ ERROR does not live long enough
+    &*s
+}
+
+impl<'a, 'b> Shl<&'b u32> for S<'a, 'b> {
+    type Output = &'a u32;
+    fn shl(self, t: &'b u32) -> &'a u32 { t }
+}
+
+fn return_dangling_pointer_binary_op(s: S2) -> &u32 {
+    let s = s;
+    s << &mut 3 //~ ERROR does not live long enough
+}
+
+fn return_dangling_pointer_method(s: S2) -> &u32 {
+    let s = s;
+    s.shl(&mut 3) //~ ERROR does not live long enough
+}
+
+fn return_dangling_pointer_ufcs(s: S2) -> &u32 {
+    let s = s;
+    S2::shl(s, &mut 3) //~ ERROR does not live long enough
+}
+
+fn main() {
+    let s = S { marker: PhantomData, bomb: None };
+    let _inherent_dp = return_dangling_pointer_inherent(s);
+    let _coerce_dp = return_dangling_pointer_coerce(s);
+    let _unary_dp = return_dangling_pointer_unary_op(s);
+    let _binary_dp = return_dangling_pointer_binary_op(s);
+    let _method_dp = return_dangling_pointer_method(s);
+    let _ufcs_dp = return_dangling_pointer_ufcs(s);
+}
diff --git a/src/test/compile-fail/wf-static-method.rs b/src/test/compile-fail/wf-static-method.rs
new file mode 100644
index 00000000000..6c6522fe658
--- /dev/null
+++ b/src/test/compile-fail/wf-static-method.rs
@@ -0,0 +1,64 @@
+// 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.
+
+// check that static methods don't get to assume their trait-ref
+// is well-formed.
+// FIXME(#27579): this is just a bug. However, our checking with
+// static inherent methods isn't quite working - need to
+// fix that before removing the check.
+
+trait Foo<'a, 'b, T>: Sized {
+    fn make_me() -> Self { loop {} }
+    fn static_evil(u: &'b u32) -> &'a u32;
+}
+
+struct Evil<'a, 'b: 'a>(Option<&'a &'b ()>);
+
+impl<'a, 'b> Foo<'a, 'b, Evil<'a, 'b>> for () {
+    fn make_me() -> Self { }
+    fn static_evil(u: &'b u32) -> &'a u32 {
+        u //~ ERROR cannot infer an appropriate lifetime
+    }
+}
+
+struct IndirectEvil<'a, 'b: 'a>(Option<&'a &'b ()>);
+
+impl<'a, 'b> Foo<'a, 'b, ()> for IndirectEvil<'a, 'b> {
+    fn make_me() -> Self { IndirectEvil(None) }
+    fn static_evil(u: &'b u32) -> &'a u32 {
+        let me = Self::make_me(); //~ ERROR lifetime bound not satisfied
+        loop {} // (`me` could be used for the lifetime transmute).
+    }
+}
+
+impl<'a, 'b> Evil<'a, 'b> {
+    fn inherent_evil(u: &'b u32) -> &'a u32 {
+        u //~ ERROR cannot infer an appropriate lifetime
+    }
+}
+
+// while static methods don't get to *assume* this, we still
+// *check* that they hold.
+
+fn evil<'a, 'b>(b: &'b u32) -> &'a u32 {
+    <()>::static_evil(b) //~ ERROR cannot infer an appropriate lifetime
+}
+
+fn indirect_evil<'a, 'b>(b: &'b u32) -> &'a u32 {
+    <IndirectEvil>::static_evil(b)
+    //~^ ERROR cannot infer an appropriate lifetime
+}
+
+fn inherent_evil<'a, 'b>(b: &'b u32) -> &'a u32 {
+    <Evil>::inherent_evil(b) // bug? shouldn't this be an error
+}
+
+
+fn main() {}