Skip to content

Commit

Permalink
librustc: Match trait self types exactly.
Browse files Browse the repository at this point in the history
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 rust-lang#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 rust-lang#12828,
which accidentally landed as part of DST.

Closes rust-lang#5781.

[breaking-change]
  • Loading branch information
pcwalton committed Jun 28, 2014
1 parent afdfe40 commit 05e3248
Show file tree
Hide file tree
Showing 20 changed files with 214 additions and 84 deletions.
29 changes: 29 additions & 0 deletions src/liballoc/owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use core::cmp::{PartialEq, PartialOrd, Eq, Ord, Ordering};
use core::default::Default;
use core::fmt;
use core::intrinsics;
use core::kinds::Send;
use core::mem;
use core::raw::TraitObject;
use core::result::{Ok, Err, Result};
Expand Down Expand Up @@ -106,6 +107,34 @@ impl AnyOwnExt for Box<Any> {
}
}

/// Extension methods for an owning `Any+Send` trait object
pub trait AnySendOwnExt {
/// Returns the boxed value if it is of type `T`, or
/// `Err(Self)` if it isn't.
fn move_send<T: 'static>(self) -> Result<Box<T>, Self>;
}

impl AnySendOwnExt for Box<Any+Send> {
#[inline]
fn move_send<T: 'static>(self) -> Result<Box<T>, Box<Any+Send>> {
if self.is::<T>() {
unsafe {
// Get the raw representation of the trait object
let to: TraitObject =
*mem::transmute::<&Box<Any+Send>, &TraitObject>(&self);

// Prevent destructor on self being run
intrinsics::forget(self);

// Extract the data pointer
Ok(mem::transmute(to.data))
}
} else {
Err(self)
}
}
}

impl<T: fmt::Show> fmt::Show for Box<T> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
(**self).fmt(f)
Expand Down
14 changes: 8 additions & 6 deletions src/libcore/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ pub trait Iterator<A> {
/// let a = [0i];
/// let b = [1i];
/// let mut it = a.iter().zip(b.iter());
/// assert_eq!(it.next().unwrap(), (&0, &1));
/// let (x0, x1) = (0i, 1i);
/// assert_eq!(it.next().unwrap(), (&x0, &x1));
/// assert!(it.next().is_none());
/// ```
#[inline]
Expand Down Expand Up @@ -202,8 +203,9 @@ pub trait Iterator<A> {
/// ```rust
/// let a = [100i, 200];
/// let mut it = a.iter().enumerate();
/// assert_eq!(it.next().unwrap(), (0, &100));
/// assert_eq!(it.next().unwrap(), (1, &200));
/// let (x100, x200) = (100i, 200i);
/// assert_eq!(it.next().unwrap(), (0, &x100));
/// assert_eq!(it.next().unwrap(), (1, &x200));
/// assert!(it.next().is_none());
/// ```
#[inline]
Expand All @@ -220,11 +222,11 @@ pub trait Iterator<A> {
/// ```rust
/// let xs = [100i, 200, 300];
/// let mut it = xs.iter().map(|x| *x).peekable();
/// assert_eq!(it.peek().unwrap(), &100);
/// assert_eq!(*it.peek().unwrap(), 100);
/// assert_eq!(it.next().unwrap(), 100);
/// assert_eq!(it.next().unwrap(), 200);
/// assert_eq!(it.peek().unwrap(), &300);
/// assert_eq!(it.peek().unwrap(), &300);
/// assert_eq!(*it.peek().unwrap(), 300);
/// assert_eq!(*it.peek().unwrap(), 300);
/// assert_eq!(it.next().unwrap(), 300);
/// assert!(it.peek().is_none());
/// assert!(it.next().is_none());
Expand Down
12 changes: 5 additions & 7 deletions src/librustc/middle/typeck/check/vtable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,17 +352,15 @@ fn search_for_vtable(vcx: &VtableContext,
// the next impl.
//
// FIXME: document a bit more what this means
//
// FIXME(#5781) this should be mk_eqty not mk_subty
let TypeAndSubsts {
substs: substs,
ty: for_ty
} = impl_self_ty(vcx, span, impl_did);
match infer::mk_subty(vcx.infcx,
false,
infer::RelateSelfType(span),
ty,
for_ty) {
match infer::mk_eqty(vcx.infcx,
false,
infer::RelateSelfType(span),
ty,
for_ty) {
Err(_) => continue,
Ok(()) => ()
}
Expand Down
36 changes: 10 additions & 26 deletions src/librustc/middle/typeck/infer/combine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,41 +84,25 @@ pub trait Combine {
fn tys(&self, a: ty::t, b: ty::t) -> cres<ty::t>;

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

if as_.len() != bs.len() {
return Err(ty::terr_ty_param_size(expected_found(self,
as_.len(),
bs.len())));
}

match space {
subst::SelfSpace => {
result::fold(as_
.iter()
.zip(bs.iter())
.map(|(a, b)| self.contratys(*a, *b)),
Vec::new(),
|mut v, a| { v.push(a); v })
}

subst::TypeSpace | subst::FnSpace => {
try!(result::fold_(as_
.iter()
.zip(bs.iter())
.map(|(a, b)| eq_tys(self, *a, *b))));
Ok(Vec::from_slice(as_))
}
}
try!(result::fold_(as_
.iter()
.zip(bs.iter())
.map(|(a, b)| eq_tys(self, *a, *b))));
Ok(Vec::from_slice(as_))
}

fn substs(&self,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/typeck/variance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ receiver position from being called via an object.)
#### Trait variance and vtable resolution
But traits aren't only used with objects. They're also used when
deciding whether a given impl satisfies a given trait bound (or should
be -- FIXME #5781). To set the scene here, imagine I had a function:
deciding whether a given impl satisfies a given trait bound. To set the
scene here, imagine I had a function:
fn convertAll<A,T:ConvertTo<A>>(v: &[T]) {
...
Expand Down
9 changes: 8 additions & 1 deletion src/librustdoc/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,14 @@ fn runtest(test: &str, cratename: &str, libs: HashSet<Path>, should_fail: bool,
let old = io::stdio::set_stderr(box w1);
spawn(proc() {
let mut p = io::ChanReader::new(rx);
let mut err = old.unwrap_or(box io::stderr() as Box<Writer + Send>);
let mut err = match old {
Some(old) => {
// Chop off the `Send` bound.
let old: Box<Writer> = old;
old
}
None => box io::stderr() as Box<Writer>,
};
io::util::copy(&mut p, &mut err).unwrap();
});
let emitter = diagnostic::EmitterWriter::new(box w2);
Expand Down
5 changes: 4 additions & 1 deletion src/libstd/io/comm_adapters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,10 @@ mod test {
writer.write_be_u32(42).unwrap();

let wanted = vec![0u8, 0u8, 0u8, 42u8];
let got = task::try(proc() { rx.recv() }).unwrap();
let got = match task::try(proc() { rx.recv() }) {
Ok(got) => got,
Err(_) => fail!(),
};
assert_eq!(wanted, got);

match writer.write_u8(1) {
Expand Down
8 changes: 5 additions & 3 deletions src/libstd/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -630,9 +630,11 @@ mod test {
let mut reader = ChanReader::new(rx);
let stdout = ChanWriter::new(tx);

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

let output = reader.read_to_str().unwrap();
assert_eq!(output, "Hello, world!".to_string());
Expand Down
3 changes: 2 additions & 1 deletion src/libsyntax/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use std::fmt;
use std::io;
use std::iter::range;
use std::string::String;
use term::WriterWrapper;
use term;

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

enum Destination {
Terminal(Box<term::Terminal<Box<Writer + Send>> + Send>),
Terminal(Box<term::Terminal<WriterWrapper> + Send>),
Raw(Box<Writer + Send>),
}

Expand Down
74 changes: 52 additions & 22 deletions src/libterm/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,54 +66,84 @@ pub mod terminfo;
#[cfg(windows)]
mod win;

/// A hack to work around the fact that `Box<Writer + Send>` does not
/// currently implement `Writer`.
pub struct WriterWrapper {
wrapped: Box<Writer + Send>,
}

impl Writer for WriterWrapper {
#[inline]
fn write(&mut self, buf: &[u8]) -> IoResult<()> {
self.wrapped.write(buf)
}

#[inline]
fn flush(&mut self) -> IoResult<()> {
self.wrapped.flush()
}
}

#[cfg(not(windows))]
/// Return a Terminal wrapping stdout, or None if a terminal couldn't be
/// opened.
pub fn stdout() -> Option<Box<Terminal<Box<Writer + Send>> + Send>> {
let ti: Option<TerminfoTerminal<Box<Writer + Send>>>
= Terminal::new(box std::io::stdout() as Box<Writer + Send>);
ti.map(|t| box t as Box<Terminal<Box<Writer + Send> + Send> + Send>)
pub fn stdout() -> Option<Box<Terminal<WriterWrapper> + Send>> {
let ti: Option<TerminfoTerminal<WriterWrapper>>
= Terminal::new(WriterWrapper {
wrapped: box std::io::stdout() as Box<Writer + Send>,
});
ti.map(|t| box t as Box<Terminal<WriterWrapper> + Send>)
}

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

match ti {
Some(t) => Some(box t as Box<Terminal<Box<Writer + Send> + Send> + Send>),
Some(t) => Some(box t as Box<Terminal<WriterWrapper> + Send>),
None => {
let wc: Option<WinConsole<Box<Writer + Send>>>
= Terminal::new(box std::io::stdout() as Box<Writer + Send>);
wc.map(|w| box w as Box<Terminal<Box<Writer + Send> + Send> + Send>)
let wc: Option<WinConsole<WriterWrapper>>
= Terminal::new(WriterWrapper {
wrapped: box std::io::stdout() as Box<Writer + Send>,
});
wc.map(|w| box w as Box<Terminal<WriterWrapper> + Send>)
}
}
}

#[cfg(not(windows))]
/// Return a Terminal wrapping stderr, or None if a terminal couldn't be
/// opened.
pub fn stderr() -> Option<Box<Terminal<Box<Writer + Send> + Send> + Send> + Send> {
let ti: Option<TerminfoTerminal<Box<Writer + Send>>>
= Terminal::new(box std::io::stderr() as Box<Writer + Send>);
ti.map(|t| box t as Box<Terminal<Box<Writer + Send> + Send> + Send>)
pub fn stderr() -> Option<Box<Terminal<WriterWrapper> + Send> + Send> {
let ti: Option<TerminfoTerminal<WriterWrapper>>
= Terminal::new(WriterWrapper {
wrapped: box std::io::stderr() as Box<Writer + Send>,
});
ti.map(|t| box t as Box<Terminal<WriterWrapper> + Send>)
}

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

match ti {
Some(t) => Some(box t as Box<Terminal<Box<Writer + Send> + Send> + Send>),
Some(t) => Some(box t as Box<Terminal<WriterWrapper> + Send>),
None => {
let wc: Option<WinConsole<Box<Writer + Send>>>
= Terminal::new(box std::io::stderr() as Box<Writer + Send>);
wc.map(|w| box w as Box<Terminal<Box<Writer + Send> + Send> + Send>)
let wc: Option<WinConsole<WriterWrapper>>
= Terminal::new(WriterWrapper {
wrapped: box std::io::stderr() as Box<Writer + Send>,
});
wc.map(|w| box w as Box<Terminal<WriterWrapper> + Send>)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/libtest/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ pub enum TestResult {
}

enum OutputLocation<T> {
Pretty(Box<term::Terminal<Box<Writer + Send>> + Send>),
Pretty(Box<term::Terminal<term::WriterWrapper> + Send>),
Raw(T),
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/regions-escape-via-trait-or-not.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ fn with<R:deref>(f: |x: &int| -> R) -> int {
}

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

fn main() {
Expand Down
31 changes: 31 additions & 0 deletions src/test/compile-fail/variance-trait-matching-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2014 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.

extern crate serialize;

use std::io::MemWriter;
use std::io;
use serialize::{Encodable, Encoder};

pub fn buffer_encode<'a,
T:Encodable<serialize::json::Encoder<'a>,io::IoError>>(
to_encode_object: &T)
-> Vec<u8> {
let mut m = MemWriter::new();
{
let mut encoder =
serialize::json::Encoder::new(&mut m as &mut io::Writer);
//~^ ERROR `m` does not live long enough
to_encode_object.encode(&mut encoder);
}
m.unwrap()
}

fn main() {}
Loading

1 comment on commit 05e3248

@pcwalton
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=huonw

Please sign in to comment.