Skip to content

Commit 130851e

Browse files
borsbors
committed
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
2 parents 6d11a81 + 2f23e17 commit 130851e

File tree

10 files changed

+206
-10
lines changed

10 files changed

+206
-10
lines changed

‎src/librustc/middle/ty/structural_impls.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,9 @@ impl<'tcx, A: Lift<'tcx>, B: Lift<'tcx>> Lift<'tcx> for (A, B) {
417417
impl<'tcx, T: Lift<'tcx>> Lift<'tcx> for [T] {
418418
type Lifted = Vec<T::Lifted>;
419419
fn lift_to_tcx(&self, tcx: &ty::ctxt<'tcx>) -> Option<Self::Lifted> {
420-
let mut result = Vec::with_capacity(self.len());
420+
// type annotation needed to inform `projection_must_outlive`
421+
let mut result : Vec<<T as Lift<'tcx>>::Lifted>
422+
= Vec::with_capacity(self.len());
421423
for x in self {
422424
if let Some(value) = tcx.lift(x) {
423425
result.push(value);

‎src/librustc_typeck/check/method/confirm.rs

+14-8
Original file line numberDiff line numberDiff line change
@@ -103,22 +103,23 @@ impl<'a,'tcx> ConfirmContext<'a,'tcx> {
103103
// Unify the (adjusted) self type with what the method expects.
104104
self.unify_receivers(self_ty, method_self_ty);
105105

106-
// Add any trait/regions obligations specified on the method's type parameters.
107-
self.add_obligations(&pick, &all_substs, &method_predicates);
108-
109-
// Create the final `MethodCallee`.
106+
// Create the method type
110107
let method_ty = pick.item.as_opt_method().unwrap();
111108
let fty = self.tcx().mk_fn(None, self.tcx().mk_bare_fn(ty::BareFnTy {
112109
sig: ty::Binder(method_sig),
113110
unsafety: method_ty.fty.unsafety,
114111
abi: method_ty.fty.abi.clone(),
115112
}));
113+
114+
// Add any trait/regions obligations specified on the method's type parameters.
115+
self.add_obligations(fty, &all_substs, &method_predicates);
116+
117+
// Create the final `MethodCallee`.
116118
let callee = ty::MethodCallee {
117119
def_id: pick.item.def_id(),
118120
ty: fty,
119121
substs: self.tcx().mk_substs(all_substs)
120122
};
121-
122123
// If this is an `&mut self` method, bias the receiver
123124
// expression towards mutability (this will switch
124125
// e.g. `Deref` to `DerefMut` in overloaded derefs and so on).
@@ -422,11 +423,11 @@ impl<'a,'tcx> ConfirmContext<'a,'tcx> {
422423
}
423424

424425
fn add_obligations(&mut self,
425-
pick: &probe::Pick<'tcx>,
426+
fty: Ty<'tcx>,
426427
all_substs: &subst::Substs<'tcx>,
427428
method_predicates: &ty::InstantiatedPredicates<'tcx>) {
428-
debug!("add_obligations: pick={:?} all_substs={:?} method_predicates={:?}",
429-
pick,
429+
debug!("add_obligations: fty={:?} all_substs={:?} method_predicates={:?}",
430+
fty,
430431
all_substs,
431432
method_predicates);
432433

@@ -439,6 +440,11 @@ impl<'a,'tcx> ConfirmContext<'a,'tcx> {
439440
self.fcx.add_wf_bounds(
440441
all_substs,
441442
self.call_expr);
443+
444+
// the function type must also be well-formed (this is not
445+
// implied by the substs being well-formed because of inherent
446+
// impls and late-bound regions - see issue #28609).
447+
self.fcx.register_wf_obligation(fty, self.span, traits::MiscObligation);
442448
}
443449

444450
///////////////////////////////////////////////////////////////////////////

‎src/librustc_typeck/check/method/mod.rs

+3
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,9 @@ pub fn lookup_in_trait_adjusted<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
255255
traits::ObligationCause::misc(span, fcx.body_id),
256256
&method_bounds);
257257

258+
// Also register an obligation for the method type being well-formed.
259+
fcx.register_wf_obligation(fty, span, traits::MiscObligation);
260+
258261
// FIXME(#18653) -- Try to resolve obligations, giving us more
259262
// typing information, which can sometimes be needed to avoid
260263
// pathological region inference failures.

‎src/libstd/thread/mod.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,8 @@ impl Builder {
266266
let my_thread = Thread::new(name);
267267
let their_thread = my_thread.clone();
268268

269-
let my_packet = Arc::new(UnsafeCell::new(None));
269+
let my_packet : Arc<UnsafeCell<Option<Result<T>>>>
270+
= Arc::new(UnsafeCell::new(None));
270271
let their_packet = my_packet.clone();
271272

272273
let main = move || {

‎src/test/compile-fail/issue-18959.rs

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ fn foo(b: &Bar) {
2222
b.foo(&0)
2323
//~^ ERROR the trait `Foo` is not implemented for the type `Bar`
2424
//~| ERROR E0038
25+
//~| WARNING E0038
2526
}
2627

2728
fn main() {

‎src/test/compile-fail/object-safety-issue-22040.rs

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ struct SExpr<'x> {
2626
impl<'x> PartialEq for SExpr<'x> {
2727
fn eq(&self, other:&SExpr<'x>) -> bool {
2828
println!("L1: {} L2: {}", self.elements.len(), other.elements.len());
29+
2930
let result = self.elements.len() == other.elements.len();
3031

3132
println!("Got compare {}", result);

‎src/test/compile-fail/trait-test-2.rs

+1
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,5 @@ fn main() {
2121
//~^ ERROR E0038
2222
//~| ERROR E0038
2323
//~| ERROR E0277
24+
//~| WARNING E0038
2425
}

‎src/test/compile-fail/wf-method-late-bound-regions.rs

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
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+
// A method's receiver must be well-formed, even if it has late-bound regions.
12+
// Because of this, a method's substs being well-formed does not imply that
13+
// the method's implied bounds are met.
14+
15+
struct Foo<'b>(Option<&'b ()>);
16+
17+
trait Bar<'b> {
18+
fn xmute<'a>(&'a self, u: &'b u32) -> &'a u32;
19+
}
20+
21+
impl<'b> Bar<'b> for Foo<'b> {
22+
fn xmute<'a>(&'a self, u: &'b u32) -> &'a u32 { u }
23+
}
24+
25+
fn main() {
26+
let f = Foo(None);
27+
let f2 = f;
28+
let dangling = {
29+
let pointer = Box::new(42);
30+
f2.xmute(&pointer) //~ ERROR `pointer` does not live long enough
31+
};
32+
println!("{}", dangling);
33+
}

‎src/test/compile-fail/wf-misc-methods-issue-28609.rs

+84
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
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+
// check that misc. method calls are well-formed
12+
13+
use std::marker::PhantomData;
14+
use std::ops::{Deref, Shl};
15+
16+
#[derive(Copy, Clone)]
17+
struct S<'a, 'b: 'a> {
18+
marker: PhantomData<&'a &'b ()>,
19+
bomb: Option<&'b u32>
20+
}
21+
22+
type S2<'a> = S<'a, 'a>;
23+
24+
impl<'a, 'b> S<'a, 'b> {
25+
fn transmute_inherent(&self, a: &'b u32) -> &'a u32 {
26+
a
27+
}
28+
}
29+
30+
fn return_dangling_pointer_inherent(s: S2) -> &u32 {
31+
let s = s;
32+
s.transmute_inherent(&mut 42) //~ ERROR does not live long enough
33+
}
34+
35+
impl<'a, 'b> Deref for S<'a, 'b> {
36+
type Target = &'a u32;
37+
fn deref(&self) -> &&'a u32 {
38+
self.bomb.as_ref().unwrap()
39+
}
40+
}
41+
42+
fn return_dangling_pointer_coerce(s: S2) -> &u32 {
43+
let four = 4;
44+
let mut s = s;
45+
s.bomb = Some(&four); //~ ERROR does not live long enough
46+
&s
47+
}
48+
49+
fn return_dangling_pointer_unary_op(s: S2) -> &u32 {
50+
let four = 4;
51+
let mut s = s;
52+
s.bomb = Some(&four); //~ ERROR does not live long enough
53+
&*s
54+
}
55+
56+
impl<'a, 'b> Shl<&'b u32> for S<'a, 'b> {
57+
type Output = &'a u32;
58+
fn shl(self, t: &'b u32) -> &'a u32 { t }
59+
}
60+
61+
fn return_dangling_pointer_binary_op(s: S2) -> &u32 {
62+
let s = s;
63+
s << &mut 3 //~ ERROR does not live long enough
64+
}
65+
66+
fn return_dangling_pointer_method(s: S2) -> &u32 {
67+
let s = s;
68+
s.shl(&mut 3) //~ ERROR does not live long enough
69+
}
70+
71+
fn return_dangling_pointer_ufcs(s: S2) -> &u32 {
72+
let s = s;
73+
S2::shl(s, &mut 3) //~ ERROR does not live long enough
74+
}
75+
76+
fn main() {
77+
let s = S { marker: PhantomData, bomb: None };
78+
let _inherent_dp = return_dangling_pointer_inherent(s);
79+
let _coerce_dp = return_dangling_pointer_coerce(s);
80+
let _unary_dp = return_dangling_pointer_unary_op(s);
81+
let _binary_dp = return_dangling_pointer_binary_op(s);
82+
let _method_dp = return_dangling_pointer_method(s);
83+
let _ufcs_dp = return_dangling_pointer_ufcs(s);
84+
}

‎src/test/compile-fail/wf-static-method.rs

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
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+
// check that static methods don't get to assume their trait-ref
12+
// is well-formed.
13+
// FIXME(#27579): this is just a bug. However, our checking with
14+
// static inherent methods isn't quite working - need to
15+
// fix that before removing the check.
16+
17+
trait Foo<'a, 'b, T>: Sized {
18+
fn make_me() -> Self { loop {} }
19+
fn static_evil(u: &'b u32) -> &'a u32;
20+
}
21+
22+
struct Evil<'a, 'b: 'a>(Option<&'a &'b ()>);
23+
24+
impl<'a, 'b> Foo<'a, 'b, Evil<'a, 'b>> for () {
25+
fn make_me() -> Self { }
26+
fn static_evil(u: &'b u32) -> &'a u32 {
27+
u //~ ERROR cannot infer an appropriate lifetime
28+
}
29+
}
30+
31+
struct IndirectEvil<'a, 'b: 'a>(Option<&'a &'b ()>);
32+
33+
impl<'a, 'b> Foo<'a, 'b, ()> for IndirectEvil<'a, 'b> {
34+
fn make_me() -> Self { IndirectEvil(None) }
35+
fn static_evil(u: &'b u32) -> &'a u32 {
36+
let me = Self::make_me(); //~ ERROR lifetime bound not satisfied
37+
loop {} // (`me` could be used for the lifetime transmute).
38+
}
39+
}
40+
41+
impl<'a, 'b> Evil<'a, 'b> {
42+
fn inherent_evil(u: &'b u32) -> &'a u32 {
43+
u //~ ERROR cannot infer an appropriate lifetime
44+
}
45+
}
46+
47+
// while static methods don't get to *assume* this, we still
48+
// *check* that they hold.
49+
50+
fn evil<'a, 'b>(b: &'b u32) -> &'a u32 {
51+
<()>::static_evil(b) //~ ERROR cannot infer an appropriate lifetime
52+
}
53+
54+
fn indirect_evil<'a, 'b>(b: &'b u32) -> &'a u32 {
55+
<IndirectEvil>::static_evil(b)
56+
//~^ ERROR cannot infer an appropriate lifetime
57+
}
58+
59+
fn inherent_evil<'a, 'b>(b: &'b u32) -> &'a u32 {
60+
<Evil>::inherent_evil(b) // bug? shouldn't this be an error
61+
}
62+
63+
64+
fn main() {}

0 commit comments

Comments
 (0)