Skip to content

Commit 05e3248

Browse files
pcwaltonpcwalton
committed
librustc: Match trait self types exactly.
This can break code that looked like: impl Foo for Box<Any> { fn f(&self) { ... } } let x: Box<Any + Send> = ...; x.f(); Change such code to: impl Foo for Box<Any> { fn f(&self) { ... } } let x: Box<Any> = ...; x.f(); That is, upcast before calling methods. This is a conservative solution to #5781. A more proper treatment (see the xfail'd `trait-contravariant-self.rs`) would take variance into account. This change fixes the soundness hole. Some library changes had to be made to make this work. In particular, `Box<Any>` is no longer showable, and only `Box<Any+Send>` is showable. Eventually, this restriction can be lifted; for now, it does not prove too onerous, because `Any` is only used for propagating the result of task failure. This patch also adds a test for the variance inference work in #12828, which accidentally landed as part of DST. Closes #5781. [breaking-change]
1 parent afdfe40 commit 05e3248

20 files changed

+214
-84
lines changed

‎src/liballoc/owned.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use core::cmp::{PartialEq, PartialOrd, Eq, Ord, Ordering};
1616
use core::default::Default;
1717
use core::fmt;
1818
use core::intrinsics;
19+
use core::kinds::Send;
1920
use core::mem;
2021
use core::raw::TraitObject;
2122
use core::result::{Ok, Err, Result};
@@ -106,6 +107,34 @@ impl AnyOwnExt for Box<Any> {
106107
}
107108
}
108109

110+
/// Extension methods for an owning `Any+Send` trait object
111+
pub trait AnySendOwnExt {
112+
/// Returns the boxed value if it is of type `T`, or
113+
/// `Err(Self)` if it isn't.
114+
fn move_send<T: 'static>(self) -> Result<Box<T>, Self>;
115+
}
116+
117+
impl AnySendOwnExt for Box<Any+Send> {
118+
#[inline]
119+
fn move_send<T: 'static>(self) -> Result<Box<T>, Box<Any+Send>> {
120+
if self.is::<T>() {
121+
unsafe {
122+
// Get the raw representation of the trait object
123+
let to: TraitObject =
124+
*mem::transmute::<&Box<Any+Send>, &TraitObject>(&self);
125+
126+
// Prevent destructor on self being run
127+
intrinsics::forget(self);
128+
129+
// Extract the data pointer
130+
Ok(mem::transmute(to.data))
131+
}
132+
} else {
133+
Err(self)
134+
}
135+
}
136+
}
137+
109138
impl<T: fmt::Show> fmt::Show for Box<T> {
110139
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
111140
(**self).fmt(f)

‎src/libcore/iter.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,8 @@ pub trait Iterator<A> {
135135
/// let a = [0i];
136136
/// let b = [1i];
137137
/// let mut it = a.iter().zip(b.iter());
138-
/// assert_eq!(it.next().unwrap(), (&0, &1));
138+
/// let (x0, x1) = (0i, 1i);
139+
/// assert_eq!(it.next().unwrap(), (&x0, &x1));
139140
/// assert!(it.next().is_none());
140141
/// ```
141142
#[inline]
@@ -202,8 +203,9 @@ pub trait Iterator<A> {
202203
/// ```rust
203204
/// let a = [100i, 200];
204205
/// let mut it = a.iter().enumerate();
205-
/// assert_eq!(it.next().unwrap(), (0, &100));
206-
/// assert_eq!(it.next().unwrap(), (1, &200));
206+
/// let (x100, x200) = (100i, 200i);
207+
/// assert_eq!(it.next().unwrap(), (0, &x100));
208+
/// assert_eq!(it.next().unwrap(), (1, &x200));
207209
/// assert!(it.next().is_none());
208210
/// ```
209211
#[inline]
@@ -220,11 +222,11 @@ pub trait Iterator<A> {
220222
/// ```rust
221223
/// let xs = [100i, 200, 300];
222224
/// let mut it = xs.iter().map(|x| *x).peekable();
223-
/// assert_eq!(it.peek().unwrap(), &100);
225+
/// assert_eq!(*it.peek().unwrap(), 100);
224226
/// assert_eq!(it.next().unwrap(), 100);
225227
/// assert_eq!(it.next().unwrap(), 200);
226-
/// assert_eq!(it.peek().unwrap(), &300);
227-
/// assert_eq!(it.peek().unwrap(), &300);
228+
/// assert_eq!(*it.peek().unwrap(), 300);
229+
/// assert_eq!(*it.peek().unwrap(), 300);
228230
/// assert_eq!(it.next().unwrap(), 300);
229231
/// assert!(it.peek().is_none());
230232
/// assert!(it.next().is_none());

‎src/librustc/middle/typeck/check/vtable.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -352,17 +352,15 @@ fn search_for_vtable(vcx: &VtableContext,
352352
// the next impl.
353353
//
354354
// FIXME: document a bit more what this means
355-
//
356-
// FIXME(#5781) this should be mk_eqty not mk_subty
357355
let TypeAndSubsts {
358356
substs: substs,
359357
ty: for_ty
360358
} = impl_self_ty(vcx, span, impl_did);
361-
match infer::mk_subty(vcx.infcx,
362-
false,
363-
infer::RelateSelfType(span),
364-
ty,
365-
for_ty) {
359+
match infer::mk_eqty(vcx.infcx,
360+
false,
361+
infer::RelateSelfType(span),
362+
ty,
363+
for_ty) {
366364
Err(_) => continue,
367365
Ok(()) => ()
368366
}

‎src/librustc/middle/typeck/infer/combine.rs

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -84,41 +84,25 @@ pub trait Combine {
8484
fn tys(&self, a: ty::t, b: ty::t) -> cres<ty::t>;
8585

8686
fn tps(&self,
87-
space: subst::ParamSpace,
87+
_: subst::ParamSpace,
8888
as_: &[ty::t],
8989
bs: &[ty::t])
90-
-> cres<Vec<ty::t>>
91-
{
92-
// FIXME(#5781) -- In general, we treat variance a bit wrong
93-
// here. For historical reasons, we treat Self as
94-
// contravariant and other tps as invariant. Both are wrong:
95-
// Self may or may not be contravariant, and other tps do not
96-
// need to be invariant.
90+
-> cres<Vec<ty::t>> {
91+
// FIXME -- In general, we treat variance a bit wrong
92+
// here. For historical reasons, we treat tps and Self
93+
// as invariant. This is overly conservative.
9794

9895
if as_.len() != bs.len() {
9996
return Err(ty::terr_ty_param_size(expected_found(self,
10097
as_.len(),
10198
bs.len())));
10299
}
103100

104-
match space {
105-
subst::SelfSpace => {
106-
result::fold(as_
107-
.iter()
108-
.zip(bs.iter())
109-
.map(|(a, b)| self.contratys(*a, *b)),
110-
Vec::new(),
111-
|mut v, a| { v.push(a); v })
112-
}
113-
114-
subst::TypeSpace | subst::FnSpace => {
115-
try!(result::fold_(as_
116-
.iter()
117-
.zip(bs.iter())
118-
.map(|(a, b)| eq_tys(self, *a, *b))));
119-
Ok(Vec::from_slice(as_))
120-
}
121-
}
101+
try!(result::fold_(as_
102+
.iter()
103+
.zip(bs.iter())
104+
.map(|(a, b)| eq_tys(self, *a, *b))));
105+
Ok(Vec::from_slice(as_))
122106
}
123107

124108
fn substs(&self,

‎src/librustc/middle/typeck/variance.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,8 @@ receiver position from being called via an object.)
7878
#### Trait variance and vtable resolution
7979
8080
But traits aren't only used with objects. They're also used when
81-
deciding whether a given impl satisfies a given trait bound (or should
82-
be -- FIXME #5781). To set the scene here, imagine I had a function:
81+
deciding whether a given impl satisfies a given trait bound. To set the
82+
scene here, imagine I had a function:
8383
8484
fn convertAll<A,T:ConvertTo<A>>(v: &[T]) {
8585
...

‎src/librustdoc/test.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,14 @@ fn runtest(test: &str, cratename: &str, libs: HashSet<Path>, should_fail: bool,
140140
let old = io::stdio::set_stderr(box w1);
141141
spawn(proc() {
142142
let mut p = io::ChanReader::new(rx);
143-
let mut err = old.unwrap_or(box io::stderr() as Box<Writer + Send>);
143+
let mut err = match old {
144+
Some(old) => {
145+
// Chop off the `Send` bound.
146+
let old: Box<Writer> = old;
147+
old
148+
}
149+
None => box io::stderr() as Box<Writer>,
150+
};
144151
io::util::copy(&mut p, &mut err).unwrap();
145152
});
146153
let emitter = diagnostic::EmitterWriter::new(box w2);

‎src/libstd/io/comm_adapters.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,10 @@ mod test {
183183
writer.write_be_u32(42).unwrap();
184184

185185
let wanted = vec![0u8, 0u8, 0u8, 42u8];
186-
let got = task::try(proc() { rx.recv() }).unwrap();
186+
let got = match task::try(proc() { rx.recv() }) {
187+
Ok(got) => got,
188+
Err(_) => fail!(),
189+
};
187190
assert_eq!(wanted, got);
188191

189192
match writer.write_u8(1) {

‎src/libstd/task.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -630,9 +630,11 @@ mod test {
630630
let mut reader = ChanReader::new(rx);
631631
let stdout = ChanWriter::new(tx);
632632

633-
TaskBuilder::new().stdout(box stdout as Box<Writer + Send>).try(proc() {
634-
print!("Hello, world!");
635-
}).unwrap();
633+
let r = TaskBuilder::new().stdout(box stdout as Box<Writer + Send>)
634+
.try(proc() {
635+
print!("Hello, world!");
636+
});
637+
assert!(r.is_ok());
636638

637639
let output = reader.read_to_str().unwrap();
638640
assert_eq!(output, "Hello, world!".to_string());

‎src/libsyntax/diagnostic.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use std::fmt;
1818
use std::io;
1919
use std::iter::range;
2020
use std::string::String;
21+
use term::WriterWrapper;
2122
use term;
2223

2324
// maximum number of lines we will print for each error; arbitrary.
@@ -281,7 +282,7 @@ pub struct EmitterWriter {
281282
}
282283

283284
enum Destination {
284-
Terminal(Box<term::Terminal<Box<Writer + Send>> + Send>),
285+
Terminal(Box<term::Terminal<WriterWrapper> + Send>),
285286
Raw(Box<Writer + Send>),
286287
}
287288

‎src/libterm/lib.rs

Lines changed: 52 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -66,54 +66,84 @@ pub mod terminfo;
6666
#[cfg(windows)]
6767
mod win;
6868

69+
/// A hack to work around the fact that `Box<Writer + Send>` does not
70+
/// currently implement `Writer`.
71+
pub struct WriterWrapper {
72+
wrapped: Box<Writer + Send>,
73+
}
74+
75+
impl Writer for WriterWrapper {
76+
#[inline]
77+
fn write(&mut self, buf: &[u8]) -> IoResult<()> {
78+
self.wrapped.write(buf)
79+
}
80+
81+
#[inline]
82+
fn flush(&mut self) -> IoResult<()> {
83+
self.wrapped.flush()
84+
}
85+
}
86+
6987
#[cfg(not(windows))]
7088
/// Return a Terminal wrapping stdout, or None if a terminal couldn't be
7189
/// opened.
72-
pub fn stdout() -> Option<Box<Terminal<Box<Writer + Send>> + Send>> {
73-
let ti: Option<TerminfoTerminal<Box<Writer + Send>>>
74-
= Terminal::new(box std::io::stdout() as Box<Writer + Send>);
75-
ti.map(|t| box t as Box<Terminal<Box<Writer + Send> + Send> + Send>)
90+
pub fn stdout() -> Option<Box<Terminal<WriterWrapper> + Send>> {
91+
let ti: Option<TerminfoTerminal<WriterWrapper>>
92+
= Terminal::new(WriterWrapper {
93+
wrapped: box std::io::stdout() as Box<Writer + Send>,
94+
});
95+
ti.map(|t| box t as Box<Terminal<WriterWrapper> + Send>)
7696
}
7797

7898
#[cfg(windows)]
7999
/// Return a Terminal wrapping stdout, or None if a terminal couldn't be
80100
/// opened.
81-
pub fn stdout() -> Option<Box<Terminal<Box<Writer + Send> + Send> + Send>> {
82-
let ti: Option<TerminfoTerminal<Box<Writer + Send>>>
83-
= Terminal::new(box std::io::stdout() as Box<Writer + Send>);
101+
pub fn stdout() -> Option<Box<Terminal<WriterWrapper> + Send>> {
102+
let ti: Option<TerminfoTerminal<WriterWrapper>>
103+
= Terminal::new(WriterWrapper {
104+
wrapped: box std::io::stdout() as Box<Writer + Send>,
105+
});
84106

85107
match ti {
86-
Some(t) => Some(box t as Box<Terminal<Box<Writer + Send> + Send> + Send>),
108+
Some(t) => Some(box t as Box<Terminal<WriterWrapper> + Send>),
87109
None => {
88-
let wc: Option<WinConsole<Box<Writer + Send>>>
89-
= Terminal::new(box std::io::stdout() as Box<Writer + Send>);
90-
wc.map(|w| box w as Box<Terminal<Box<Writer + Send> + Send> + Send>)
110+
let wc: Option<WinConsole<WriterWrapper>>
111+
= Terminal::new(WriterWrapper {
112+
wrapped: box std::io::stdout() as Box<Writer + Send>,
113+
});
114+
wc.map(|w| box w as Box<Terminal<WriterWrapper> + Send>)
91115
}
92116
}
93117
}
94118

95119
#[cfg(not(windows))]
96120
/// Return a Terminal wrapping stderr, or None if a terminal couldn't be
97121
/// opened.
98-
pub fn stderr() -> Option<Box<Terminal<Box<Writer + Send> + Send> + Send> + Send> {
99-
let ti: Option<TerminfoTerminal<Box<Writer + Send>>>
100-
= Terminal::new(box std::io::stderr() as Box<Writer + Send>);
101-
ti.map(|t| box t as Box<Terminal<Box<Writer + Send> + Send> + Send>)
122+
pub fn stderr() -> Option<Box<Terminal<WriterWrapper> + Send> + Send> {
123+
let ti: Option<TerminfoTerminal<WriterWrapper>>
124+
= Terminal::new(WriterWrapper {
125+
wrapped: box std::io::stderr() as Box<Writer + Send>,
126+
});
127+
ti.map(|t| box t as Box<Terminal<WriterWrapper> + Send>)
102128
}
103129

104130
#[cfg(windows)]
105131
/// Return a Terminal wrapping stderr, or None if a terminal couldn't be
106132
/// opened.
107-
pub fn stderr() -> Option<Box<Terminal<Box<Writer + Send> + Send> + Send>> {
108-
let ti: Option<TerminfoTerminal<Box<Writer + Send>>>
109-
= Terminal::new(box std::io::stderr() as Box<Writer + Send>);
133+
pub fn stderr() -> Option<Box<Terminal<WriterWrapper> + Send> + Send> {
134+
let ti: Option<TerminfoTerminal<WriterWrapper>>
135+
= Terminal::new(WriterWrapper {
136+
wrapped: box std::io::stderr() as Box<Writer + Send>,
137+
});
110138

111139
match ti {
112-
Some(t) => Some(box t as Box<Terminal<Box<Writer + Send> + Send> + Send>),
140+
Some(t) => Some(box t as Box<Terminal<WriterWrapper> + Send>),
113141
None => {
114-
let wc: Option<WinConsole<Box<Writer + Send>>>
115-
= Terminal::new(box std::io::stderr() as Box<Writer + Send>);
116-
wc.map(|w| box w as Box<Terminal<Box<Writer + Send> + Send> + Send>)
142+
let wc: Option<WinConsole<WriterWrapper>>
143+
= Terminal::new(WriterWrapper {
144+
wrapped: box std::io::stderr() as Box<Writer + Send>,
145+
});
146+
wc.map(|w| box w as Box<Terminal<WriterWrapper> + Send>)
117147
}
118148
}
119149
}

‎src/libtest/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ pub enum TestResult {
474474
}
475475

476476
enum OutputLocation<T> {
477-
Pretty(Box<term::Terminal<Box<Writer + Send>> + Send>),
477+
Pretty(Box<term::Terminal<term::WriterWrapper> + Send>),
478478
Raw(T),
479479
}
480480

‎src/test/compile-fail/regions-escape-via-trait-or-not.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ fn with<R:deref>(f: |x: &int| -> R) -> int {
2323
}
2424

2525
fn return_it() -> int {
26-
with(|o| o) //~ ERROR lifetime of function argument does not outlive the function call
26+
with(|o| o) //~ ERROR cannot infer an appropriate lifetime
2727
}
2828

2929
fn main() {

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright 2014 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+
extern crate serialize;
12+
13+
use std::io::MemWriter;
14+
use std::io;
15+
use serialize::{Encodable, Encoder};
16+
17+
pub fn buffer_encode<'a,
18+
T:Encodable<serialize::json::Encoder<'a>,io::IoError>>(
19+
to_encode_object: &T)
20+
-> Vec<u8> {
21+
let mut m = MemWriter::new();
22+
{
23+
let mut encoder =
24+
serialize::json::Encoder::new(&mut m as &mut io::Writer);
25+
//~^ ERROR `m` does not live long enough
26+
to_encode_object.encode(&mut encoder);
27+
}
28+
m.unwrap()
29+
}
30+
31+
fn main() {}

0 commit comments

Comments
 (0)